perf(traversal): adaptive mode switch, limit pushdown, persisted CSR - #544
Open
azimafroozeh wants to merge 2 commits into
Open
perf(traversal): adaptive mode switch, limit pushdown, persisted CSR#544azimafroozeh wants to merge 2 commits into
azimafroozeh wants to merge 2 commits into
Conversation
azimafroozeh
force-pushed
the
traversal-refactor
branch
from
August 22, 2026 18:23
8baf728 to
d416a65
Compare
azimafroozeh
marked this pull request as ready for review
August 22, 2026 18:24
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
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
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.
What & why
Closes #533. The traversal-mode chooser ran once before hop 1, so a single-seed high-fanout traversal (frontier 1 at dispatch, 92,889 by hop 4 on the reported shape) committed to the indexed path and stayed there. This PR reworks the Expand execution path around that bug and ships four improvements; every measured result is collected in the results section below.
limiton a final filterless Expand stops the traversal at the cap, with an uncapped rerun if hydration drops below it.optimizewrites the CSR/CSC plus dictionaries to__graph_index/csr-current.bin(binary sections, per-edge-dataset identity stamps, payload sha256); cold traversal builds load it with one GET instead of scanning every edge table.Measured results, before this PR vs after
Environment: local NVMe, release build, medians of warm runs, both legs from the same instrument on the same machine; the before leg is the pristine merge base. The instrument is a drop-in example (the #533 reproduction extended with scale arguments), not part of this PR. Query legs use the #533 graph shape: 388k nodes, 2,519,918 edges, seed degree 238, per-hop frontier 1 / 238 / 5,418 / 92,889 against the 1,024 ceiling.
What the engine does on its own (auto mode):
limit 100Sanity: results are identical in every mode and every scenario (100 and 250,442 rows respectively), the after-auto uncapped time matches forced CSR (247.8 vs 247.9 ms), and the forced modes themselves are unchanged within noise, so the wins come from better decisions, not a changed engine core.
Format decision behind the artifact (not a before/after: upstream never had an artifact): both encodings measured over identical in-memory data at 100M edges (1,060 MiB raw). Binary ships because it decodes 1.9x faster than JSON+base64 (0.61 s vs 1.17 s, paid on every cold load) and is 21% smaller (1,061 vs 1,339 MiB, paid on every cold GET). A digest-free floor variant (0.30 s) was measured and rejected: about half of binary decode is the sha256, kept unconditionally because raw arrays have no syntax to fail on and a flipped byte would otherwise become silently wrong topology.
Backing issue / RFC
Checklist
traversal_adaptivesuite: 8, incl. switch equality directed/undirected, min-hops across the switch, capped-limit subset validity, artifact write/corruption/staleness; indexed/CSR equivalence battery 13 now exercises the unified core; chooser units 15; persist units 22 incl. the crafted-artifact class)docs/user/queries/index.md,docs/user/reference/constants.md,docs/user/operations/maintenance.md,docs/user/concepts/storage.md,docs/dev/invariants.md)__graph_index/is derived, regenerable, optimize-only-written, never authoritativeLocal verification
cargo test --workspace --no-fail-fast: 80 suites ok; the one failure is the pre-existing local-environmentexternal_blob_file_policy_rejects_special_filescargo clippy --workspace --all-targetsclean;cargo fmt --allapplieduser-docsandopenapiclean;rust-stringinventory updated for the executor rename and persist.rs (rows reviewed)Notes for reviewers
limit nnow returns an arbitrary valid n-subset instead of the previous deterministic slice (count exact; ordered/aggregated queries untouched). Multi-seed CSR row order also changed (hop-major); unordered order is now explicitly uncontracted. Both documented.__graph_index/is a new top-level store prefix: one derived object,optimizeis the sole writer, queries only read, every failure or staleness falls open to the in-memory scan build; no GC yet.read_bytes_if_exists_bounded/write_byteson both StorageAdapter traits; forbidden_apis registers the persist write as PhysicalOnly (derived, regenerable, never graph-visible).Greptile Summary
The PR unifies indexed and CSR traversal execution while adding adaptive strategy switching, safe limit pushdown, and persisted graph-index loading.
Confidence Score: 5/5
The PR appears safe to merge.
No blocking failure remains.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD Q[Execute query] --> D{Initial traversal mode} D -->|CSR| L[Load persisted graph index] L -->|Valid| C[Unified BFS with CSR source] L -->|Missing, stale, or invalid| B[Build graph index in memory] B --> C D -->|Indexed| I[Unified BFS with indexed source] I --> H{Next-hop frontier exceeds adaptive threshold?} H -->|No| I H -->|Yes| S[Translate BFS state into CSR dictionaries] S --> C I --> E[Hydrate emitted destinations] C --> E E --> U{Capped result under-filled?} U -->|Yes| R[Rerun Expand uncapped] U -->|No| P[Project and apply final limit] R --> P O[Optimize edge datasets] --> A[Encode and persist CSR/CSC artifact] A --> LReviews (2): Last reviewed commit: "vocab" | Re-trigger Greptile
Context used: