feat(graph): wave-2 engine improvements (W2-1..W2-14) + P0 v3 graph-durability fix, GCP-hardened#237
Merged
Merged
Conversation
…hase 0)
Adds tests/graph_freeze_boundary.rs driving the real GRAPH.* command
handlers with edge_threshold=8 so MemGraph::freeze_and_compact fires
after 8 inserts. Documents the P0 found in the 2026-07 graph deep
review (tmp/GRAPH-DEEP-REVIEW-2026-07.md): freeze() DRAINS the write
buffer and CSR segments carry no properties, so compacted data is
lost to queries.
Red (9 failing / 1 passing mechanism-sanity):
- MATCH label scan / property filter / RETURN prop lose frozen nodes
- 1-hop expand from a frozen point query returns 0 rows
- mixed-tier scan sees only the mutable tail
- GRAPH.NEIGHBORS rejects frozen nodes ("node not found")
- GRAPH.ADDEDGE between frozen endpoints fails (append-once flaw)
- GRAPH.PROFILE returns 0 rows post-freeze
- GRAPH.HYBRID loses compacted candidates
These go green across Phase 0 (property blob v5, MergedNodeView,
NEIGHBORS/PROFILE/HYBRID rewiring).
author: Tin Dang
…ross freeze Fixes the freeze-boundary data-loss P0 (storage half): CsrSegment::from_frozen wrote property_offset: 0 unconditionally, silently DISCARDING node properties, embeddings, edge weights, and edge properties when a graph crossed edge_threshold (64K edges). - src/graph/csr/props.rs: self-contained record codec (tagged prop entries + embedding dim + edge weight), offset+1 scheme so 0 keeps meaning "absent". Bounds-checked decoders never panic; fuzz target graph_props_record added to fuzz/ and both CI fuzz matrices. - Format v5: two trailing [len:u64][bytes] blob sections after label_overflow; header layout unchanged (positional parsing like v4). v1-v4 files load with empty blobs (graceful degradation). New unsafe: two mmap pointer-slice accessors following the module's existing validated-pattern (SAFETY comments; flagged for unsafe-policy review). - from_frozen + compact_segments now encode/carry records; the merge copies each winner's record verbatim from its winning segment. - Bonus fix: compact_segments left node_id_to_row + MPH EMPTY, so a freshly merged in-memory segment could not resolve any NodeKey (lookup_node → None) until persisted and reloaded. Rebuilt from external_id like from_bytes does. - parse_label_overflow now returns its end offset (v5 sections follow it). Tests: props codec unit suite (incl. truncation sweep), v5 roundtrip (from_frozen / bytes / mmap), v4-compat parse, merged-segment carry-over + NodeKey lookup, downgrade helpers fixed for v5 trailer. graph lib 424/424. Read-side wiring (NodeScan/eval/NEIGHBORS/HYBRID) lands next — the freeze-boundary integration suite stays red until then by design. author: Tin Dang
…y collision fix Phase 0 task 3 of the graph-engine overhaul: post-freeze read correctness for Cypher MATCH / property eval, plus a newly discovered NodeKey aliasing bug in the freeze path. MergedNodeView (src/graph/view.rs): one seam over both tiers (mutable MemGraph write buffer + immutable CSR segments) — segment_row / contains / is_visible / property / properties / embedding / labels / for_each_visible_node. Frozen rows resolve properties through the v5 property blob, labels through label_bitmap + overflow, visibility through the new visibility::is_meta_visible (NodeMeta twin of is_node_visible, txn_id = 0 since frozen segments hold only committed data). Wired through: - Cypher NodeScan (execute + execute_profile): scans both tiers via for_each_visible_node — frozen nodes were invisible to MATCH before (freeze DRAINS the memgraph; the read side assumed a copy). - Expand target visibility: frozen targets now get the CSR NodeMeta MVCC/valid-time check instead of a silent free pass. - eval_expr PropertyAccess + labels(): merged-tier lookup, so RETURN n.prop / WHERE n.prop = x / labels(n) work on compacted nodes. NodeKey collision fix (P0, pre-existing): freeze_and_compact replaced write_buf with a FRESH MemGraph. A fresh SlotMap restarts key allocation at the same (index, generation) pairs, so the first node added after a freeze received the same NodeKey ffi as the first frozen node — aliasing new nodes onto frozen CSR rows in every merged lookup (SegmentMergeReader included). Fixed with MemGraph::thaw(): re-arm the SAME drained slot maps (drain bumps each slot's generation, guaranteeing fresh keys) and reset live counters; freeze_and_compact now thaws in place on both the success and CSR-build-failure paths. Freeze-boundary integration suite: 6/10 green (was 1/10) — label scan, property filter, property return, one-hop expand, mixed-tier scan. Remaining 4 red are the next tasks: NEIGHBORS/ADDEDGE existence + PROFILE Expand (task 4), GRAPH.HYBRID (task 5). Tests: graph lib suite 429/429; new red/green coverage: - store: test_freeze_and_compact_preserves_key_uniqueness (red on the fresh-MemGraph design, green with thaw) - view: two-tier fixture tests (contains / property+embedding / labels / merged scan with label filter) Also: /target-fast/ gitignored (release-fast iteration dir, matches the target-* convention). author: Tin Dang
… see frozen tier Phase 0 task 4: the native command surface and WAL replay now work across the freeze boundary, built on a new delta-edge mechanism in MemGraph. Delta edges (memgraph.rs): an edge whose endpoint was frozen into a CSR segment has no MutableNode to carry adjacency, and CsrSegment::from_frozen silently drops edges without in-segment endpoint rows. New design: - ghost_out / ghost_in maps hold adjacency keyed by frozen NodeKey; neighbors() serves them for non-resident nodes, so cross-tier edges are traversable from BOTH ends (SegmentMergeReader picks them up for free). - add_edge_across_tiers(): validates resident endpoints alive; trusts the caller (MergedNodeView::is_visible) for frozen ones. Plain add_edge keeps rejecting unknown endpoints. - freeze() now PARTITIONS edges: live + both endpoints resident → CSR; live + any earlier-frozen endpoint → retained in the mutable tier with its EdgeKey intact and ghost adjacency rebuilt; dead → dropped. freeze_and_compact skips empty freezes (all-delta buffer would otherwise churn empty segments) and thaw() recounts retained edges. Fixed call sites (each silently lost or rejected data post-freeze): - GRAPH.ADDEDGE: verifies frozen endpoints via MergedNodeView (alive in a segment), then inserts a delta edge — previously "ERR source or destination node not found". - GRAPH.NEIGHBORS: existence check via the merged view (comment claimed "freeze copies, doesn't move" — it drains); new DIRECTION IN|OUT|BOTH argument is parsed and honored (was hardcoded Both). - GRAPH.PROFILE Expand: now uses SegmentMergeReader + merged-view target visibility (parity with GRAPH.QUERY) — profiled expansions returned 0 rows on frozen graphs. - WAL replay AddEdge: node_map already seeded CSR-resident endpoints, but add_edge refused them, silently dropping every cross-segment edge during recovery; now inserts delta edges (node_map membership is the existence proof). Freeze-boundary suite: 10/11 green (was 6/10; +new DIRECTION test). Last red is GRAPH.HYBRID (next task). Graph lib 431/431, command::graph 25/25. author: Tin Dang
…complete Phase 0 task 5 (last freeze-boundary correctness gap): the hybrid graph+vector executors (HYB-01 FILTER, HYB-02 EXPAND, HYB-03 WALK, HYB-04 RERANK) read only the mutable write buffer — post-freeze they rejected the start node (NodeNotFound) or silently scored an empty candidate set. - All four executors now take the immutable CSR segments alongside the MemGraph: traversal goes through SegmentMergeReader (frozen + delta edges), node existence and embeddings resolve through MergedNodeView (embeddings decode from the CSR v5 blob for frozen rows). bfs_collect / collect_context are segment-aware; RERANK's all-nodes scan enumerates both tiers with MVCC visibility. Pass `&[]` for a bare MemGraph. - GRAPH.VSEARCH and GRAPH.HYBRID handlers load the segment snapshot and thread it through. - extract_f32_vector: binary little-endian f32 fallback after the text form — the SAME blob format GRAPH.ADDNODE's VECTOR argument accepts, so vectors round-trip between ADDNODE and VSEARCH/HYBRID unchanged (previously HYBRID only accepted whitespace-separated text and answered "ERR invalid vector" to its own storage format). Freeze-boundary suite: 11/11 GREEN (was 1/10 at review time) — Phase 0 exit criterion met. Graph lib 431/431. author: Tin Dang
…elete dead insert-time index Phase 1 task 6 — the storage half of the property-index lever identified by the 2026-07 deep review (point query = 99% unindexed label scan; FalkorDB wins reads on exactly this). SegmentPropertyIndexes (index.rs): per-CSR-segment indexes over row ids, built lazily on first use from the v5 node-property blob (OnceLock, same pattern as the incoming-edge index — one code path covers from_frozen, compact, from_bytes, and mmap reload). Numeric values (Int/Float/Bool) share a per-property B-tree (equality + range via the existing PropertyIndex); String/Bytes index by xxh64 hash (equality). Hash collisions and Bool/Int aliasing only ever produce SUPERSET candidate sets — the planner keeps its residual Filter, so collisions cost a re-check, never a wrong row. The build is exhaustive over segment rows, so lookups return empty bitmaps for absent values (no fall-back-to-scan signal needed; pre-v5 segments genuinely hold no properties). Accessor: CsrStorage::property_index() (heap + mmap variants). Deleted the dead insert-time maintenance: NamedGraph.property_indexes was populated on every ADDNODE with `ext_id as u32` — TRUNCATED external ids, a row space nothing could resolve — and no query path ever read it. Field, initializer, and the per-ADDNODE update loop are gone (the _key registration that lived in the same loop is preserved). Next (task 7): PhysicalOp::IndexScan consumes these via label-bitmap ∧ property-bitmap per segment. Tests: graph lib 432/432; new test_segment_property_index_eq_and_range covers Int/Float equality on one B-tree, string hash equality, range, and absent-property emptiness. author: Tin Dang
…operty indexes
Phase 1 task 7, the read-side half of the property-index lever (probe
showed Cypher point queries spend ~99% of their time in the unindexed
label scan; this is FalkorDB's entire read advantage).
Planner: a pattern node whose inline properties are all literals or
parameters (`(n:L {k:3})`, `(n:L {k:$v})`) now plans as
PhysicalOp::IndexScan { variable, label, prop_eq } instead of NodeScan —
emitted for the scanned first node and shortestPath endpoints (shared
push_node_scan helper). The full residual Filter is ALWAYS kept
immediately downstream: index lookups have superset semantics (string
hashes can collide, Bool/Int alias numerically), so the index only
prunes, never decides.
Executor (index_scan_keys, both execute and execute_profile):
- prop_eq values resolve per run via eval_expr (parameters index-hit under
the plan cache); an unresolvable value degrades to the merged label scan.
- Frozen tier: per segment, property bitmaps intersect (short-circuit on
empty) ∧ label bitmap, then MVCC/valid-time visibility per surviving row.
- Mutable tail: linear scan with a superset-consistent loose-equality
check (numeric coercion mirrors the index) — bounded by edge_threshold.
- Write-path executor treats IndexScan as its NodeScan (mutable-tier
MATCH semantics unchanged; residual Filter keeps it exact).
- GRAPH.PROFILE reports the op as "IndexScan".
Planner contract tests updated: literal inline props now pin
IndexScan+Filter (the v3-2 "Filter immediately follows the binding op"
contract is preserved, the binding op changed); new emission tests cover
literal / parameter / multi-prop / no-prop cases.
Known scope cut: WHERE-clause equality (`WHERE n.k = 3`) still plans as
NodeScan+Filter — extracting index-eligible conjuncts from WHERE is a
follow-up.
Tests: graph lib 433/433; freeze-boundary 11/11 (point-query tests now
exercise IndexScan end-to-end); tokio cypher suites green (inline-filter,
shortest-path, txn-write-rollback).
author: Tin Dang
…spatch
Phase 2 task 8. The plan cache was keyed on the raw query bytes, so the
dominant graph workload — point queries differing only in literal values —
NEVER hit it (every GRAPH.QUERY re-parsed and re-compiled). Eviction was
also arbitrary (HashMap keys().next()), and the server routing path parsed
every Cypher query TWICE (is_cypher_write_query full parse + handler parse).
Auto-parameterization (new src/graph/cypher/parameterize.rs): value
literals (int/float/string) are rewritten into synthetic parameters
($__p0, $__p1, …) at the TOKEN level, with values extracted into the
params map — so `{id: 3}` and `{id: 4}` share one cached plan and still
index-hit (PhysicalOp::IndexScan resolves Expr::Parameter per run).
Structural literals stay literal because they are baked into the plan,
not evaluated: variable-length hop bounds (prev token `*`/`..` or next
`..`), LIMIT/SKIP counts (conservative pin), and leading-minus literals
(logos maximal munch lexes `x-1` as Ident+Integer(-1); rewriting would
swallow a binary minus). Fail-open by construction: lex error, non-UTF-8
string, unparsable number, or a pre-existing `$__p` name returns None and
the caller falls back to raw-text raw-hash caching; if normalized text
ever failed to parse, parse_effective re-parses the raw text so user
errors reference their own query. Wrong results are impossible — only a
missed cache share.
Plan cache: proper LRU (monotonic access tick, evict min on overflow;
O(capacity) scan only when a full cache takes a new query text) and a new
invariant — only READ-ONLY plans are inserted (graph_query previously
cached write plans compiled for its error path).
Single-parse dispatch, enabled by that invariant:
- Cache HIT now runs with ZERO parse/compile work (previously: parse +
hash + lookup). The hit path routes read-only directly since only read
plans are cached.
- is_cypher_write_query: full parse → token scan for CREATE/DELETE/SET/
MERGE. May false-positive (property named `set`) which only routes via
the write-capable handler; graph_query_or_write AST-classifies and runs
the read path correctly. Never false-negatives for a parsable write.
- GRAPH.RO_QUERY: parsed twice before (own classify parse + delegated
graph_query parse); now shares graph_query_readonly (single parse, and
zero on cache hit — a hit is read-only by the invariant).
- Write path merges auto-params before execute_mut (CREATE literals are
normalized too; without the merge they would store Nulls).
Behavior notes: RO_QUERY of a write query against a MISSING graph now
reports "graph not found" instead of the write-clause error (graph lookup
precedes parse in the shared core); GRAPH.EXPLAIN/PROFILE stay raw-parse
(stable output, no cache).
Tests (red→green): plan-cache sharing across int and string literal
variants with per-query correct results, write-plan non-caching, LRU
eviction order, var-length hop bounds pinned to distinct plans (wrongly
parameterizing hops would return wrong row counts — pinned 5 vs 6 rows),
11 parameterize unit tests incl. normalized-text-reparses.
Suites: lib 3800 green, graph_freeze_boundary 11/11, tokio cypher suites
(inline-filter, shortest-path, txn-write-rollback) green, clippy both
feature sets, fmt.
author: Tin Dang
…tion Phase 2 task 9. Every executor row was a HashMap<String, Value>: each scan/expand/unwind emission allocated a fresh hasher table AND cloned the variable name String per binding; row clones re-hashed everything. For multi-row pipelines (the normal case) this dominated per-row cost. Row is now a slot-indexed SmallVec<[Value; 4]> plus a reference to a per-execution SlotTable (all variables the plan can bind, collected once by walking the binding operators: scans, expands, unwind, shortestPath, create/merge patterns). Name→slot resolution is a linear scan over a handful of short names — cheaper than hashing at this cardinality. The public row API is shape-compatible (get/insert/iter), so eval_expr's signature is unchanged; insert now takes &str, deleting the per-binding String clone. Semantics: unbound slots hold Value::Null, which every consumer already treats identically to the old HashMap miss — expression eval defaulted a missing variable to Null, and operator sources pattern-match a concrete variant (Some(Value::Node(_))), which Null fails exactly like None did. Binding is uniform per operator (each binding op binds its variable on every row it emits), so the no-Project column set (SlotTable names, sorted) and RETURN * maps match the old bound-keys view. Referenced-but-never-bound names resolve to no slot (None), matching a HashMap miss; a bound name missing from the table would be a plan/table desync and is debug_assert'ed. Tests: SlotTable collection (match + create/merge pattern vars), Row get/insert/iter/clone semantics incl. unbound-vs-unknown distinction. Suites: lib 3802 green, graph_freeze_boundary 11/11, tokio cypher suites green, clippy both feature sets. author: Tin Dang
Phase 2 task 10 — the deep review's allocation-discipline findings (NEIGHBORS profile: ~25% of time dropping Frames, ~20% in the ALLOCATING SegmentMergeReader::neighbors; ADDNODE 2× WAL-encode). - ADDNODE double WAL serialize deleted: a full record (incl. embedding bytes) was built with a placeholder id, dropped unread, and rebuilt from the stored node. Now a single serialization from the stored node's refs. - wal.rs: 15 `itoa::Buffer::format().to_owned()` sites drop the String — the formatted &str is passed straight to write_bulk (statement-temporary pattern already used by write_bulk itself). format_f64's format!() is replaced by ryu (already a dep) for the edge weight and Float property sites; replay parses f64 so "1.0"-vs-"1" style differences are inert. - New src/graph/fasthash.rs: FxHash multiply-fold (rustc's internal hasher, inlined — no new dependency). NodeKey/EdgeKey are slotmap PODs in maps we control (visited sets, dist maps), where SipHash's DoS resistance buys nothing. Module docs pin the provenance contract: never key by attacker-controlled bytes. - traversal.rs: all non-test NodeKey sets/maps → FxHashSet/FxHashMap; Dijkstra's dist/prev/depth map TRIPLE merged into one FxHashMap<NodeKey, DijkstraNodeState> — one hash lookup per relaxation where three (×SipHash) used to run. - Cypher Expand (both executors): the allocating `neighbors()` (fresh HashSet+Vec per source node — worst in variable-length BFS, per frontier node) replaced with `neighbors_into` on scratch hoisted per operator; BFS visited sets → FxHashSet. Deferred (follow-up, noted in review doc): Value::String(String)→Bytes to stop value_to_frame cloning string cells — ripples through all of eval's string ops, separate change. Tests: fasthash unit tests (map/set behavior, 10k-distinct-hash sanity); suites: lib 3804 green, graph_freeze_boundary 11/11, tokio cypher suites green, clippy both feature sets. author: Tin Dang
…n-optimizing pull Phase 3 task 11. BFS on a fully-frozen graph paid NodeKey hashing on every edge probe (visited set), MPH lookups per hop, and — in the old "ParallelBfs" — fake parallelism: the reader borrows !Send MemGraph, so neighbor expansion ran sequentially (with a per-node nb_buf.clone()) and worker threads only did DashSet merging. New src/graph/row_bfs.rs, gated per query (mutable tier absent-or-empty ∧ exactly one CSR segment ∧ segment visible at the snapshot — every case the reader path handles generically is a reason to fall back, never a behavior fork; BoundedBfs/ParallelBfs::execute try the gate first): - Row-space expansion: rows are dense u32s — visited set is a bitmap, adjacency is row_offsets/col_indices slice walks, NodeKey materialization happens once per RESULT row instead of per edge probe. - TRUE parallel levels: CsrStorage is Send+Sync, so frontier chunks expand on worker threads (thread::scope) against a shared AtomicU64 visited bitmap (fetch_or test-and-set); level triggers at frontier ≥ 256, ≤ 8 workers. - Direction-optimizing BFS (Beamer α=14/β=24): when the frontier's out-edge count dwarfs the unexplored remainder, the level switches to bottom-up pull over the v3-2 IncomingIndex (each unvisited row scans its in-edges for a frontier parent; rows partitioned per worker). Outgoing direction only; the lazy incoming index is warmed outside the worker scope. Semantics parity with the reader path: same edge-validity bitmap, same edge-type filter, same deleted_lsn node visibility, same MergedNeighbor materialization for CSR edges (placeholder edge key, weight 1.0, segment-LSN timestamp). Within a parallel level, discovery ORDER is unspecified (visited set + depths deterministic). Tests (7): push parity vs a hand-rolled reader-path oracle across all 3 directions × filters; pull-vs-push equality (incl. edge-type filter); parallel-level parity on a 2000-node star; gate fallback (mutable tier non-empty / two segments / future segment); deleted-node exclusion parity; depth limit + frontier cap; synthetic-key NodeNotFound (a key from a fresh MemGraph would ALIAS frozen rows — the task-3 collision class — so the test uses an out-of-range synthetic key). Suites: lib 3811 green, graph_freeze_boundary 11/11, tokio cypher suites green, clippy both feature sets. author: Tin Dang
…wPreFilter Phase 3 task 12 — the deep review's last two items. TraversalGuard threading (bounded epoch hold, wall-clock): - ExecutionContext gains `guard: Option<TraversalGuard>` (None default — Default/unit-test behavior unchanged) and ExecErrorKind gains Timeout(TraversalTimeout). - Both read executors (execute + execute_profile) check the guard once per hop in variable-length Expand BFS and once per row before each ShortestPath run; GRAPH.QUERY/RO_QUERY/PROFILE create the guard with the default 30s budget. The write executor is untouched: it has no ExecutionContext and walks the mutable tier only (hop-capped), so the epoch-hold risk the guard bounds — long traversals pinning old CSR snapshots — doesn't apply. Hybrid HnswPreFilter (was a silent stub — select_strategy picked it at >= 10K candidates, execute brute-forced anyway): - New src/graph/hnsw_bridge.rs: GraphHnsw builds the vector engine's HnswGraph (HnswBuilder, m=16/efC=128, deterministic seed) over a CSR segment's v5 embeddings with raw-f32 cosine distances — no TurboQuant. Embeddings copied once, unit-normalized (scores identical to simd::cosine_similarity). Search = greedy upper descent + L0 ef-beam (ef = max(64, 4k)), allow-filter applied to the collected beam. - Lazy per-segment build via CsrStorage::hnsw_bridge (OnceLock, same pattern as incoming/property indexes), gated at >= 4096 embeddings; the negative answer is cached. In-memory only, never persisted. - GraphFilteredSearch::execute now honors the strategy: CSR-resident candidates route through their segment's bridge (group top-k with exact cosines), everything else — mutable tier, bridge-less rows, dim mismatch, and any group whose approximate beam under-fills k — falls back to exact scoring. Never silently truncates. Tests: guard timeout red/green for var-length Expand + shortestPath (both executors) with generous-guard parity; bridge build/recall (>= 8/10 vs brute force, exact-score parity), allow-filter, min-vectors gate, degenerate queries, unembedded-row skip; hybrid HnswPreFilter vs brute-force overlap + bit-exact fallback without a bridge + tier partition contract. Suites: lib 3821 green, graph_freeze_boundary 11/11, tokio cypher suites green, clippy both feature sets, fmt. author: Tin Dang
CHANGELOG entry covering the P0-P3 graph deep-review implementation (freeze-boundary correctness, point-query indexes, plan-cache + executor cost, row-space BFS + TraversalGuard + HNSW bridge), per the CI Lint changelog gate. author: Tin Dang
…e_buf (P0-2) Two data-visibility bugs in graph restart recovery: 1. WAL replay discarded logged ids: ADDNODE/ADDEDGE WAL records carry the ids returned to clients (slotmap as_ffi values), but replay re-inserted entities under fresh SlotMap keys via an internal node_map. Every client-cached node/edge handle silently died at restart, and REMOVEEDGE records could only resolve via the same in-memory map. 2. Replay wrote into segments.mutable, which NO command handler reads — every handler consults NamedGraph::write_buf. Replayed unfrozen graph data was invisible after restart until the next freeze. Fix: - MemGraph storage: SlotMap → FxHashMap keyed by monotonically allocated ids in slotmap ffi encoding (odd version word, index rollover into version+2), plus insertion-order side vectors for deterministic iteration/freeze. Slot maps cannot insert at a chosen (index, generation) pair, which is exactly what replay needs. - add_node_with_id / add_edge_across_tiers_with_id re-materialize entities under the ORIGINAL logged ids and bump the allocation floor past them; monotonic cursors are never reset at thaw (replaces the old slotmap-generation anti-aliasing argument). - replay.rs: take/put_memgraph now operate on write_buf (mem::replace); CSR seed loop floors node-id allocation past frozen external_ids. - recovery.rs/manifest.rs: GraphManifest persists (next_node_id, next_edge_id) cursors (#[serde(default)] — pre-cursor manifests read as 0/unknown); recovery restores cursors AND floors from frozen external_ids, so post-restart inserts can never alias frozen rows even with a truncated WAL. TDD: 3 red replay tests (ids preserved in write_buf, REMOVEEDGE by original id, no post-replay aliasing) + recovery floor test with pre-cursor-manifest fallback. lib 3825 green, graph_freeze_boundary 11/11, tokio+graph 473/473, clippy -D warnings both feature sets. author: Tin Dang
The Cypher write path matched against the MUTABLE tier only: after a freeze, MATCH..SET silently set nothing, MATCH..DELETE deleted nothing, and MERGE duplicated every frozen node (its match scan couldn't see it). Cypher DELETEs also emitted no WAL record — even mutable-tier deletes resurrected at restart. Copy-up design (enabled by W2-1's stable-id storage — the write buffer can now insert at a chosen key): - NamedGraph::copy_up_node / copy_up_into: materialize the newest frozen row (labels, props via v5 blob, embedding, MVCC stamps) into write_buf under the SAME NodeKey. The mutable tier is authoritative everywhere, so mutating the shadow updates reads, and soft-deleting it tombstones the frozen row. - Write executor now sees both tiers: NodeScan/IndexScan via MergedNodeView, Expand (1-hop + var-length BFS) via SegmentMergeReader, all eval_expr sites get the segment list (filters/projections on frozen properties). SET/DELETE/apply_set_items copy up before mutating; MERGE matches across tiers (find_node_merged) and creates edges across tiers. - Read-side shadow correctness: MergedNodeView scans and index_scan_keys skip segment rows shadowed in the mutable tier + dedup keys across segments (a re-frozen live shadow leaves a stale row in the older segment); SegmentMergeReader hides tombstoned sources and targets. - segment_row/copy_up iterated the list with .rev() assuming newest-last; the list is newest-FIRST (add_immutable inserts at 0) — stale rows won point lookups once overlap became possible. Fixed to forward iteration. - freeze_and_compact retains tombstones across refreeze (freeze drops dead nodes → would resurrect the frozen row): dead shadows are collected pre-freeze and re-materialized at their original deleted_lsn. - Durability: Cypher DeleteNode/DeleteEdge mutations now WAL REMOVENODE/REMOVEEDGE records (both query paths); replay materializes a tombstone shadow when a REMOVENODE targets a CSR-resident node. Known gaps (documented in code): SET has no WAL record format yet (restart replays pre-SET state — pre-existing, now explicit); frozen EDGES carry a placeholder EdgeKey, so SET/DELETE on a frozen edge remains a no-op (nodes are the copy-up unit). TDD: 7 red tests in tests/graph_freeze_boundary.rs (SET prop/label, DELETE hides from scan+traversal, tombstone survives refreeze, MERGE no-duplicate, DELETE WALs REMOVENODE, re-frozen shadow no-duplicate + newest-wins) + replay frozen-REMOVENODE tombstone unit test. Suite 19 green, lib 3826, tokio+graph parity green, clippy -D warnings both. author: Tin Dang
…tree (W2-3) `WHERE n.age > 30` compiled to a full label scan + row-by-row Filter, even though every immutable segment already carries a per-property numeric B-tree (SegmentPropertyIndexes). Ranges now prune at the index. Planner: extract_range_conjuncts walks WHERE's AND-tree (OR trees are never extracted); each `n.p <cmp> value` conjunct with a numeric-literal or parameter threshold (auto-parameterization rewrites literals to Parameter before compile) upgrades the pattern's NodeScan to an IndexScan or extends an existing one via the new `prop_range: Vec<(String, RangeCmp, Expr)>` field. `10 > n.b` flips to `n.b < 10`. The residual Filter is ALWAYS kept — pruning is superset-only. Executor: index_scan_keys resolves each conjunct per run (plan-cache safe, like prop_eq); frozen tier ANDs `PropertyIndex` gt/gte/lt/new-lte bitmaps into the per-segment accumulator (no numeric tree for the prop => empty — the build is exhaustive), mutable tail applies the same comparison with the index's Int/Float/Bool-as-0-1 normalization. A threshold resolving non-numeric (e.g. String parameter) drops that conjunct rather than mis-pruning; all conjuncts unresolvable falls back to the plain merged label scan. Soundness required a semantics fix: the ordering operators < > <= >= evaluated via compare_values' TOTAL type-rank order (Null=0 < Bool < numbers < String), so `"zzz" > 5` and missing-prop `< 5` were TRUE — rows a numeric index legitimately excludes. eval_binary_op now follows openCypher: only same-kind operands compare (Int/Float promote; String/ String; Bool/Bool), cross-type/Null/NaN yield Null and are dropped by WHERE — exactly the rows the index excludes. compare_values keeps its total order for ORDER BY and equality. TDD: 3 red planner tests (scan upgrade, AND-composition + operand flip, OR non-extraction), red e2e range query across mutable+frozen tiers + string-typed prop exclusion in tests/graph_freeze_boundary.rs (suite 19), PropertyIndex::lte unit coverage. Lib 3829 green, tokio+graph parity green, clippy -D warnings both runtimes, fmt clean. author: Tin Dang
…(W2-4)
Every string cell in a Cypher result paid two allocation+copies: the
stored property (`PropertyValue::String(Bytes)`) was copied into a
`String` at property access (property_value_to_value), then copied AGAIN
into `Bytes` at reply serialization (value_to_frame). The variant now
holds `Bytes`, so a stored property flows to the RESP frame as two
refcount bumps and zero copies.
Correctness side-effect (red-first test): property_value_to_value
lossily degraded non-UTF8 payloads to "" via `from_utf8().unwrap_or("")`
— RESP bulk strings are arbitrary bytes, so GRAPH.ADDNODE could store a
binary property that RETURN silently erased. Bytes round-trips it
verbatim (RESP is binary-safe end to end).
Text-shaped ops validate UTF-8 at their own boundary and yield Null on
binary input (toInteger/toFloat); `=~` and its contains/prefix/suffix
fallback matcher operate on raw bytes; ordering/equality compare
bytewise (identical to UTF-8 String order); toString/display go through
from_utf8_lossy. StringLit/parameter/JSON construction sites copy once
into Bytes (same cost as the old String clone).
TDD: red e2e binary_property_roundtrips_through_return (returned [] for
[0xff,0xfe,0x21] before, verbatim after; suite 20). Lib 3829 green, all
7 graph integration suites green under tokio+graph (52 tests), clippy
-D warnings both runtimes, fmt clean.
author: Tin Dang
…lk_rerank) Pure move ahead of W2-5: hybrid.rs was at 1479 lines, about to breach the 1500-line module rule. mod.rs keeps types/strategy/helpers/tests; search.rs holds HYB-01+HYB-02; walk_rerank.rs holds HYB-03+HYB-04. Public API unchanged (pub use re-exports). Hybrid suite 39 green. author: Tin Dang
…(W2-5) Wave 1 wired the per-segment HNSW bridge into HYB-01 only; HYB-02 (vector-to-graph expansion) and HYB-04 (graph-constrained rerank) still brute-forced every embedding, and the lazy bridge build itself ran synchronously INSIDE the first query — stalling the shard event loop for the full HNSW construction on a large segment. - Off-thread build: CsrStorage::hnsw_bridge() no longer blocks. The first caller CAS-arms `hnsw_building` and spawns a detached build thread; every caller treats "not ready" as `None` (= exact scoring), so queries brute-force until the bridge installs. A panicking build converges the OnceLock to None (fail-safe: segment stays exact-scored forever, never wedged). Test hook stays synchronous. - HYB-02 gains the strategy selector + hnsw_prefilter_score routing: at >= threshold candidates, frozen rows answer via their segment bridge (exact cosines), mutable/bridge-less rows stay exact, under-filled beams rescue to exact. - HYB-04's combined score decomposes: every node OUTSIDE the BFS frontier shares the penalty graph term, so within that class the combined ranking is monotone in cosine. The bridged path scores frontier + mutable tier + bridge-uncovered rows exactly (all bounded) and takes the segment bridge's top-k for the rest — replacing the all-nodes scan. Row admissibility mirrors the W2-2 merged-view rules (MVCC-visible, copy-up shadow wins, newest segment copy wins). - GraphHnsw now records `uncovered_embedded_rows` (dim mismatch / degenerate norm) at zero extra build cost — what makes the HYB-04 whole-segment bridge scan exactly correct without a row scan. - hybrid.rs (1479 lines, about to breach the 1500 rule) split into a directory module in the preceding commit. Tests: bridge uncovered-partition + off-thread install (first call must return immediately); HYB-02 and HYB-04 bridge-vs-brute parity (exact cosines / exact combined formula, >= 4/5 and 7/8 top-k overlap, mutable-tier node parity). Lib 3833 green, freeze suite 20, tokio+graph parity green, clippy -D warnings both runtimes, fmt clean. author: Tin Dang
The CSR row-space BFS fast path gated on EXACTLY one segment — a graph that froze more than once (or hadn't compacted yet) fell all the way back to the NodeKey-hashing SegmentMergeReader path even with an empty mutable tier. Key insight making the widening cheap: cross-segment edges CANNOT exist in the gated state — freeze() retains any edge touching an earlier-frozen endpoint as a mutable-tier delta edge, and the gate requires the mutable tier empty. The only cross-segment coupling left is a NodeKey resident in several segments (a re-frozen W2-2 copy-up shadow leaves a stale row in the older segment), whose adjacency the reader unions across segments. multi_row_bfs: per-segment dense visited bitmaps + slice adjacency (push-only, sequential — the parallel levels and direction-optimizing pull stay single-segment), with a key-level boundary sync: on first emission a node's resident rows in EVERY segment are marked and enqueued in their own segment's frontier, so all copies' edges expand. An ffi-keyed emitted set is the authoritative once-per-node dedup; bitmaps stay the per-edge-probe fast reject. Gate falls back if ANY segment postdates the snapshot (the reader handles per-segment skips). Tests: two-identical-segments parity vs single-segment path; disjoint two-freeze clusters vs reader oracle (both starts); overlapping stale-copy fixture (X->B frozen old, copy-up X->C frozen new) reaches BOTH neighborhoods and matches the reader in all three directions. Lib 3835 green, tokio parity, clippy -D warnings both, fmt. author: Tin Dang
…+compile (W2-7)
The plan cache held READ-ONLY plans only ("a cache hit skips
classification, so a cached write plan would mis-route"), so every
CREATE/MERGE/SET/DELETE re-parsed and re-compiled its Cypher text —
the dominant fixed cost of small repeated writes, the most common
write shape once literals are auto-parameterized.
- PlanCache entries carry a `read_only` flag; the invariant moves from
"only read plans inserted" to "hits route on the flag". get() returns
(plan, read_only), insert() takes the flag.
- graph_query_or_write: a read-only hit routes to the read path (as
before); a WRITE hit now routes to the write path — both with zero
parse/compile. The write tail (decay validation before side effects,
LSN, execute_mut with auto-params merged, mutation → WAL/intent/undo
fan-out) is factored into execute_write_plan, shared by the hit and
compile paths, so cache hits produce identical WAL records and txn
rollback intents.
- graph_query_write (the &mut dispatch entry) gains the same literal
normalization + cache lookup/insert; its executor now merges
auto-extracted literal params (it previously compiled raw text).
- Pure read handlers (GRAPH.QUERY read path / RO_QUERY) treat a write
hit as a miss — RO_QUERY still rejects with its classification error.
TDD: e2e write_plans_cached_and_resolve_fresh_literals (CREATE {id:7}
then {id:8} — one cache entry, BOTH rows exist, id=8 resolves from the
cached plan) + or_write cache-hit still emits WAL records and rollback
intents; PlanCache flag round-trip unit test. Two existing tests
asserting "writes must not be cached" updated to the new invariant
(one write + one read plan). Lib 3836 green, freeze suite 22, tokio
parity, clippy -D warnings both, fmt.
author: Tin Dang
…raph-timeout-ms (W2-8) The traversal guard's 30s budget was a hardcoded constant: no way to tighten it for latency-sensitive deployments, loosen it for analytic queries, or override it per query. RedisGraph exposes both knobs. - `--graph-timeout-ms <ms>`: process-wide default read by TraversalGuard::with_default_timeout (AtomicU64, set in main before shard threads spawn). 0 = unlimited. Default stays 30_000. - Per-query `TIMEOUT <ms>` argument on GRAPH.QUERY / GRAPH.RO_QUERY / GRAPH.PROFILE / GRAPH.TRAVERSE (RedisGraph parity; TIMEOUT 0 = unlimited for that query). Strict validation like --decay: dangling keyword or non-integer value is an error, not a silent no-op. - All three production guard sites (traverse hop loop, run_read_query, profile) now route through one query_guard helper: per-query override first, else the configured default. Write execution (execute_mut) has no guard plumbing — timeouts remain a read-traversal concern. TDD (red-first): e2e all-pairs shortestPath over a 400-node chain (160k per-row guard checks ⇒ deterministically >1ms of work) aborts under TIMEOUT 1 with "traversal timeout", completes under TIMEOUT 0 and the default; garbage/dangling TIMEOUT rejected; parse + guard round-trip unit tests. Lib 3837 green, freeze suite 25, tokio parity, clippy -D warnings both runtimes, fmt. author: Tin Dang
…(W2-9)
New integration suite tests/crash_recovery_graph_durability.rs (model:
the vector engine's B4 suite): real server processes, SIGKILL at a
deterministic point (poll the on-disk shard WAL for a marker byte-string
— kill -9 preserves the page cache, so only the 1ms flush tick matters),
bounded condition-based waits throughout, MOON_BIN pinned, fresh --dir.
G1 mutable-tier replay: ADDNODE/ADDEDGE + Cypher CREATE/SET/DELETE
survive kill -9; pre-crash external handles still resolve (W2-1
e2e) and post-recovery inserts never alias replayed ids.
G2 freeze-threshold crossing: 300 nodes + 64_100 pipelined edges froze
a CSR segment pre-crash; kill -9 never persists segments (that runs
only on graceful shutdown), so replay re-executes all inserts,
re-freezes, and every answer matches pre-crash exactly.
G3 double-crash idempotence: recover → crash with no new writes →
recover; identical answers and the WAL must not grow (catches
replay-time re-append/double-apply).
The suite immediately caught a REAL loss: Cypher SET had NO WAL record
(graph_query_write even documented it: "SET has no WAL record format
yet") and `SET n:Label` produced no mutation record at all — both
silently reverted on kill -9 to the original ADDNODE state.
Fix, red→green against G1/G3:
- MutationRecord::SetProperty gains `new_value`; new SetLabel variant
(WAL-only: label rollback was never captured — pre-existing Phase 174
scope). ON CREATE SET paths now pass the mutations vec too (the
CreateNode WAL snapshot predates the SET; the extra RestoreProperty
undo is a no-op on a node the CreateNode intent removes entirely).
- New WAL records GRAPH.SETPROP / GRAPH.SETLABEL (src/graph/wal.rs),
emitted from BOTH write fan-outs (graph_query_write + the shared
execute_write_plan).
- Replay: SETs bucketed per epoch in WAL order, applied after inserts
and before removes; a frozen target copies up first (W2-2 parity).
Last-write-wins and label idempotence unit-tested; frozen copy-up
unit-tested.
Lib 3841 green, freeze suite 25 + segment merge, crash suite 3/3, tokio
parity incl. the Phase 174 rollback adversarial suite, clippy -D
warnings both runtimes, fmt.
author: Tin Dang
benches/graph_traversal.rs only built mutable-only graphs, so the CSR row-space fast path (row_bfs) — the whole point of the frozen tier — was invisible to benchmarking. New `frozen_bfs` group freezes the SAME 10K/50 fixture into four directly comparable shapes: - row_bfs_frozen_single: one CSR segment (parallel levels + Beamer) - row_bfs_frozen_multi2: the graph frozen twice into identical clone segments (the W2-2 copy-up aftermath shape) — W2-6 multi-segment row path with worst-case key-level boundary sync - reader_bfs_mixed_tier: same segment + non-empty mutable tail (gate declines → SegmentMergeReader fallback — what row-BFS saves) - reader_bfs_frozen_multi2: reader over the SAME two segments (the only fair multi-segment pairing; multi2 doubles edge probes vs single) Correctness prechecks assert all shapes visit the identical node set before timing. Dev-machine directional numbers (quiet run): single row-BFS ~1.4ms vs ~5.9ms reader fallback (~4x); multi2 row path ~1.55ms vs ~2.89ms reader on the same segments (~1.9x). Absolute numbers belong to the Linux VM/GCE per repo policy. author: Tin Dang
…nt single-shard model (W2-11) The scatter-gather cross-shard traversal line (src/graph/cross_shard.rs: handle_graph_traverse + parse_traverse_response, ShardMessage:: GraphTraverse + GraphTraversePayload, two spsc_handler arms) never had a sender — no code ever constructed the SPSC message and the coordinator the module doc described was never built (2026-07 graph deep review). Receive-side handlers for a message nobody sends are pure audit surface. Deleted the module, its re-exports (all dead: graph_has_hash_tag had no callers either), the enum variant, the boxed payload, and both handler arms. Replaced by a sharding-model section in src/graph/mod.rs: a named graph lives entirely on one shard (routing hashes the graph name), hash tags co-locate graphs, and partitioning happens BY graph — there is no within-graph cross-shard traversal. Lib 3832 green (9 removed tests were the dead module's own), clippy -D warnings both runtimes, fmt. author: Tin Dang
… implicit grouping (W2-12)
count()/collect() were documented no-ops in eval ("handled in the
Project phase as a future enhancement") — RETURN count(n) just echoed
the per-row value. The Project operator now detects top-level aggregate
calls and switches into grouped aggregation (openCypher semantics):
- Implicit grouping: non-aggregate RETURN items form the group key;
each group emits one row (first-seen order; ORDER BY still applies).
- count(expr) counts non-null values, count(*) counts rows; sum stays
Int for all-Int inputs (0 when empty) and promotes to Float on mixed;
avg is Float (Null when no numeric input); min/max use the executor's
total value order (Null when empty); collect returns a Value::List
(already RESP-encodable). DISTINCT inside a call (parser support
pre-existed) dedups inputs before finalizing.
- Zero rows: global aggregate emits exactly ONE row (count=0, sum=0,
collect=[]); a grouped aggregate emits none.
- Shared try_project_aggregate helper wired into BOTH executor loops
(execute + execute_profile); non-aggregate projections are untouched.
Aggregates nested inside larger expressions (count(n) + 1) are out of
scope and keep the pre-existing per-row behavior (noted in code).
TDD: 5 red-first e2e tests (count(*)/count(n), grouped count+sum with
ORDER BY, sum/avg/min/max, collect(DISTINCT), zero-row global vs
grouped). Lib 3832 green, freeze suite 30, tokio parity (47 e2e),
clippy -D warnings both runtimes, fmt.
author: Tin Dang
…g (W2-13)
Two silent-wrong-results gaps in the Cypher pipeline, both fixed loudly:
OPTIONAL MATCH previously compiled identically to inner MATCH (the parser
set `optional: true`, the planner never read it) — rows without a match
were silently dropped. Now `compile_optional_match` emits
`Expand { optional: true }` for the dominant shape (single relationship
expanding from a previously bound bare variable): a source row whose
expansion yields zero matches — or whose source is Null from an earlier
optional hop — survives with the target and edge variables bound to Null
(`push_null_padded`, shared by the execute / execute_profile / write
loops). Unsupported shapes are rejected with explicit PlanError messages
instead of behaving like inner MATCH: standalone patterns (first variable
unbound), multi-relationship chains (whole-pattern null semantics need
grouped execution), inline properties on the optional target (a
post-filter would drop null-padded rows), and labels/properties on the
bound first node. The planner now tracks bound variables across clauses
(MATCH/CREATE/MERGE/UNWIND/shortestPath bind; WITH resets scope to its
outputs) to validate the bound-variable requirement.
WITH previously compiled to the same pipeline-terminating Project as
RETURN — the executor set `projected_rows` and cleared `rows`, so every
clause after WITH ran on an empty row stream (zero results for any
WITH...RETURN query). `PhysicalOp::Project` gains `rebind: bool` (true
for WITH): SlotTable::from_plan binds the projection's output names
(alias or expression text) and the executor re-seeds the variable-binding
row stream from the projected values instead of terminating, so
WHERE (HAVING over aggregates), ORDER BY/SKIP/LIMIT, and RETURN keep
executing — W2-12 grouped aggregation composes (`WITH n.city AS city,
sum(n.score) AS total WHERE total > 30`). Guards: `WITH *` is rejected
loudly (was silently wrong), and W2-3 range-conjunct extraction stops
after the first WITH — a post-WITH WHERE is a HAVING and must never
prune pre-aggregation scans.
Tests: 8 new e2e tests (tests/graph_freeze_boundary.rs section 12,
red-first) covering null-padding with ORDER BY, edge-variable Null,
the four rejected optional shapes, aggregate+HAVING, entity passthrough
(`WITH n AS m`), ORDER BY/LIMIT between WITH and RETURN, WITH DISTINCT,
and the WITH * rejection. Full graph suite green on both runtimes
(monoio 3832 lib + 38 freeze-boundary; tokio 3669 incl. graph_integration
and graph_cypher_inline_filter); clippy -D warnings clean on both.
author: Tin Dang
…ig-literal fix - CHANGELOG: add the wave-2 `[Unreleased]` entry (durability, Cypher coverage, copy-up writes, query performance, operability, dead-code deletion) after the wave-1 graph entry. - tests/mq_integration.rs + tests/workspace_integration.rs (2 sites): add the new `graph_timeout_ms` field (W2-8) to their hand-built ServerConfig literals — the only tests constructing ServerConfig without Default; broke the tokio+jemalloc (no-graph) CI leg at compile time. author: Tin Dang
…ave 2 Reconciles the wave-2 stack with main's squash-merge of graph wave 1 (c950806), which landed pre-merge review fixes absent from the local wave-1 base. Key reconciliation decisions: - Restart NodeKey aliasing (P0): wave-2's monotonic id cursors + ensure_node_id_floor + add_node_with_id SUBSUME main's SlotMap id_offset watermark — main's with_id_offset/to_public_key/ to_internal_key machinery removed; main's CSR-dedup skip in the AddNode replay loop retained (full-history replay of a frozen node keeps the CSR identity instead of shadowing it). - recover_graph_store: restores manifest id cursors and raises the allocation floor past every loaded segment's external_id. - PlanCache: fused main's raw+normalized dual-key design (CacheEntry/ CachedPlan, FxHashMap, raw-hash pre-lookup skipping parameterize) with W2-7's write-plan caching via a read_only flag; every read-path hit now checks the flag, graph_query_or_write routes hits by it. - tests/graph_restart_id_aliasing.rs: probes the wave-2 tier contract (NamedGraph::write_buf is the live mutable tier) and its docs reference the cursor/floor mechanism instead of with_id_offset. - W2-7 cache-count tests updated for dual-key entries (2 per shape). - graph_timeout_ms (W2-8) added to main-side hand-built ServerConfig literals (mq_integration, workspace_integration, txn_kv_wiring). Verified: full default suite 4261 passed; tokio parity 3612 passed; tokio+graph integration 62 passed; graph battery 3916 passed; kill-9 graph crash suite 3/3; clippy clean on both feature configs; fmt clean. author: Tin Dang
…ass + checkpoint graph snapshot (P0)
Found by the wave-2 GCP production-hardening soak: a 10-minute mixed
Cypher workload (98,435 nodes, zero errors) vanished entirely on
kill -9 + restart ("ERR graph not found") under the DEFAULT server
configuration. Two stacked pre-existing bugs:
Bug A — graph WAL replay was a no-op in v3 mode. Graph records land in
`shard-<N>/wal-v3/` (via the wal_append channel), but
`Shard::restore_from_persistence` replays v3 through a THROWAWAY
DispatchReplayEngine whose collected graph commands are dropped, and
`replay_graph_wal` only reads the legacy `shard-<N>.wal` v2 file. The
existing crash suite passes `--disk-offload disable`, so it never saw
the default path.
Fix: `shared_databases::replay_graph_wal_v3` — a dedicated boot pass
that scans the v3 WAL dir, collects GRAPH.* records into a
GraphReplayCollector, and applies them to the slice's graph store
(wired in main.rs + embedded.rs after recover_graph_stores).
Bug B — the checkpoint erased graph durability. Checkpoint finalize
advances control.last_checkpoint_lsn (the WAL replay floor) and
recycles WAL segments, but `save_graph_store` ran only on graceful
shutdown and never persisted the mutable write_buf tier (freeze is the
only path to disk). Any crash after the first checkpoint (the soak
crossed 17) lost every covered graph record permanently.
Fix: checkpoint finalize now invokes a graph snapshot hook BEFORE the
control-file update — freezes each graph's write buffer into a CSR
segment, persists segments + manifests + metadata, and stamps a
`snapshot_lsn` (= current_lsn - 1; current_lsn is next-to-assign)
replay floor in graph_metadata.json (serde-default 0 keeps old dirs
loadable). A failed snapshot ABORTS the finalize: the old floor stays
in force and the WAL segments holding the records are not recycled.
Gated on a GraphStore dirty flag (set by drain_wal / replay_into) so
KV-only workloads never pay for graph saves. force_checkpoint's
drive-to-completion loop is now bounded (100k ticks) instead of
spinning forever on a persistently failing finalize.
Red/green: new ignored crash tests G4 (default v3 config, kill -9, no
checkpoint — catches Bug A) and G5 (BGSAVE-forced checkpoint between
writes, kill -9 — catches Bug B; also caught the current_lsn
off-by-one on its first green run). Full suite: crash suite 5/5,
default 4261, tokio parity green, clippy clean both configs.
author: Tin Dang
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
Warning Review limit reached
Next review available in: 43 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. 📝 WalkthroughWalkthroughThis PR expands graph durability, replay, Cypher execution, traversal, caches, hybrid search, and benchmarks. It also removes the cross-shard traversal path and adds crash-recovery and query-semantic coverage. ChangesWAL v3 durability and crash recovery
Stable node and edge IDs
Cypher execution, planning, and caches
HNSW bridge and hybrid search
Multi-segment BFS and traversal
Cross-shard removal
Documentation and benchmarks
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant graph_query_or_write
participant PlanCache
participant run_read_query
participant execute_write_plan
Client->>graph_query_or_write: GRAPH.QUERY
graph_query_or_write->>PlanCache: lookup
PlanCache-->>graph_query_or_write: CachedPlan{read_only}
alt read_only
graph_query_or_write->>run_read_query: execute
else write
graph_query_or_write->>execute_write_plan: execute
end
sequenceDiagram
participant EventLoop
participant persistence_tick
participant GraphStore
EventLoop->>persistence_tick: checkpoint tick
persistence_tick->>GraphStore: persist_graph_at_checkpoint(lsn)
GraphStore-->>persistence_tick: ok/fail
Note over EventLoop,GraphStore: restart later replays graph WAL v3 above snapshot_lsn
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Add the Linux criterion results for the wave-2 graph engine (PR #237): frozen-tier CSR 1-hop at 1.07 ns (~106× over the mutable memgraph tier), row-BFS frozen 10K depth-3 at 658 µs (~14× over sequential memgraph BFS), command-dispatch costs (ADDNODE 223 ns / ADDEDGE 204 ns / NEIGHBORS 429 ns), the 19.5 ms 64K-edge CSR freeze that bounds the checkpoint graph-snapshot cost from the P0 durability fix, and NEON cosine kernels. Documents the ParallelBfs finding: its memgraph parallel path is slower than sequential (11.8 ms vs 9.44 ms, +25%) by construction — neighbor collection stays sequential behind a !Send reader, so it parallelizes only visited-set dedup while paying neighbor-list clones, a per-level DashSet rebuild, and ~76 raw thread spawns per level. It has zero production callers (query path uses BoundedBfs + the row-BFS frozen fast path); flagged as a retire/fold cleanup candidate. author: Tin Dang
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/graph/cypher/executor/write.rs (1)
891-899: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winON CREATE SET is dropped on the missing-node edge-MERGE path — pass
Some(&mut mutations).This is the only edge-MERGE create path that passes
Nonefor mutations. The two other create paths (Lines 692-703 and 780-789) passSome(&mut mutations)with the W2-9 rationale that the precedingCreateNodeWAL snapshot predates the SET, so without aSetProperty/SetLabelrecord the ON CREATE SET values are silently lost onkill -9replay and cannot be rolled back byTXN.ABORT. This branch (which creates the missing src/dst nodes and the edge) has the same exposure.🐛 Proposed fix
apply_set_items( on_create, &new_row, graph, &csr_segs, params, &mut properties_set, - None, + // W2-9: see apply_set_items doc — ON CREATE + // SET must WAL/rollback here too. + Some(&mut mutations), );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/graph/cypher/executor/write.rs` around lines 891 - 899, The missing-node edge-MERGE create path in apply_set_items is currently dropping ON CREATE SET mutations because it passes None for mutations. Update this branch to pass Some(&mut mutations), matching the other create paths in write.rs, so the SetProperty/SetLabel records are captured for the newly created src/dst nodes and edge and can survive replay/rollback correctly.src/command/graph/graph_read.rs (1)
592-604: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winPlan-cache mutex held for the whole query execution (if-let scrutinee temporary lifetime extension).
graph.plan_cache.lock().get(raw_hash)used directly as theif letscrutinee keeps theMutexGuardalive for the entire block body — including thereturn run_read_query(...)call — because Rust extends scrutinee temporaries to the whole match/if-let arm even thoughget()returns an ownedCachedPlan, not a borrow. This is the classic "mutex held across match/if-let" footgun. The very next lookup a few lines below (line 612) avoids this correctly by binding to aletfirst.🔒 Proposed fix — bind the lock result before matching
- if let Some(cached) = graph.plan_cache.lock().get(raw_hash) { - // W2-7 caches WRITE plans too — a write hit falls through to the - // parse path, which reports it exactly like an uncached write query. - if cached.read_only { - let auto_params = cached.auto_params.as_ref().clone(); - return run_read_query(graph, args, &cached.plan, &cached.slots, auto_params); - } - } + let raw_cached = graph.plan_cache.lock().get(raw_hash); + if let Some(cached) = raw_cached { + // W2-7 caches WRITE plans too — a write hit falls through to the + // parse path, which reports it exactly like an uncached write query. + if cached.read_only { + let auto_params = cached.auto_params.as_ref().clone(); + return run_read_query(graph, args, &cached.plan, &cached.slots, auto_params); + } + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/command/graph/graph_read.rs` around lines 592 - 604, The plan-cache mutex is being held across the whole if-let arm because graph.plan_cache.lock().get(raw_hash) is used directly as the scrutinee in graph_read’s read-path lookup. Bind the lock result to a local first, then call get(raw_hash) on that bound guard and match on the result, so the MutexGuard is dropped before run_read_query is invoked. Use the existing raw-hash pre-lookup block in graph_read and mirror the safer pattern already used later in the function for the next lookup.
🧹 Nitpick comments (1)
src/graph/recovery.rs (1)
280-286: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoffFreezing every dirty write buffer on each checkpoint can proliferate tiny CSR segments.
Under steady write load with frequent checkpoints, each finalize freezes the mutable tier into a fresh immutable segment, growing segment count until compaction catches up. Worth confirming compaction/merge cadence keeps segment count bounded so read-path fan-out and recovery time don't degrade.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/graph/recovery.rs` around lines 280 - 286, The checkpoint path in recovery is freezing every dirty write buffer via freeze_and_compact for each graph, which can create too many tiny immutable CSR segments under steady writes. Review the checkpoint/finalize flow around store.list_graphs, store.allocate_lsn, and graph.freeze_and_compact, and adjust it so compaction or merge happens often enough to keep segment count bounded. If needed, batch or defer freezing behavior in this path and ensure there is an explicit merge cadence or threshold that prevents segment proliferation while preserving recovery correctness.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/command/graph/graph_read.rs`:
- Around line 80-119: The TIMEOUT parser is scanning the full argv slice, so
positional values like the graph name or query text can be mistaken for the
TIMEOUT keyword. Update `parse_timeout_ms`/`query_guard` callers to only inspect
the trailing option arguments, and pass an offset or sliced view from
`graph_neighbors`, `run_read_query`, `graph_profile`, and the
`graph_query_readonly`/`graph_query_write`/`graph_query_or_write` path before
calling `query_guard`. This keeps bareword `TIMEOUT` from colliding with
`args[0]`/`args[1]` while preserving the intended override behavior.
---
Outside diff comments:
In `@src/command/graph/graph_read.rs`:
- Around line 592-604: The plan-cache mutex is being held across the whole
if-let arm because graph.plan_cache.lock().get(raw_hash) is used directly as the
scrutinee in graph_read’s read-path lookup. Bind the lock result to a local
first, then call get(raw_hash) on that bound guard and match on the result, so
the MutexGuard is dropped before run_read_query is invoked. Use the existing
raw-hash pre-lookup block in graph_read and mirror the safer pattern already
used later in the function for the next lookup.
In `@src/graph/cypher/executor/write.rs`:
- Around line 891-899: The missing-node edge-MERGE create path in
apply_set_items is currently dropping ON CREATE SET mutations because it passes
None for mutations. Update this branch to pass Some(&mut mutations), matching
the other create paths in write.rs, so the SetProperty/SetLabel records are
captured for the newly created src/dst nodes and edge and can survive
replay/rollback correctly.
---
Nitpick comments:
In `@src/graph/recovery.rs`:
- Around line 280-286: The checkpoint path in recovery is freezing every dirty
write buffer via freeze_and_compact for each graph, which can create too many
tiny immutable CSR segments under steady writes. Review the checkpoint/finalize
flow around store.list_graphs, store.allocate_lsn, and graph.freeze_and_compact,
and adjust it so compaction or merge happens often enough to keep segment count
bounded. If needed, batch or defer freezing behavior in this path and ensure
there is an explicit merge cadence or threshold that prevents segment
proliferation while preserving recovery correctness.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1c6ff679-53f0-45ce-8e44-e5c659172185
📒 Files selected for processing (46)
CHANGELOG.mdbenches/graph_traversal.rssrc/command/graph/graph_read.rssrc/command/graph/mod.rssrc/config.rssrc/graph/compaction.rssrc/graph/cross_shard.rssrc/graph/csr/mmap.rssrc/graph/csr/mod.rssrc/graph/csr/storage.rssrc/graph/cypher/executor/eval.rssrc/graph/cypher/executor/mod.rssrc/graph/cypher/executor/read.rssrc/graph/cypher/executor/write.rssrc/graph/cypher/parameterize.rssrc/graph/cypher/planner.rssrc/graph/hnsw_bridge.rssrc/graph/hybrid/mod.rssrc/graph/hybrid/search.rssrc/graph/hybrid/walk_rerank.rssrc/graph/index.rssrc/graph/manifest.rssrc/graph/memgraph.rssrc/graph/mod.rssrc/graph/recovery.rssrc/graph/replay.rssrc/graph/row_bfs.rssrc/graph/store.rssrc/graph/traversal.rssrc/graph/traversal_guard.rssrc/graph/view.rssrc/graph/wal.rssrc/main.rssrc/server/embedded.rssrc/shard/dispatch.rssrc/shard/event_loop.rssrc/shard/mod.rssrc/shard/persistence_tick.rssrc/shard/shared_databases.rssrc/shard/spsc_handler.rstests/crash_recovery_graph_durability.rstests/graph_freeze_boundary.rstests/graph_restart_id_aliasing.rstests/mq_integration.rstests/txn_kv_wiring.rstests/workspace_integration.rs
💤 Files with no reviewable changes (2)
- src/graph/cross_shard.rs
- src/shard/spsc_handler.rs
2026-07-07 run of scripts/bench-graph-compare.sh --nodes 5000 on the OrbStack Linux VM (Docker FalkorDB), wave-2 engine: Moon leads every row — node insert 1.2×, edge insert 1.7×, 1-hop 1.3×, 2-hop 1.3×, Cypher pattern match 1.1×. Caveated as a single-client latency comparison (sequential redis-cli, fork-bound ~1 ms/op on both sides), not server throughput; §11.5's 8-thread GCloud run stays the throughput reference, and re-running that harness post-wave-2 is flagged as the follow-up before claiming the Cypher point-query gap vs FalkorDB's property index is closed. author: Tin Dang
…p vs FalkorDB (§11.8) Re-ran the §11.5 concurrent harness (graph phase of gce-4feature-bench.sh: 5K nodes / 15K edges, redis-py 8 threads) with the wave-2 engine and a same-box Docker FalkorDB on GCE. Same-box ratios: cypher_1hop 0.90× (was 0.25× in June), cypher_2hop 1.22× — Moon now WINS 2-hop at better p50 (0.89 ms vs 2.11 ms) — build 19×, match_rows parity. The June "property index vs filtered label scan" deficit is gone at this scale (write-side plan cache + IndexScan + executor work). Caveats recorded: instance is e2-standard-16 (c2d zone capacity exhausted), so cross-run absolutes are not comparable; Moon p99 trails FalkorDB on shared cores (pinned-core follow-up noted); Moon Cypher 1-hop now outruns its own native GRAPH.NEIGHBORS under concurrency (plan cache amortizes parsing). author: Tin Dang
#31) Profiling the 8-thread GCE Cypher harness (5K nodes/15K edges) showed 82.5% of shard CPU inside index_scan_keys's mutable-tail loop: MATCH (a:N {id:X}) already planned as PhysicalOp::IndexScan, but the mutable write buffer had no property index, so every point query degraded to an O(live_node_count) scan of every resident node (label check, MVCC visibility, property equality) regardless of selectivity. The frozen tier already had a real index (SegmentPropertyIndexes); the mutable tier did not. Adds MutablePropertyIndex (src/graph/index.rs), mirroring SegmentPropertyIndexes's numeric-BTree/string-hash split but keyed by NodeKey and maintained incrementally on every write instead of built once at freeze time. Hooked into MemGraph's existing single-writer mutation primitives (insert_node_at, remove_node) plus three new methods — set_node_property, remove_node_property, undelete_node — that become the single source of truth for node-property mutation, replacing five previously hand-rolled find-and-replace-or-push call sites (Cypher SET, MERGE ON CREATE/MATCH SET, TXN.ABORT undo, WAL replay) that could silently let the index drift from live state. index_scan_keys's mutable-tail branch now seeds its candidate set from an index probe instead of memgraph.iter_nodes(); the residual label/MVCC/property checks are byte-for-byte unchanged (SUPERSET contract preserved, zero planner changes). Design review surfaced a real gap: TXN.ABORT's UndeleteNode undo flipped deleted_lsn back to u64::MAX without re-indexing the node's properties, which would leave a rolled-back DELETE live but permanently unreachable by index probes. MemGraph::undelete_node centralizes the flip + live-count bump + re-index and closes it; a dedicated regression test (transaction::abort::tests) fails without the fix and passes with it. Extracted normalize_numeric as a shared helper so the frozen and mutable tiers can never silently drift on what counts as numerically equal (Int/Float/Bool aliasing). Verified with red/green TDD: every new/regression test was confirmed to fail against a temporarily reverted implementation before the fix landed (index_scan_keys candidate seeding, MemGraph::undelete_node). - src/graph/index.rs: MutablePropertyIndex + normalize_numeric extraction - src/graph/memgraph.rs: prop_index field, maintenance hooks, new API - src/graph/cypher/executor/read.rs: index_scan_keys mutable-tail rewrite - src/graph/cypher/executor/write.rs: SET call sites route through set_node_property - src/transaction/abort.rs: RestoreProperty/UndeleteNode undo route through the new primitives; new regression test module - src/graph/replay.rs: WAL replay SetProp routes through set_node_property - CHANGELOG.md: [Unreleased] bullet under graph engine wave 2 Gates: cargo test --lib (default features, includes graph) 3894 passed (23 new); cargo check --no-default-features --features runtime-tokio,jemalloc,graph; cargo clippy (both configs) -- -D warnings; cargo fmt --check. Two pre-existing, unrelated failures observed in full `cargo test --release` (metrics_endpoint_emits_seven_ memory_kinds — Phase 190 Prometheus test, reproduces in isolation on this branch with no graph-related code touched; unlink_colocated_ fastpath_persists_across_restart — MOONERR diskfull on the shared /Volumes/Games volume, a documented pre-existing environment gotcha) are not caused by this change. author: Tin Dang
…ask #32) Read-only GRAPH.QUERY / GRAPH.RO_QUERY now cache the fully-encoded RESP reply bytes for repeated identical queries (same raw Cypher text + args), served on a cache hit as a zero-recompute Frame::PreSerialized passthrough instead of re-parsing, re-planning, and re-executing. Design (per tmp/DESIGN-RESULT-CACHE-FTS.md section A, implemented against current graph-wave-2 code rather than the doc's line references, which had shifted after the P1 mutable-property-index landing): - Reuses the existing Frame::PreSerialized(Bytes) variant instead of adding the design doc's proposed Frame::Raw -- both serialize()/serialize_resp3() already write it verbatim regardless of protocol version, so this needs zero protocol-layer changes. - Cache key is (query_hash, args_hash) via xxh64, reusing the planner's existing hash_query. Separate RESP2/RESP3 OnceLock slots per entry so a protocol-version switch is a clean miss, never a stale re-encode. - Invalidation is a per-graph monotonic write_gen: u64, bumped by the new NamedGraph::touch() at every real mutation site: GRAPH.ADDNODE, GRAPH.ADDEDGE (after, not before, its freeze_and_compact block -- compaction reshapes storage without changing query-visible content and must NOT invalidate), the Cypher write-plan mutation loop (both graph_query_write and execute_write_plan), TS.* temporal invalidation, and TXN.ABORT's graph undo-op rollback (RestoreProperty / UndeleteNode / UndeleteEdge / intent removal). freeze_and_compact deliberately does not touch(). - Race guard: write_gen is captured before executing a read, and the encoded reply is only stored if write_gen is unchanged after -- a miss is always safe (SUPERSET semantics), so the guard is defensive/future- proofing rather than a fix for an observed race (NamedGraph is shard-owned and single-threaded today). - Decayed queries and errored/timed-out results are never cached. - ResultCache is a bounded per-graph LRU (256 entries / 4MiB default, src/graph/cypher/result_cache.rs) with byte-budget + entry-count eviction (mirrors PlanCache's tick scheme); its resident bytes are folded into GraphStore::resident_bytes() (the props_index/hnsw_bridge memory-accounting precedent cited in the design doc). GRAPH.DELETE drops a graph's cache with it since it is a field on NamedGraph. FLUSHALL/FLUSHDB never touch graph_store today (verified, out of scope), so GRAPH.DELETE is the only clear-path needed. - The cache is only consulted where the negotiated protocol version is reliably known: dispatch_graph_read on the connection-local path (handler_monoio/write.rs, handler_sharded/write.rs), threaded as Option<u8>. The cross-shard GraphCommand hop, handler_single, and graph_query_or_write's three internal read-execute call sites all pass None and bypass the cache rather than risk keying on an unverified protocol version -- None is a type-enforced, safe-by-construction opt-out. Discovered pre-existing bug, explicitly NOT fixed (out of scope): the write-mutation loop in execute_write_plan (graph_query_or_write's write branch) never calls store.bump_version(), unlike its sibling graph_query_write. touch() was still added there for cache correctness, independent of that pre-existing gap. Testing (red/green TDD): 11 unit tests in result_cache.rs (miss/hit, write-gen mismatch, protocol isolation, stale-slot reset, LRU eviction, byte-budget eviction, oversized-single-entry non-starvation, resident-byte accounting) plus 18 integration tests in tests/graph_result_cache.rs covering the full invalidation matrix: byte-identical hit vs cold run (RESP2 + RESP3), miss on different query, invalidation by ADDNODE/ADDEDGE/ Cypher CREATE via query path/Cypher SET/Cypher DELETE/temporal invalidate/ TXN.ABORT (create-intent removal + real-undo-op RestoreProperty), no invalidation from idempotent MERGE or freeze/compact, no caching of decay queries or timeout/error results, resident-byte visibility, and GRAPH.DELETE clearing the cache. Two invalidation-site removals were temporarily reintroduced and reverted to confirm the corresponding tests actually detect regressions (not just coincidentally passing). Gates: cargo test --features graph (graph_result_cache 18/18, graph_freeze_boundary 38/38, result_cache unit 11/11, full suite green apart from the two pre-existing unrelated failures already flagged on this branch: metrics_endpoint_emits_seven_memory_kinds, unlink_colocated_fastpath_persists_across_restart); cargo check --no-default-features --features runtime-tokio,jemalloc,graph; cargo clippy -- -D warnings and cargo clippy --no-default-features --features runtime-tokio,jemalloc,graph -- -D warnings both clean; cargo fmt --check clean. author: Tin Dang
Benchmarking the result cache on the 8-thread cycling workload (5K distinct point queries, working set >> the 256-entry LRU) measured a 21% qps REGRESSION (29,014 -> 22,925): every query paid RESP serialization, entry insert, and an O(n) LRU eviction scan for a 0% hit rate — the classic LRU cycling pathology the design doc's hit-rate analysis warned about. Fix: a TinyLFU-style one-shot doorkeeper (2048-slot direct-mapped fingerprint table, 16 KiB/graph). A key is admitted to the cache only on its SECOND sighting; the admission probe runs BEFORE serialization, so one-shot/scan queries pay a single array probe and nothing else. Repeat-heavy (Zipfian) workloads still cache from the second occurrence on. Already-cached keys always admit (refresh after invalidation + second-protocol-slot population). Fingerprint collisions are benign both ways: a false admit reverts to pre-doorkeeper behavior for that key; an overwritten fingerprint delays admission by one sighting. Tests: 3 new unit tests (decline-then-admit, cached-key bypass, cycling-scan-never-admits regression guard); the 18-test invalidation matrix updated for second-sighting admission (warm-up call before each prime). 14 + 18 green; clippy both configs; fmt. author: Tin Dang
…ITH/ENDS WITH + SegmentTextIndex (task #33) Adds index-backed text predicates to Cypher by reusing Moon's existing FTS machinery, per tmp/DESIGN-RESULT-CACHE-FTS.md section B (P3 design part B). No new text engine — CONTAINS/STARTS WITH/ENDS WITH and the existing =~ operator now plan through a new per-frozen-segment SegmentTextIndex when they appear as a top-level WHERE conjunct. Grammar (src/graph/cypher/lexer.rs, ast.rs, parser/expr.rs, executor/eval.rs): first-class CONTAINS / STARTS WITH / ENDS WITH operators, pure syntax sugar over the same byte-level substring/prefix/ suffix checks `=~` already performs for its three recognized shapes — zero new evaluation logic, just three new BinaryOperator variants and lexer tokens (STARTS WITH / ENDS WITH reuse the existing WITH token). Indexing (src/graph/text_index.rs, new module): SegmentTextIndex is built lazily per immutable CSR segment via the same OnceLock + resident_bytes pattern as SegmentPropertyIndexes/hnsw_bridge (src/graph/csr/{mod,mmap,storage}.rs). CSR row (u32) is the posting doc-id directly — no id-mapping layer. Reuses crate::text::{posting::PostingStore, bm25::{FieldStats, bm25_score}, term_dict::TermDictionary} verbatim (confirmed zero coupling to TextIndex/TextStore's doc-id space or MVCC model — no file move was needed to decouple them). Correctness crux: CONTAINS/STARTS WITH/ENDS WITH are substring/prefix/ suffix predicates over the RAW value, but Moon's analyzer pipeline case-folds/normalizes/stems every token before it reaches a posting list. A token-identity lookup is UNSOUND as a pruning source here — e.g. CONTAINS 'rust' must also match "trusted" (the substring "rust" is not a token-boundary match, so a term-postings lookup for "rust" would MISS it). SegmentTextIndex therefore prunes on PRESENCE only: candidate_rows(prop_id) returns every row whose value at that property is a String/Bytes at all — always a safe superset for ANY string predicate on that property, verified by a dedicated regression test covering exactly the substring-across-token-boundary case plus case- sensitivity, empty string, and Unicode edge cases. The residual Filter (planner.rs's existing SUPERSET-candidates contract, same as prop_eq/ prop_range) remains the sole authority. Planner (src/graph/cypher/planner.rs): IndexScan gains a text_pred field; a new extract_text_conjuncts walks WHERE's top-level AND-chain and upgrades NodeScan -> IndexScan for CONTAINS/STARTS WITH/ENDS WITH/ =~ conjuncts, mirroring W2-3's extract_range_conjuncts. Executor (executor/read.rs): index_scan_keys prunes the FROZEN tier via SegmentTextIndex::candidate_rows; the MUTABLE tier has no text index (pre-approved scope decision) and falls back to an exact full scan of the write buffer, matching the pre-task-#31 numeric-property story. GraphUnion merge (src/graph/compaction.rs): does not remap/merge posting row ids across input segments — the merged segment starts with an empty text_index cell and rebuilds lazily, from scratch, on first use post-merge. Verified correct (not just non-crashing) by a dedicated merge test. Deferred, documented in code + CHANGELOG: BM25 relevance scoring (SegmentTextIndex::bm25_score_for) is implemented and tested against real per-segment postings, proving the bm25_score reuse end-to-end, but is not wired to a Cypher ORDER BY surface — no concrete grammar was specified in the design doc for this phase. 21 new tests (lexer, parser, planner, eval, executor tier-parity, resident_bytes accounting, GraphUnion merge correctness). Both runtime feature configs compile and pass clippy -D warnings; full default- feature lib suite (3928 tests) green. author: Tin Dang
One assert_eq! in the new GraphUnion text-index merge test exceeded the line-length limit; formatting-only, no behavior change. author: Tin Dang
…text predicates (§11.7b) P1 mutable-tier property index: 25,506 → 29,014 qps, p50 −27%, shard CPU −70% (index_scan_keys eliminated from the perf profile), same-box FalkorDB ratio 2.44× → 2.78× — and the win scales with graph size (O(1) probe vs O(N) scan). P2 result cache: interleaved A/B shows the doorkeeper bounded the cache-hostile worst case at −2.4% (from −21% pre-doorkeeper) with +1.9% on hot-key repeats; real payoff documented as expensive-read replay. P3: CONTAINS/STARTS WITH/ENDS WITH + SegmentTextIndex presence-bitmap pruning (token-postings deliberately not used for substring soundness). author: Tin Dang
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
tests/graph_result_cache.rs (1)
1-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffTest suite bypasses server/connection plumbing, per its own docstring.
Tests call command handlers directly against a
GraphStorerather than through a real server instance. No mocks are involved, and this explicitly mirrors an existing file's pattern, but it's a literal deviation from the path guideline. Flagging for awareness rather than requesting a rewrite, given the precedent and the low incremental value of a full server-based rewrite for this suite.As per path instructions,
tests/**/*.rs: "Integration tests must use real server instances and not mocks."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/graph_result_cache.rs` around lines 1 - 16, The graph result cache suite currently bypasses the real server/connection path by calling command handlers directly on GraphStore, which conflicts with the integration-test rule. Update the tests in graph_result_cache to exercise the real server instance and connection plumbing instead of invoking the handlers directly; keep the cache assertions, but drive them through the same end-to-end path used by other integration tests so the Frame variant checks still validate real behavior.Source: Path instructions
src/graph/memgraph.rs (1)
743-821: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winFreeze/thaw property-index invariant only checked in debug builds.
The correctness argument for clearing
prop_indexup front infreeze()is sound (index is derived solely fromself.nodes, which is unconditionally drained right after). However,thaw()only verifies this viadebug_assert!, so a future regression that leaves stale entries inprop_indexwould silently ship in release builds — property lookups could then return phantom/missing rows onceNodeKeys are reused post-thaw, with no runtime signal.Consider a cheap non-debug guard (e.g.
if !self.prop_index.is_empty() { self.prop_index.clear(); }plus atracing::error!/metric, or an actualassert!) so a violation is visible in production rather than silently corrupting query results.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/graph/memgraph.rs` around lines 743 - 821, The freeze/thaw property-index invariant is only enforced by a debug-only check, so a stale prop_index could slip into release builds and cause incorrect property lookups after thaw. Update the thaw() path in MemGraph to include a non-debug runtime guard for prop_index being empty, and if it is not, either clear it with a visible error/metric or fail fast with an actual assert so the violation is detected in production. Use the existing freeze() invariant and thaw() method as the reference points when adding the guard.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/command/graph/graph_read.rs`:
- Around line 1389-1398: Add the missing store version bump in
execute_write_plan alongside the existing graph.touch() invalidation logic.
After confirming mutations are non-empty and before returning, update the
GraphStore version token by calling the same store bump method used in the
sibling Cypher write path so version_token advances whenever real writes occur.
Keep the change near the graph.get_graph_mut(graph_name) / graph.touch() block
so the write-generation and version tracking stay aligned.
In `@src/graph/cypher/result_cache.rs`:
- Around line 282-286: `resident_bytes()` undercounts shard memory by omitting
the eagerly allocated `doorkeeper` in `ResultCache`. Update the memory estimate
in `resident_bytes()` to include the fixed 16 KiB doorkeeper allocation
alongside `cur_bytes` and per-entry overhead, using the `doorkeeper` field and
`ResultCache::resident_bytes` as the place to fix it.
---
Nitpick comments:
In `@src/graph/memgraph.rs`:
- Around line 743-821: The freeze/thaw property-index invariant is only enforced
by a debug-only check, so a stale prop_index could slip into release builds and
cause incorrect property lookups after thaw. Update the thaw() path in MemGraph
to include a non-debug runtime guard for prop_index being empty, and if it is
not, either clear it with a visible error/metric or fail fast with an actual
assert so the violation is detected in production. Use the existing freeze()
invariant and thaw() method as the reference points when adding the guard.
In `@tests/graph_result_cache.rs`:
- Around line 1-16: The graph result cache suite currently bypasses the real
server/connection path by calling command handlers directly on GraphStore, which
conflicts with the integration-test rule. Update the tests in graph_result_cache
to exercise the real server instance and connection plumbing instead of invoking
the handlers directly; keep the cache assertions, but drive them through the
same end-to-end path used by other integration tests so the Frame variant checks
still validate real behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1dd20173-9990-4932-9173-8822538ece99
📒 Files selected for processing (32)
BENCHMARK.mdCHANGELOG.mdsrc/command/graph/graph_read.rssrc/command/graph/graph_write.rssrc/command/graph/mod.rssrc/command/temporal.rssrc/graph/compaction.rssrc/graph/csr/mmap.rssrc/graph/csr/mod.rssrc/graph/csr/storage.rssrc/graph/cypher/ast.rssrc/graph/cypher/executor/eval.rssrc/graph/cypher/executor/mod.rssrc/graph/cypher/executor/read.rssrc/graph/cypher/executor/write.rssrc/graph/cypher/lexer.rssrc/graph/cypher/mod.rssrc/graph/cypher/parser/expr.rssrc/graph/cypher/parser/mod.rssrc/graph/cypher/planner.rssrc/graph/cypher/result_cache.rssrc/graph/index.rssrc/graph/memgraph.rssrc/graph/mod.rssrc/graph/replay.rssrc/graph/store.rssrc/graph/text_index.rssrc/server/conn/handler_monoio/write.rssrc/server/conn/handler_sharded/write.rssrc/transaction/abort.rstests/graph_freeze_boundary.rstests/graph_result_cache.rs
✅ Files skipped from review due to trivial changes (1)
- CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (9)
- src/graph/csr/mmap.rs
- src/graph/csr/mod.rs
- src/graph/mod.rs
- tests/graph_freeze_boundary.rs
- src/graph/replay.rs
- src/graph/store.rs
- src/graph/cypher/planner.rs
- src/graph/cypher/executor/read.rs
- src/graph/cypher/executor/write.rs
Comment on lines
+1389
to
1398
| // Task #32: invalidate the graph's cached query results whenever this | ||
| // write actually produced mutations (mirrors the `graph_query_write` | ||
| // gate -- an idempotent MERGE match-branch must not pay an | ||
| // invalidation for a no-op). Runs on both Ok and Err (Phase 174 FIX-02: | ||
| // `mutations` already includes partial pre-error writes). | ||
| if !mutations.is_empty() { | ||
| if let Some(graph) = store.get_graph_mut(graph_name) { | ||
| graph.touch(); | ||
| } | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Compare the two write paths and inspect bump_version/touch semantics.
rg -nP '\bfn (bump_version|touch)\b' src/graph
rg -nP 'bump_version\s*\(' src/command/graph src/graph src/shard -C2Repository: pilotspace/moon
Length of output: 3176
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant write paths and store version semantics.
sed -n '960,1020p' src/command/graph/graph_read.rs
printf '\n---\n'
sed -n '1360,1415p' src/command/graph/graph_read.rs
printf '\n---\n'
sed -n '160,205p' src/command/graph/graph_write.rs
printf '\n---\n'
sed -n '360,385p' src/command/graph/graph_write.rs
printf '\n---\n'
sed -n '60,90p' src/graph/store.rs
printf '\n---\n'
sed -n '320,345p' src/graph/store.rsRepository: pilotspace/moon
Length of output: 10314
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the function boundaries and look for any higher-level version bumps
# around graph_query_or_write / execute_write_plan / transaction commit.
ast-grep outline src/command/graph/graph_read.rs --view expanded
printf '\n---\n'
rg -n "execute_write_plan|graph_query_or_write|bump_version\(" src -C 3
printf '\n---\n'
rg -n "version_token|bump_version|write_gen" src/transaction src/command src/graph -C 2Repository: pilotspace/moon
Length of output: 49258
Add the store version bump here. execute_write_plan only calls graph.touch(), but the sibling Cypher write path also advances store.bump_version() after successful mutations, and GraphStore expects callers to bump the version token on writes. Without it, version_token can miss real graph updates while write_gen still changes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/command/graph/graph_read.rs` around lines 1389 - 1398, Add the missing
store version bump in execute_write_plan alongside the existing graph.touch()
invalidation logic. After confirming mutations are non-empty and before
returning, update the GraphStore version token by calling the same store bump
method used in the sibling Cypher write path so version_token advances whenever
real writes occur. Keep the change near the graph.get_graph_mut(graph_name) /
graph.touch() block so the write-generation and version tracking stay aligned.
Comment on lines
+282
to
+286
| pub fn resident_bytes(&self) -> usize { | ||
| self.cur_bytes | ||
| + self.entries.len() | ||
| * (std::mem::size_of::<ResultCacheKey>() + std::mem::size_of::<ResultCacheEntry>()) | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win
resident_bytes() omits the 16 KiB doorkeeper.
doorkeeper is a fixed 2048 × 8 B = 16 KiB allocation created eagerly in new(), but resident_bytes() reports only cur_bytes + per-entry bookkeeping. The struct doc (Lines 276-281) explicitly frames this method as the shard's memory-budget view and warns against exactly this "unaccounted resident structure" bug class. Across many named graphs the omission compounds (16 KiB per cache), so the elastic budget systematically under-counts.
🛡️ Include the doorkeeper in the estimate
pub fn resident_bytes(&self) -> usize {
self.cur_bytes
+ self.entries.len()
* (std::mem::size_of::<ResultCacheKey>() + std::mem::size_of::<ResultCacheEntry>())
+ + self.doorkeeper.len() * std::mem::size_of::<u64>()
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| pub fn resident_bytes(&self) -> usize { | |
| self.cur_bytes | |
| + self.entries.len() | |
| * (std::mem::size_of::<ResultCacheKey>() + std::mem::size_of::<ResultCacheEntry>()) | |
| } | |
| pub fn resident_bytes(&self) -> usize { | |
| self.cur_bytes | |
| self.entries.len() | |
| * (std::mem::size_of::<ResultCacheKey>() + std::mem::size_of::<ResultCacheEntry>()) | |
| self.doorkeeper.len() * std::mem::size_of::<u64>() | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/graph/cypher/result_cache.rs` around lines 282 - 286, `resident_bytes()`
undercounts shard memory by omitting the eagerly allocated `doorkeeper` in
`ResultCache`. Update the memory estimate in `resident_bytes()` to include the
fixed 16 KiB doorkeeper allocation alongside `cur_bytes` and per-entry overhead,
using the `doorkeeper` field and `ResultCache::resident_bytes` as the place to
fix it.
…B (§10.7) Same-box Docker Qdrant, identical 50K x 384d clustered COSINE KNN10 workload with exact ground truth, matched recall@10 (>=0.999 both): Moon ingests 8.9x faster (65,695 vec/s, searchable immediately) and searches 2.53x faster (3,092 vs 1,223 qps; p50 2.60 vs 6.05 ms). Qdrant reaches HNSW-tier serving sooner after bulk load (7.8s green vs 28.7s FT.COMPACT); Moon serves from the brute tier in that window. REST-transport and shared-host caveats recorded. Also re-ran the graph head-to-head vs FalkorDB at final HEAD (interleaved, noisy host): Moon 20,777 qps mean vs 8,133 = 2.55x with ~2.9x better p50 — consistent with §11.7b's 2.78x quiet-host reading. author: Tin Dang
…omparisons (§10.8, §11.9) Re-ran both head-to-heads on real GCP instances (c3-standard-8 Xeon Platinum 8481C + t2a-standard-8 Neoverse-N1, us-central1-a, fresh native builds of branch HEAD 852d53a, competitor Docker images on the same box) to validate the earlier same-box OrbStack VM ratios. Graph vs FalkorDB (8-thread 1-hop Cypher, 2 interleaved reps): Moon wins outright on both arches — x86 11,641 vs 4,988 qps (2.34x), ARM 10,938 vs 4,028 qps (2.67x), p50 2.7-2.9x better, rep spread <0.2%. Supersedes the §11.8 shared-core e2 caveats: the p99-tail concern was a shared-core artifact (dedicated-core p99 beats FalkorDB on both). Vector vs Qdrant (50K x 384d COSINE KNN10, exact ground truth): Moon ingest 10.1x/10.9x, matched-recall (>=0.999 both) search 3.4x (x86) / 2.7x (ARM); Qdrant keeps its time-to-index-green edge. Moon's 0.9992 recall@10 reproduces identically across VM, GCE x86, and GCE ARM. author: Tin Dang
…time-to-index-green 11x (FT.COMPACT 30.2s -> 2.7s) Instrumentation showed 99.3% of FT.COMPACT wall time (30.0s of 30.2s at 50K x 384d) was the single-threaded HnswBuilder insert loop, and that a pure bulk load never compacted at all until the first explicit FT.COMPACT (the auto-compact trigger lived only on the search path; the autovacuum backstop ticks every 30s). Three changes: 1. parallel_build.rs (new): concurrent inserts into ONE shared graph — per-node parking_lot Mutex adjacency (copy-under-lock, distances unlocked, one lock at a time), RwLock entry point, levels from the same seeded LCG as the sequential builder, 1K sequential warmup, atomic-cursor fan-out. Finalize adds a connectivity repair pass (concurrent back-link pruning orphaned ~0.2% of nodes; BFS + force-link restores 100% reachability, unit-asserted) then the same BFS-reorder -> HnswGraph path — drop-in for search, persistence, and GraphUnion merge. compact() routes n >= 10K builds here; smaller segments keep the deterministic sequential builder. 2. Affinity-mask trap (3 sites): shard threads are core-pinned and spawned threads inherit the single-core mask, so available_parallelism() returned 1 — the parallel path silently ran sequential and the bg-compactor pool sized itself to 1 worker. New numa::system_parallelism() (sysfs online-CPU count, affinity- independent) sizes both; build workers re-pin round-robin across the machine, pool workers from the last core downward (they previously all time-sliced shard 0's core). 3. Insert-path trigger: the HSET auto-index hook now calls try_compact() after appending a vector, so background builds start and install DURING ingest instead of piling onto the first FT.COMPACT. Respects FT.CONFIG AUTOCOMPACT OFF; red/green-tested (test fails with the trigger removed). Measured (50K x 384d, 6-core Linux VM, release-fast): FT.COMPACT wall 30.2s -> 2.71s; 24K segment build 14.1s -> 2.65s (~88% scaling efficiency); with the trigger, segments are already built+installed by the time ingest-then-measure workloads reach FT.COMPACT (~0s residual). Multi-segment recall@10 0.9986 (vs 0.9992 single-segment). Multi-shard verified at --shards 4. Full CI-parity gate green (fmt, clippy default+ tokio, tests default+tokio). author: Tin Dang
…rant on both arches (§10.9) Re-ran the §10.8 Qdrant head-to-head on the same GCE instances at commit 061c73c (parallel HNSW build + insert-path compaction trigger + affinity-mask fixes): load-to-HNSW-serving 22.7s -> 9.9s on x86 (Qdrant 15.7s, 1.6x win) and 43.5s -> 9.5s on ARM (Qdrant 22.2s, 2.3x win). Every §10.8 metric is now a Moon win. Trade-offs recorded: ingest rate drops (builds overlap ingest, same trade Qdrant makes; still 5.8-10.4x faster) and recall@10 dips ~0.001 from multi-segment serving. author: Tin Dang
# Conflicts: # CHANGELOG.md
# Conflicts: # CHANGELOG.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Graph engine wave 2: 14 planned improvements (W2-1..W2-14) from the 2026-07 graph deep review, plus a P0 graph-durability fix discovered (and re-verified) by the GCP production-hardening battery that closes out the wave.
Wave-2 features
SET/DELETEon frozen (CSR-segment) rows>,>=,<,<=pushed into the property index)Value::String→ compactBytesreply path (allocation-free reply tier)read_onlyhonored on every read-path hit)--graph-timeout-ms, per-queryTIMEOUT)count/sum/avg/min/max/collectwith implicit groupingOPTIONAL MATCHnull-padding +WITHmid-pipeline rebindingMain-merge reconciliation (origin/main is fully merged, including deps waves): manifest cursors subsume the old
id_offset, PlanCacheread_onlyfusion, and the write_buf-is-live-tier contract (segments.load().mutableis a stale snapshot; tests re-pointed accordingly).🔴 P0 durability fix (1bbaec6) — found by the hardening soak
Under the default server config (disk-offload/WAL-v3), a 10-minute mixed-Cypher soak (98,435 nodes, zero errors) vanished entirely on kill -9 + restart. Two stacked pre-existing bugs (not wave-2 regressions):
shard-<N>/wal-v3/, but boot replayed v3 through a throwawayDispatchReplayEnginewhose collected graph commands were dropped, andreplay_graph_walonly reads the legacy v2 file. Fix: dedicated boot passshared_databases::replay_graph_wal_v3(wired inmain.rs+embedded.rs).write_buftier's only durability was WAL replay. Any crash after the first checkpoint lost every covered graph record. Fix: checkpoint finalize now runs a graph snapshot hook before the control-file update — freezes each dirty graph's write_buf to a CSR segment, persists, stamps asnapshot_lsnreplay floor (serde-default 0 keeps old dirs loadable); a failed snapshot aborts the finalize (old floor stays, WAL not recycled). Gated on a graph dirty flag so KV-only workloads pay nothing.current_lsnoff-by-one by losing exactly one node).GCP production hardening (report:
tmp/GRAPH-WAVE2-GCP-HARDENING.md, local)GCE x86_64, native-CPU release build:
Test plan
--no-default-features --features runtime-tokio,jemalloc)cargo clippy -- -D warningsclean on both feature configs;cargo fmt --checkclean[Unreleased]entry includedAddendum: Cypher-2× wave (P1/P2/P3, same branch)
User-directed follow-up: "beat FalkorDB 2× and reuse KV cache / BM25 / fuzzy search." Investigation first established Moon already beats FalkorDB 2.44× same-box (25,506 vs 10,448 qps, 8-thread harness; earlier GCE 0.90× reading was e2 shared-core steal) — then three features shipped (full analysis: BENCHMARK.md §11.7b):
8d5794b9): profiling showed 82.5% of shard CPU in the mutable-tier linear scan (index_scan_keys). NewMutablePropertyIndex(numeric BTree + string-hash, NodeKey-keyed, unified write-hook maintenance, freeze-time clear, TXN-abort re-index). Result: 29,014 qps (p50 −27%), shard CPU −70%, FalkorDB ratio → 2.78×, and the win scales with graph size (O(1) probe vs O(N) scan). 23 new tests.9b38f56c+ doorkeeper1abc4998): per-graphwrite_gen-invalidated cache of pre-encoded RESP bytes (RESP2/RESP3 keyed, decay/error/TXN-scoped reads excluded, resident-bytes accounted). Benching caught a −21% regression on cache-hostile cycling → fixed with a TinyLFU-style doorkeeper (admit on second sighting): interleaved A/B now −2.4% worst-case / +1.9% hot-key; big-read replay (24ms aggregation → byte-copy) is the production win. 18-test invalidation matrix + 14 unit tests.9d634607):CONTAINS/STARTS WITH/ENDS WITHadded to Cypher +=~, pruning frozen segments through a newSegmentTextIndexreusing the FTS posting/BM25 machinery. Presence-bitmap pruning (NOT token postings —CONTAINS 'rust'must match"trusted"); mutable tier stays exact-scan; GraphUnion merges rebuild lazily. BM25 scoring implemented + tested, Cypher syntax surface deferred. 21 new tests.Final HEAD gates: fmt clean, full default+graph lib suite 3,929 passed / 0 failed, clippy clean both configs (per-commit), tokio-feature checks green. Three pre-existing unrelated failures documented (Prometheus memory-kinds test, diskfull-environment test, AOF-pool parallelism flake).
Summary by CodeRabbit
New Features
--graph-timeout-ms) withGRAPH.QUERY/GRAPH.NEIGHBORS TIMEOUT <ms>overrides.OPTIONAL MATCH,WITHrebinding, byte-safe text predicates (CONTAINS/STARTS WITH/ENDS WITH), and richer aggregations.Bug Fixes
Performance / Operability