Skip to content

Fix/prop relax - #2705

Open
miratepuffin wants to merge 23 commits into
remote-py-apisfrom
fix/prop-relax
Open

Fix/prop relax#2705
miratepuffin wants to merge 23 commits into
remote-py-apisfrom
fix/prop-relax

Conversation

@miratepuffin

@miratepuffin miratepuffin commented Aug 9, 2026

Copy link
Copy Markdown
Collaborator

What changes were proposed in this pull request?

Property comparisons work across numeric types. A filter written as > 5 only ever compared
against properties of the same type, so it silently matched nothing on a u8, f64 or decimal
property. Comparisons now widen numerics first: 1, 1.0 and Decimal(1) are equal, integers
compare exactly, and a decimal on either side compares in decimal rather than losing precision
through floats. Property arithmetic got the same treatment — sums widen instead of overflowing
(u8u16→…→u64→decimal), an unsigned subtraction that goes negative becomes signed, and mean
keeps its running sum exact rather than drifting.

Composite filters no longer return edges to nodes they filtered out. filter(a & b) could
return an edge whose far endpoint had been excluded. The edge walk trusted its node list and only
checked the far end of each edge, but combining an enumerable filter with a non-enumerable one
produces a list that is a superset of the real nodes. It now only takes that shortcut when the list
is exact, and scans edges properly otherwise.

GraphQL filter fields are singular. Nodes/Edges became Node/Edge.

Vectors: six bugs fixed, plus partial indexing.

  • Re-vectorising a graph destroyed its index — the second attempt always failed on an existing table.
  • Vectorising failed after anything had read a reloaded graph, because the embedding cache opened
    its store twice. This is why vectorising appeared to "only work at startup".
  • A failed vectorise silently threw away the live index and still reported success.
  • Every update to a node or edge added a duplicate index row instead of replacing it, so a
    frequently-updated entity came back several times and matched on text it no longer had.
  • Embedding failures on writes were discarded entirely — no error, no log. All of them now log at
    error level, naming the graph.
  • An embedding failure during a vectorise panicked instead of returning an error.

New: vectoriseMissing embeds only entities absent from the index, for repairing gaps without
re-embedding the whole graph. vectoriseGraph still rebuilds everything, and now does it atomically
— it builds a new generation and switches over in one step, so the existing index keeps serving
throughout and an interrupted rebuild leaves it untouched. The rebuild also no longer holds the
graph's cache entry, which used to block every read of that graph for its duration.

Tantivy leftovers removed. The dependency was already gone; this removes the API it used to back,
all of it unreachable — the index spec types, unused error variants, and the client-side
create_index with its input classes, which errored against the server anyway.

Server odds and ends. A config flag to disable the UI, role handling in auth, and small fixes
around filtering, metadata and property handling.

Why are the changes needed?

The filter and vector issues are all silent wrong answers rather than errors: filters that quietly
matched nothing, searches that quietly returned nothing, entities quietly missing from an index. The
rest is cleanup of code that could not work if called.

Does this PR introduce any user-facing change? If yes is this documented?

Yes, three:

  • Nodes/Edges filter fields renamed to Node/Edge — breaking for existing queries.
  • Filters now return different results where numeric types differ (they match what they should) and
    composite filters return fewer, correct edges.
  • create_index and its input classes are gone from the client; vectoriseMissing is new.

How was this patch tested?

New tests for each area: a property type matrix covering comparison and arithmetic across every
numeric type, composite node/edge filter cases, vector coverage in both Rust and Python (partial
indexing, rebuild atomicity, failure handling, eviction and reload, concurrent reads and writes),
and auth tests for the UI flag. Full Rust and Python suites pass.

Are there any further changes required?

Not for this PR, but three things are worth their own tickets: nothing compacts or prunes the vector
store over time, the parquet bulk loaders skip embedding entirely, and vectorising needs an admin
token because there is no permission level between "can write" and "can set up vectorisation".

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Rust Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: 043250d Previous: 9823ef7 Ratio
lotr_graph/num_edges 4 ns/iter (± 0) 0 ns/iter (± 0) +∞
lotr_graph/num_nodes 5 ns/iter (± 0) 1 ns/iter (± 0) 5
lotr_graph/has_node_nonexisting 5 ns/iter (± 0) 2 ns/iter (± 0) 2.50
lotr_graph/graph_latest 3 ns/iter (± 0) 0 ns/iter (± 0) +∞
lotr_graph_materialise/materialize 8482038 ns/iter (± 167695) 1564816 ns/iter (± 35303) 5.42
lotr_graph_window_100/num_nodes 15 ns/iter (± 0) 5 ns/iter (± 0) 3
lotr_graph_window_100_materialise/materialize 8418868 ns/iter (± 62754) 1669150 ns/iter (± 10700) 5.04
lotr_graph_window_10/has_node_existing 144 ns/iter (± 9) 62 ns/iter (± 11) 2.32
lotr_graph_window_10_materialise/materialize 3368485 ns/iter (± 8265) 971980 ns/iter (± 4278) 3.47
lotr_graph_subgraph_10pc/has_node_nonexisting 5 ns/iter (± 0) 2 ns/iter (± 0) 2.50
lotr_graph_subgraph_10pc_materialise/materialize 2107548 ns/iter (± 48857) 334634 ns/iter (± 1287) 6.30
lotr_graph_subgraph_10pc_windowed/has_node_existing 150 ns/iter (± 12) 62 ns/iter (± 14) 2.42
lotr_graph_subgraph_10pc_windowed_materialise/materialize 1232827 ns/iter (± 17942) 230399 ns/iter (± 2617) 5.35
lotr_graph_window_50_layered/num_edges_temporal 150475 ns/iter (± 2091) 70121 ns/iter (± 7586) 2.15
lotr_graph_window_50_layered/has_node_existing 390 ns/iter (± 27) 129 ns/iter (± 12) 3.02
lotr_graph_window_50_layered/has_node_nonexisting 5 ns/iter (± 0) 2 ns/iter (± 0) 2.50
lotr_graph_window_50_layered/max_neighbour_degree 775823 ns/iter (± 6628) 386651 ns/iter (± 3853) 2.01
lotr_graph_window_50_layered/graph_latest 87347 ns/iter (± 1582) 36649 ns/iter (± 916) 2.38
lotr_graph_window_50_layered_materialise/materialize 32210861 ns/iter (± 674264) 3488825 ns/iter (± 24948) 9.23
lotr_graph_persistent_window_50_layered/num_edges_temporal 649378 ns/iter (± 28708) 192686 ns/iter (± 1569) 3.37
lotr_graph_persistent_window_50_layered/has_node_existing 440 ns/iter (± 424) 174 ns/iter (± 83) 2.53
lotr_graph_persistent_window_50_layered/has_node_nonexisting 5 ns/iter (± 0) 2 ns/iter (± 0) 2.50
lotr_graph_persistent_window_50_layered/iterate_exploded_edges 3602056 ns/iter (± 64519) 1659940 ns/iter (± 19402) 2.17
lotr_graph_persistent_window_50_layered/graph_latest 133134 ns/iter (± 5225) 57549 ns/iter (± 4809) 2.31
lotr_graph_persistent_window_50_layered_materialise/materialize 56468262 ns/iter (± 512517) 5298035 ns/iter (± 147912) 10.66

This comment was automatically generated by workflow using github-action-benchmark.

…in add, need to still update other operations)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants