Repository navigation
feat(commits): page a branch's commit list - #1013
Merged
Merged
Conversation
GET /commits returned a branch's whole lineage, and building it read all of __history. It now takes `limit` (1 to 1,000) and an opaque `page_token`; a paged answer names `head_commit_id`, the head its first page resolved, and a `next_page_token` while older commits remain. Later pages answer for that head's first-parent chain even after the branch moves, and the token carries a digest of the branch, never its name. Without either parameter the answer is unchanged. A page walks first parents by falling generation from the head or the token's next commit: the head and buffer cost nothing, and an older commit is read by id with the rest of its __history extent, so a page costs the extents its own commits occupy and never the history below them. `omnigraph commit list --limit/--page-token` and the MCP `commits_list` tool page the same way.
The public and coordinator API registries in forbidden_apis classify every async entry point; Omnigraph::list_commits_page and GraphCoordinator::first_parent_page read and publish nothing.
…bulary inventory The listCommits description still names the graph-manifest version in its ordering sentence; its prose digest changed with the paging text.
4 of 7 tasks
The newest and the oldest page of main's commit list on a reopened handle, after 12 and 36 publishes under a 2048-byte history release. Both rows match at the two depths: a page reads the extents of its own commits, not the history above or below them.
5 of 7 tasks
# Conflicts: # crates/omnigraph-dst/cost_table.txt # crates/omnigraph-dst/tests/scenarios.rs
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Review of
|
Re-review of
|
| Finding | Status at e3e03310 |
|---|---|
| B1 (blocking): a continuation can list commits of a branch that the caller may not read | Still open. I reproduced it again. |
B1, doc part: the list_commits_page doc says that a continuation "is refused for another graph or branch" |
Still open. |
| O1 (optional): a paged assertion on an upgraded root | Still open. db/upgrade/tests.rs did not change. |
| O2 (optional): document the limit if B1 stays | Still open. It is more visible now (see below). |
B1. I ran the probe from the last review again, at e3e03310. The actor act-bruno can read main only (POLICY_PROTECTED_READ_YAML). The secret branch has 24 loads at history_release_bytes=2048.
| Request | Result |
|---|---|
GET /commits?branch=secret |
403 |
GET /commits/{oldest secret commit} |
403 |
GET /commits?branch=main&limit=5&page_token=T' |
200, first row graph_branch: "secret" (the forged commit) |
T' is a real main token. Its next_commit_id is that secret commit, its below_generation is u64::MAX, and its SHA-256 is recomputed. The MCP tool commits_list calls the same handler (mcp.rs:1002-1005), so it has the same exposure.
The fix that I proposed is still correct. When a policy is installed and the request has a page_token, also require the graph-wide read gate (branch: None). This is the gate of server_commit_show (handlers.rs:3289-3297). For a caller who fails that gate, refuse the continuation, or prove the position with a walk from the head. A check of the branch that authored next_commit_id is not sufficient alone. As the PR body says, a branch-scoped reader correctly pages into inherited commits that the unpaged listing shows. The permanent test is now small: #1010 added resign_change_token (auth_policy.rs:3355), which does what the probe needs.
O2. #1010 added text that says each diff continuation is checked against its sides (docs/user/branching/changes.md:75-77 and :205-209). The commit-list text (docs/user/branching/index.md:129-145) says nothing about what its token can reach. So a reader can now assume that the commit-list token has the same check. If B1 stays until #1009, state the difference there. If B1 is fixed, O2 is closed.
New findings
None. The merge adds no blocking or optional finding.
Validation
These checks ran in an isolated, detached worktree at e3e03310 (Linux, 4 cores), with RUST_MIN_STACK=16777216:
cargo test --no-fail-fast -p omnigraph-engine -p omnigraph-server -p omnigraph-cli -p omnigraph-catalog --locked --test integration -- lineage_projection:: warm_read_cost::a_commit_list_page_reads_the_history_it_returns data_routes::commit_list_pages_answer_for_the_head_their_first_page_named mcp::mcp_read_parity_tools_equal_their_routes openapi:: parity_matrix::parity_commit_list_pages parity_matrix::parity_commit_list_branch auth_policy:: changes:: changes_cost::: CLI 2 passed; engine 67 passed and 1 ignored (changes_cost::changes_page_size_one_skips_transaction_history_for_multi_version_intervals, ignored onmaintoo); server 138 passed. The server count includesopenapi_spec_is_up_to_date, the wholeauth_policymodule (with fix(changes): bind diff page tokens to the commits their sides reach #1010'spolicy_branch_review_needs_read_on_the_targetandpolicy_diff_continuation_follows_a_moved_inherited_head), and the temporary B1 probe. Before that run, I rancargo build -p omnigraph-server --bin omnigraph-server, because the CLI parity rows start the server binary.- The same package set with
--lib --bins -- changes::token commit_list: engine 13 passed, CLI 2 passed. cargo test -p omnigraph-engine --locked --test forbidden_apis: 35 passed.cargo test -p omnigraph-catalog --locked --lib -- commit_graph:: history::: 36 passed.- From
crates/omnigraph-dst:cargo test --locked --test scenarios -- dst_bench_cost_count_golden --exact: 1 passed. This covers the merge resolution. - Temporary B1 probe,
cargo test -p omnigraph-server --locked --test integration -- auth_policy::<probe> --exact --nocapture: 1 passed. The probe asserted the exposure (403, 403, then 200 with asecretrow). I removed it after the run and did not push it.
CI at e3e03310: 32 check runs. 27 succeeded, including Test Workspace, GQ Logic Tests, GQT (ordinary, served, dst, dst-clippy), DST pinned suite, Lint (clippy), Format (rustfmt), RustFS S3 (default and failpoints), Azurite, Storage Upgrade Compatibility and Fix Regression Gate. 5 were skipped: the 4 release jobs and Graph Vocabulary Guard.
Before/after proof. This PR is a feature, so the rule for bug fixes does not apply to the PR. When B1 is fixed, keep the probe as a permanent auth_policy test. Today it shows 200. After the fix, it must show the refusal. GQT cannot express this case: GQ refuses commit list as a statement, and the case needs a served bearer and a policy.
Limits. I did not run the full workspace graph, the GQT corpus, the full DST suite, the failpoints suite, the S3/Azure suites, or clippy and rustfmt locally. CI covers them at this commit. I did not measure latency. I did not derive the request counts again. I did not test upgraded (legacy) roots (O1).
Recommendation
Request changes. Fix B1 at the server and keep the probe as a test. After that, I expect to approve. O1 is optional. If the maintainers decide to keep B1 until #1009, that decision is theirs to make. In that case, O2 becomes necessary.
…ller
A commit-list page token is checksummed, not signed, so the commit a
continuation starts at is the caller's claim. A caller allowed to read
main only could re-sign a main token to list a secret branch's commits,
which GET /commits?branch=secret and GET /commits/{id} both refuse it.
Under a policy, a continuation by a caller without the graph-wide read gate
(the gate of GET /commits/{commit_id}) now goes through
list_commits_page_on_branch: the token's commit must be the first commit
below its generation on the branch's first-parent chain from the head, or
the page is refused with 400. That proof walks the chain, so such a
continuation costs what the whole listing costs; a caller with graph-wide
read keeps the page cost. The list_commits_page doc says what the token
does not prove.
Pages through both doors concatenate to the listing on an upgraded root,
whose converted commits carry pre-upgrade ids.
…h a read check list_commits_page_on_branch becomes list_commits_page_readable, which takes the same ReadCheck as diff_page_readable. A continuation's commit is on the branch's history when the branch's __manifest holds it (its head or buffer), when a branch the caller may read published it, or else when a walk down the branch's first-parent chain finds it. So a branch-scoped caller paging its own branch no longer walks the history on every page; only a commit neither cheap rule covers costs the walk. The page is read once and the check uses its first commit. An unknown commit is refused exactly as an unreadable one is. A missing commit met inside the walk is an integrity error, not a 404. The server shares one read decision between the diff and commit-list doors. The API documents that a continuation's head_commit_id is the one its token carries, and openapi.json describes the scoped caller's cost.
# Conflicts: # docs/user/cli/reference.md
# Conflicts: # crates/omnigraph-cli/src/cli.rs # crates/omnigraph-cli/src/main.rs # crates/omnigraph-cli/src/output.rs
4 of 7 tasks
ragnorc
added a commit
to ragnorc/omnigraph
that referenced
this pull request
Oct 18, 2026
ModernRelay#1036) * fix(cleanup): finish a table whose interrupted sweep removed its index directory The collector deletes a swept version's files before its manifest, so a pass that stops in between leaves the manifest for the next pass. On a local store that next pass failed for the table: `delete_paths` called `remove_dir_all` again on the `_indices/<uuid>` directory the stopped pass had removed, and Lance's local provider reports an absent directory as `NotFound`. The table kept the error and deleted nothing on every later run. Object stores list the prefix empty and succeed, so S3, Azure and the DST memory store converged already. An absent index directory is now already deleted, as an absent file already was. The files-before-manifest order is unchanged, and the deletion set still holds only paths no retained root marks. The failpoints test stops a pass at `cleanup.sweep_pre_manifest_delete` after it removed an index directory only the pruned pin names, then proves the next pass removes that manifest, reports no error, and leaves every retained root's paths in place. * docs(cleanup): state the sweep order before the retry outcome Reword the maintenance guide's sentence on an interrupted cleanup so the deletion order and the next run's outcome read as two steps, and point the release note at the guide's Cleanup section. * test(cleanup): interrupt the sweep between manifests and before dead-staging manifests Cover the directory-touching windows of the collector's sweep on a local store, where Lance reports removing an absent directory as NotFound: - a pass stopped between two manifests of the published sweep, with an index directory among the files it already deleted; - a pass stopped after the dead stagings' files and before their manifests, through the new seam cleanup.sweep_pre_dead_manifest_delete. Each retry reports no error, removes the remaining manifests and keeps every retained root's paths. * test(cleanup): run the sweep crash windows on S3 and Azure The three collector-retry fixtures in failpoints.rs become helpers over a graph uri. The file tests keep their names and call them on a temporary directory; one RustFS/S3 test and one Azure/Azurite test run all three, each on a fresh graph root, and both CI jobs require them by name. The first fixture now also stops a pass at cleanup.sweep_pre_file_delete, the one sweep window no failpoint test reached: the pass deletes nothing and the next pass plans the same sweep. Its retry asserts what the other two already did, no error on any table and nothing left to remove. Index-directory presence goes through the storage adapter's exists rather than std::fs, so the same assertion holds on an object store. The RFC's crash-window gate is satisfied and leaves blocked_on; the evidence bullet names the owners per backend. * test(dst): reach the dead-staging sweep window Register cleanup.sweep_pre_dead_manifest_delete in the crash-window catalog (appended, 53 windows) and give it a milestone recipe: insert, index build, insert, cleanup. The reach census's setup crash stops the index build before its manifest commit, stranding a detached staging; the second insert moves main's head past the head the staging recorded, so the recipe's cleanup judges it dead and crosses the window. A window inside cleanup's per-table pass does not fail the op: cleanup records the table's error and returns Ok. The harness treated that as a plain success, so no retry was judged and the report kept no trace of the window. crash_op now returns CleanupDeferred when the op's deferrals name the scheduled window; the universe records deferred:<window>@op<i> and runs the obligation pass a crashed cleanup gets (a clean rerun that defers nothing, the collector oracle, the world read). This covers cleanup.table_gc and the three earlier sweep windows as well. The pin dst_dead_staging_sweep_window_is_reached_and_retried probes the window and stops the recipe's cleanup at it; the sensitivity proof gains a deferred-cleanup case. Both go red with the routing reverted. * test(dst): schedule setup crashes in the hunt and the fleet cleanup.sweep_pre_dead_manifest_delete needs an earlier crash's aftermath: an index build stopped before its manifest commit. Only the reach census added that crash, by spending the universe's one crash_on_match slot on it, so the hunt and the fleet scheduled the window on a cleanup with no dead staging and it never fired there. Scenario::setup_crash is a second slot, matched before crash_on_match and always a real crash, even under probe_only; crash_on_match starts matching after it. harness::setup_crash_for replaces the test-only census_setup and keeps only its live entry; window_needs_wide and window_needs_schema_ops include the setup's needs. The reach census, the hunt and the fleet's window arm take the setup from it, and the hunt counts a hit from the window's own verdict instead of crashes > 0, which a setup crash alone would satisfy. The pin now stops the cleanup through the fleet arm's scenario instead of a pinned op index. * feat(release): publish daily pinnable edge builds and stamp and attest every release (ModernRelay#1031) * feat(release): publish daily, pinnable and attested edge builds Edge releases now build once a day from the newest audited commit on main and publish each build as its own prerelease, edge-<commit date>-<short commit>, which never changes once published; the rolling edge release copies it and only moves forward. Both release workflows share one archive build that stamps the commit and channel into the binaries, runs each archive on its platform and verifies the exact set, and both attest build provenance. Each edge release states its storage format and HTTP contract next to the latest stable release's, then the notes recorded since that release. * fix(release): attest only the commit a release run is for The provenance predicate takes its source commit from the run's OIDC sha claim, not from the checkout. An edge run now builds only its own commit, waiting up to 90 minutes for that commit's audit instead of walking back to an older one, and both publishers refuse to attest a build of any other commit; a stable backfill from v0.14.0 is dispatched at its tag. * fix(release): keep edge runs from overlapping A newer edge run could move the rolling tag after an older run published but before its installer checks ran, failing a release that succeeded. Whole runs now share one concurrency group. * feat(release): stamp and attest the container image The image's bookworm-built binaries now carry the same commit and stable channel stamp as the release archives, checked inside the image before it is pushed, and the pushed GHCR digest carries a build provenance attestation under the same rule: only for the run's own commit, and from v0.14.0 a backfill is dispatched at its tag. * fix(release): address review of the edge release rework - Refuse an edge build while the package version is not above the latest stable release: X.Y.Z-edge+<commit> sorts before X.Y.Z, so the ordering claim held only after main advanced its version. - Require the commit's CI Gate as well as its vocabulary audit, waiting up to 180 minutes, since the build becomes a pinnable, attested release; a run cancelled because main moved on now ends the edge run with a notice. - Keep the newest edge build of every storage format beyond the retained 30, read from a marker line in the release text, so the build that can export an abandoned development format survives. - Report one build identity from omnigraph-api-types: the CLI, /healthz and the MCP server info share the stamped version and its compile-time checks. - Compose the edge release text from the real release-note preview in CI, and hold the two copies of the v0.14.0 attestation cutoff to one value. * feat(load): plan CSV overwrites and reuse validation reads (ModernRelay#1026) * fix(cli): accept --as on commit publish and drop (ModernRelay#1029) * fix(cli): accept --as on commit publish and drop The plane guard classified every `commit` subcommand as a read and refused `--as`, so `commit publish` and `commit drop` against a direct graph could only attribute `operator.actor`, although both dispatch through the write resolver and the engine's `_as` entry points. The guard now matches the `commit` subcommands exhaustively: publish and drop take `--as` on a direct target, while list, show and changes still refuse it. A served publish or drop still refuses `--as` before any request. Guard errors name the subcommand (`commit list`, ...). The published commit keeps the actor of the write that created it; the branching guide now says so. * test(cli): pin created_at and fold the served refusal into its owner The publish regression now also checks that the published commit keeps the proposal's `created_at`, the second half of the branching guide's claim. The served `--as` refusal for `commit publish` and `commit drop` moves into `as_on_a_served_write_is_rejected` as two more argument rows instead of a parallel fixture. Rewrap the edited guide paragraph. * docs(changelog): say a published commit keeps its timestamp too The branching guide and the regression test state both the actor and the timestamp; the release note now says the same. * fix(cli)!: send a server token only to the server it is keyed to (ModernRelay#1030) * refactor(cli): centralize invocation and command boundaries (ModernRelay#1047) * refactor(cli): capture operator config during client setup * refactor(cli): reuse config for aliases and served clusters * refactor(cli): isolate invocation parsing * refactor(cli): make scope resolution pure * refactor(cli): separate application completion from process exit * refactor(cli): extract query family and typed preparation * refactor(cli): capture presentation and preserve delivery outcomes * test(cli): qualify captured presentation across execution * refactor(cli): isolate graph history execution and output * fix(cli): track dedicated commit write outcomes * docs(cli): describe history delivery and outcome boundaries * refactor(cli): isolate transfer execution and delivery * refactor(cli): isolate schema lifecycle and lint execution * refactor(cli): isolate maintenance execution and delivery * refactor(cli): isolate cluster control execution and delivery * refactor(cli): complete invocation config and I/O boundaries * fix(cli): preserve failures and recovery across output errors * docs(cli): consolidate server credential guidance * feat(commits): page a branch's commit list (ModernRelay#1013) * feat(commits): page a branch's commit list GET /commits returned a branch's whole lineage, and building it read all of __history. It now takes `limit` (1 to 1,000) and an opaque `page_token`; a paged answer names `head_commit_id`, the head its first page resolved, and a `next_page_token` while older commits remain. Later pages answer for that head's first-parent chain even after the branch moves, and the token carries a digest of the branch, never its name. Without either parameter the answer is unchanged. A page walks first parents by falling generation from the head or the token's next commit: the head and buffer cost nothing, and an older commit is read by id with the rest of its __history extent, so a page costs the extents its own commits occupy and never the history below them. `omnigraph commit list --limit/--page-token` and the MCP `commits_list` tool page the same way. * test(commits): register the paged commit list as a read-only surface The public and coordinator API registries in forbidden_apis classify every async entry point; Omnigraph::list_commits_page and GraphCoordinator::first_parent_page read and publish nothing. * test(commits): track the reworded commit list description in the vocabulary inventory The listCommits description still names the graph-manifest version in its ordering sentence; its prose digest changed with the paging text. * test(commits): pin commit-list page storage calls in the DST cost golden The newest and the oldest page of main's commit list on a reopened handle, after 12 and 36 publishes under a 2048-byte history release. Both rows match at the two depths: a page reads the extents of its own commits, not the history above or below them. * fix(commits): prove a commit-list continuation for a branch-scoped caller A commit-list page token is checksummed, not signed, so the commit a continuation starts at is the caller's claim. A caller allowed to read main only could re-sign a main token to list a secret branch's commits, which GET /commits?branch=secret and GET /commits/{id} both refuse it. Under a policy, a continuation by a caller without the graph-wide read gate (the gate of GET /commits/{commit_id}) now goes through list_commits_page_on_branch: the token's commit must be the first commit below its generation on the branch's first-parent chain from the head, or the page is refused with 400. That proof walks the chain, so such a continuation costs what the whole listing costs; a caller with graph-wide read keeps the page cost. The list_commits_page doc says what the token does not prove. Pages through both doors concatenate to the listing on an upgraded root, whose converted commits carry pre-upgrade ids. * fix(commits): check a scoped commit-list continuation cheaply, through a read check list_commits_page_on_branch becomes list_commits_page_readable, which takes the same ReadCheck as diff_page_readable. A continuation's commit is on the branch's history when the branch's __manifest holds it (its head or buffer), when a branch the caller may read published it, or else when a walk down the branch's first-parent chain finds it. So a branch-scoped caller paging its own branch no longer walks the history on every page; only a commit neither cheap rule covers costs the walk. The page is read once and the check uses its first commit. An unknown commit is refused exactly as an unreadable one is. A missing commit met inside the walk is an integrity error, not a 404. The server shares one read decision between the diff and commit-list doors. The API documents that a continuation's head_commit_id is the one its token carries, and openapi.json describes the scoped caller's cost. * feat(release): publish an edge container image from each edge build (ModernRelay#1060) * feat(release): publish an edge container image from each edge build The daily edge run now also publishes a container image of the same build. It packages the published build release's Linux x86_64 archive, checked against its checksum and its attestation for this workflow and commit, on Debian trixie, since the Ubuntu-built archives need glibc 2.39 or newer. The root Dockerfile takes its base as BASE_IMAGE, bookworm by default, so the stable image is unchanged. The image is tagged edge-<commit date>-<short commit> on GHCR and Docker Hub, checked inside the image for its commit and channel, attested on both registries, and tagged edge only when the rolling edge release moved. It never moves latest, and a rerun reuses the image first pushed for the build. * fix(release): match the image's server features and follow edge as it is now - Build every release archive with the server's aws feature, as the stable image already is, so the edge image packaged from the Linux archive accepts OMNIGRAPH_SERVER_BEARER_TOKENS_AWS_SECRET like the image it replaces. Lance's S3 support already builds the AWS SDK base; the feature adds one crate. The gate checker requires both release builds to enable it. - Move the edge image tags, and check the edge installer channel, only while the rolling edge release still names this build, so a rerun of an older run's failed job cannot move them behind a newer build. * fix(release): skip an edge build whose workflows no longer match main (ModernRelay#1061) GitHub refuses the workflow token a new tag on a commit whose workflow files differ from main's current ones ("refusing to allow a GitHub App to create or update workflow"), and the token can never hold the workflows permission. An edge build of 02026b5 failed this way after a workflow change merged during its build. The edge run now checks that the build's workflow files still match main's before building and again before tagging, and otherwise publishes nothing, with a notice; the image and installer jobs run only for a published build. * fix(release): build the Linux x86_64 archive on a pinned Ubuntu 24.04 runner (ModernRelay#1063) `ubuntu-latest` moves to Ubuntu 26.04 between 2026-10-19 and 2026-11-19 (actions/runner-images#14748), and a run may land on either image during the rollout. A Linux archive needs the glibc it was linked against, so the x86_64 archive would start requiring Ubuntu 26.04's glibc, breaking Ubuntu 24.04 and Debian 13 users and the trixie edge image. Pin it to ubuntu-24.04, as the arm64 archive already is (ubuntu-24.04-arm), document the archives' glibc 2.39 floor, and have the gate checker refuse ubuntu-latest for archive builds. * fix(release): pull image bases from Docker Hub when logged in (ModernRelay#1065) The first edge image build failed twice pulling debian:trixie-slim from ECR Public: "429 Too Many Requests: Data limit exceeded". GitHub-hosted runners pull it anonymously from shared addresses, whose data limit is often spent, and the stable image pulls its bookworm base the same way. Both image workflows now log in to Docker Hub when the DOCKERHUB_* secrets exist and build on the Docker Official Image; the stable workflow's rust:1-bookworm builder pull is then logged in too. Without the secrets, the base still comes from ECR Public. * fix(schema): drop a property together with the constraints that name only it (ModernRelay#1062) A property-level @index, @unique or @key is also stored as a type-level constraint naming the property by stable id. Dropping the property made the planner see that constraint disappear and refuse the whole change, because the constraint comparison did not know which properties the plan drops. An indexed property could therefore never be removed. The property planner now returns the stable ids of the properties it drops, and the constraint comparison no longer refuses a removed constraint when every field it names is one of them. A @key, a constraint that also names a field that stays (a remaining property or @src/@dst), and a constraint removed from a property that stays are still refused. Apply needs no change: the drop's Operation::Project already removes the index on the dropped column, and the evolved table declares full-text analyzers from the desired catalog. ConstraintIR::fields replaces two copies of the same match in schema_ir.rs and serves the new rule. Fixes ModernRelay#1014 * feat(query)!: refuse full-text calls without a full-text index (ModernRelay#1066) * query: refuse full-text calls without a full-text index (T27, FullTextIndexRequired) A full-text call (`search`, `fuzzy`, `match_text`, `bm25`) reads its property's full-text index, whose analyzer folds case and stems; without one Lance scans with a bare, case-sensitive tokenizer, so the same call answered differently on an indexed and an unindexed property. - T27 at type checking: a full-text call on a String node property with no one-column full-text `@index`/`@key` is refused, from the schema alone. `PropType::index_kind` moves to the compiler and index maintenance uses it, so the type checker and the index builder cannot drift. An enum's fix points at `=`; a non-String field keeps its operand error. - FullTextIndexRequired at planning (HTTP 409, `full_text_index_required` in the error body): the planner reads the coverage of every property a full-text call names, filters included, and refuses `absent`. An untrained segment, which every write now declares, is `partial` and answers; a table with no fragment, or a type the snapshot does not hold, counts as covered. Acceptance refuses a plan that records no coverage, or `absent`, for a call, and replay re-derives every recorded coverage from the pinned dataset. Regressions: cases/v2/issue_747_full_text_calls_need_a_full_text_index.gqt and search.rs::a_full_text_index_with_no_segment_refuses_the_call_issue_747, both red with their check removed. * docs: rescope RFC 0047 step 3 and document the full-text index requirement RFC 0047 records step 3 as refusing only an index with no segment, now that every write declares a declared column's analyzer, with a decision-log entry, and its rollout table records steps 2 and 2a as merged. The search guide, troubleshooting page and execution guide describe T27 and the 409; the release notes call T27 breaking and tell operators to validate stored queries before upgrading. * feat(query): the ranked binding roots its component (ModernRelay#1069) * feat(query): the ranked binding roots its component A nearest(), bm25() or rrf() order on a binding a traversal reaches was refused at planning unless that binding was declared first, and a ranked binding no declaration introduced could not be ranked at all. Lowering now roots the top-level component holding the bindings the leading order key ranks at that binding; a ranked binding only a traversal introduces gets a NodeScan of its checked type, and every other binding of the component is reached by a forward or reverse expand. An rrf() whose arms rank two bindings one traversal connects is the type error T59. The planner codes P001 and P005 are withdrawn; the planner keeps both checks as internal errors. * docs(search): rank a traversal-reached binding * test(cleanup): qualify scoped retries and keep setup pending until a crash --------- Co-authored-by: Azim Afroozeh <13484327+azimafroozeh@users.noreply.github.com> Co-authored-by: Andrew Altshuler <andrew@modernrelay.com> Co-authored-by: aaltshuler <andrew@collectivelab.io>
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
GET /graphs/{id}/commitsreturned a branch's whole lineage, and building that answer read all of__history, with no limit or cursor. OmniGraph Studio's History view (spec 012, row E10) loads the full list and warns above 5,000 commits. This PR adds keyset paging whose cost follows the page, not the history:GET /commits?branch=&limit=&page_token=:limitis 1 to 1,000 (413above). A paged answer addshead_commit_idandnext_page_token. The list is newest first along the branch's first-parent chain.omnigraph commit list --limit N/--page-token T, embedded and served.commits_listtool takes the samelimit/page_token.The pattern follows
GET /diff. The first page resolves the branch head and names it. Later pages answer for that head's chain even after the branch moves. The token carries a digest of the branch, never its name. Withoutlimitorpage_token, the response is the same as before, byte for byte.limitandpage_tokenare refused (400) withunpublishedorunattached.Mechanism.
CommitGraph::first_parent_pagewalks first parents from the head, or from the token's next commit, by falling generation. Commits in the head and buffer (and any the handle's history cache already holds) cost nothing. Older commits are read by commit id with the rest of their__historyextent, so a page costs one read for each extent its own commits occupy and never reads the history below it. The walk refuses a first parent whose generation does not fall. The token (commit-list-page, in the shared continuation envelope) binds the hashed graph identity, the branch digest, the head, the next commit and a generation bound.Backing issue / RFC
Implements OmniGraph Studio spec 012, row E10. No issue for it exists in this repository; one can be filed if the PR should have one. Refs #1009 for the token limitation below.
Checklist
openapi.json, changelog note)__manifestversion and immutable__historyobjects (invariant 3).Local verification
On this branch (on
mainat 864495a):cargo fmt --all --check: cleancargo clippy --workspace --all-targets --locked -- -D warnings -W clippy::dbg_macro: cleancargo build -p omnigraph-server --locked, thenRUST_MIN_STACK=16777216 cargo test -p omnigraph-engine -p omnigraph-server -p omnigraph-cli --locked --test integration -- lineage_projection:: warm_read_cost:: data_routes:: mcp:: openapi:: parity_matrix::: engine 29 passed, server 198 passed (openapi.jsondrift test included), CLI parity 28 passedcargo test -p omnigraph-cli --locked --bins commit_list: 2 passedcargo test -p omnigraph-engine --locked --lib changes::token: 12 passedpython3 scripts/check-docs.py: OK;python3 scripts/release_notes.py preview --working-tree: renders the noteBefore rebasing onto current
main:cargo test -p omnigraph-catalog --locked --libpassed 207, and the full server integration suite passed 264.Not run: the canonical workspace test graph, the GQT corpus, and the S3/Azure suites. CI covers them.
Notes for reviewers
Cost instrument.
warm_read_cost::a_commit_list_page_reads_the_history_it_returnscounts__historyrequests from a cold handle:It asserts that the newest page reads nothing, that the oldest page stays flat as the history above it grows, and that the whole listing grows (so the instrument can detect growth).
Order. Pages concatenate to the unpaged listing across a fork, a three-way merge and released
__historyblocks (lineage_projection,warm_read_cost, the data-route test and the CLI parity row). Along one first-parent chain, generation andlineage_keyboth fall, so the walk and the existing sort agree.Known limitation, tracked in Authenticate continuation tokens: page tokens and feed cursors are checksummed, not signed #1009. Like every continuation token, this one is checksummed, not signed. No cheap proof shows that the token's next commit is on the branch's chain: the only check is a walk from the head, which costs O(history), the cost paging removes. So a re-signed token can list commit metadata (ids, parents, actor ids, times, authored branch) for the chain below any commit id the caller supplies. It exposes no row data, and commit ids carry random parts. Authorizing each continuation by the authored branch was considered and rejected: it would refuse branch-scoped readers paging into inherited protected history that the unpaged listing shows them.
The continuation reuses
ChangeCursorRejected(400, prefixchange cursor rejected:), because the envelope is shared with the change tokens.