Repository navigation
feat(changes): graph diff, branch review and merge dry run - #966
Conversation
…ed contract Amend RFC 0030 with a public graph diff over two read targets, a branch review form against the merge base, `branch merge-base`, and a read-only `branch merge --dry-run`. Keep `commit changes` as the cause-carrying unit, retire the table-shaped `ChangeSet`, and reshape C6 around the per-commit schema content storage format 14 retains. Adds D0 evidence gates, D1-D3 phases and a decision log recording how each premise was checked.
Implement RFC 0030 D1-D3 on the engine, HTTP and the CLI. - Omnigraph::diff_page reports the net entity changes between two branches or commits in the commit-changes shape, with a diff-page token that keeps the first page's resolved endpoints. A side naming both a branch and a commit is AmbiguousReadTarget; one naming neither is ReadTargetNotFound. - Omnigraph::merge_base returns the commit a merge would classify against, by the merge's own rule. - Session::branch_merge_dry_run runs a merge's capture, classification and validation and stops before any effect. - GET /diff, GET /branches/diff, GET /branches/merge-base and dry_run on POST /branches/merge; CLI diff, branch diff, branch merge-base and branch merge --dry-run. A /diff side the actor cannot read answers the same 404 as an unknown side. - The table-shaped diff_between/diff_commits and ChangeSet are removed; the DST oracle and tests move to diff_page. - GQT gains --- diff, --- merge-base and --- merge-dry-run steps (RFC 0045) with four cases on the engine, DST and served targets.
…a branch diff Address the review of the graph diff amendment. - A merge base is a valid feed start only on the source's first-parent chain. After the target was merged into the source, the base is the imported target commit and the feed refuses it; point at branch diff for the net change and add the shape to the D0 gates. - The branch diff and merge base require read on both named branches, continuations included: their endpoints, the base and the source head, can both be readable while the target is not. Add the D0 gate. - Defer GQ exposure of the diff surfaces to RFC 0056 rather than reject it.
…raph-diff-impl # Conflicts: # docs/rfcs/0030-cdc-time-travel.md
…ed branch review Follow the RFC 0030 review corrections: - The user guide's merge-base recipe now holds only while the source has never merged the target in; after that the base is off the source's first-parent chain, the feed refuses it, and branch diff gives the net change. The existing merge-and-feed test asserts that shape. - The branch diff and merge base already authorize read on source and target. A new policy test pins the case where both diff endpoints are readable but the target is not, on the first page, a continuation and the merge base.
The DST history audit now checks each adjacent pair of recorded commits through diff_page instead of the retired diff_commits. diff_page resolves each side as a read target, which also checks the branch namespace so an ambiguous ULID is refused, and the audit pays 4 more gets in total (_audit l.get 724 -> 728). No engine operation's count changes.
azimafroozeh
left a comment
There was a problem hiding this comment.
The new surface went into GQT instead of GQ. The three step kinds (--- diff, --- merge-base, --- merge-dry-run) should be GQ statements under branch_stmt, beside branch merge … as commit; then GQT needs no new step, and the routes and CLI stay as wrappers
A base off the source's first-parent chain is refused as a feed start. A merge of the target into a diverged source is one way to get there, and a later merge back can bring the base onto the chain again, so the RFC no longer says any past target-to-source merge prevents the recipe.
…raph-diff-impl # Conflicts: # docs/rfcs/0030-cdc-time-travel.md
… statements
Review asked that the diff surfaces be GQ statements under branch_stmt,
beside `branch merge ... as commit`, rather than GQT step kinds.
- Grammar and AST: `branch diff <source> [into <target>]`, `branch diff
<from> to <to>` with `after page "<token>"` and `limit <n>`, `branch
merge-base <source> [into <target>]`, and `branch merge <source> [into
<target>] dry run`, a BranchStmt::Read control read. A bare `dry run`
is the clause, as `as commit` is.
- Server: the statements enter POST /query and are refused at POST
/mutate as reads. Each runs its route's body (shared helpers now behind
GET /diff, /branches/diff, /branches/merge-base and the merge dry run),
so authorization, the 403/404 decisions and the dry run's 409 match.
- Engine and API types: Session::branch_read answers a statement for
embedded callers; branch_read_output renders it. A branch diff answers
one row per change, the type named by its name, with the compared
commits and the continuation token in a new optional `page` object on
ReadOutput, the shape RFC 0056 proposes.
- CLI: `omnigraph query -e` runs the statements locally and remotely; the
table, kv and jsonl renderings show the page state.
- GQT: the three step kinds are withdrawn. Branch reads are `--- query`
rows steps without a shape section, taking a bound `${name}` in a quoted
side and in expected rows; the four graph_diff cases use them on the
engine, DST and served targets.
- RFC 0030 §8.5, RFC 0045, RFC 0055, user docs, the changelog note and
openapi.json follow.
|
Done in d3b05e3. The three surfaces are now GQ branch statements under The statements. Each is a control read in RFC 0055's sense: it enters
The HTTP routes and CLI verbs stay, as wrappers of the same engine call. The server runs the route's own authorization and error mapping for each statement, so the 403/404 decisions and the dry run's 409 are identical. Embedded callers (the CLI's direct path and GQT) go through one GQT. The four
The answer shape. These reuse what RFC 0056 (#670) proposes, so the two RFCs don't diverge.
RFC 0030 §8.5, RFC 0055 (decision log) and RFC 0045 are updated in the same change. |
Update: GQ statements (d3b05e3)
Following review, the surfaces are GQ branch statements, control reads at
POST /query. The CLI verbs and HTTP routes wrap the same engine call:branch diff <source> [into <target>];branch diff <from> to <to>;branch merge-base <source> [into <target>];branch merge <source> [into <target>] dry run.A
branch diffstatement answers rows, with its compared commits and continuation token in a new optionalpageobject onReadOutput. GQT no longer has its own diff, merge-base and dry-run steps; the four cases run the statements as ordinary--- querysteps. The sections below describe the step kinds this update replaced; RFC 0030 §8.5 and RFC 0045 hold the current contract.Implements the graph diff phases D1–D3 of RFC 0030 as amended in #964. This branch carries that PR's two RFC commits, so review that one first; this PR's own changes are in
feat(changes): …. Each phase is available on the engine, HTTP and the CLI, and tested at each boundary and in GQT.What changes
D1: graph diff.
Omnigraph::diff_pagereports the net entity changes between two points.commit changesentity shape and frozen order, without a cause.AmbiguousReadTarget, 400). One that names neither isReadTargetNotFound(404).diff-pagetoken binds the graph identity, a digest of the spelled sides, the resolved commit ids and the filter. Later pages therefore answer for the commits the first page resolved, even after the branches move.GET /graphs/{id}/diff?from=&to=andGET /graphs/{id}/branches/diff?source=&target=(merge base → source head, reportingbase_commit_id).omnigraph diff --from --toandbranch diff <branch> [--into].D2: merge base.
Omnigraph::merge_basereturns the commit a merge would classify against, using the merge's own rule. Exposed asGET /branches/merge-baseandbranch merge-base.D3: merge dry run.
Session::branch_merge_dry_runruns the merge's capture, classification and validation, then returns before any effect.MergeConflictsas a real merge (409).dry_run: trueonPOST /branches/mergeandbranch merge --dry-run.readon both branches, notbranch_merge.Policy.
/diffauthorizesreadon the branch each side resolves to. A denied side answers the same 404 as an unknown one, so branch existence is not disclosed. The branch routes and the dry run use the ordinary read check (403).Retired. The table-shaped
diff_between/diff_commitsand theirChangeSettypes are removed (RFC 0030 §8.6), and their callers move todiff_page:changes.rstests;legacy_columns;warm_read_cost.Two behaviors change and are noted in the changelog. A row rewritten with equal values is no longer an update. A pair across a dropped and re-added type refuses with
ChangeSchemaBoundary.Decisions changed in implementation. These are recorded in the RFC 0030 decision log.
GQT
RFC 0045 gains three steps, documented in the RFC and the GQT README:
--- diff, infrom:/to:,source:[target:]orcommit:form;--- merge-base, with--- expect commit $name;--- merge-dry-run, which takes the merge expects.The engine and DST paths call the engine. The served path sends the HTTP routes. Four cases, each on the engine, two DST seeds and the served target:
graph_diff_branch_review: merge base, branch diff, two-point diffs, and a dry run that leaves main unchanged.graph_diff_merge_proposal: a proposal's own diff, the diff from main to it, and an empty review once published.graph_diff_net_range: a reverted update, an insert later deleted, an equal reinsert, a changed reinsert and an insert later updated. Only the net change at the endpoints is reported.graph_diff_refusals: a typed not-found side, no merge base for a missing branch, and a dry-run conflict equal to the merge's.Tests run
All locally with
RUST_MIN_STACK=16777216:--test integration changes::(56 passed), with new graph-diff tests for:merge_truth_table::: every cell first runs a measured dry run and asserts zero manifest and data writes, an unmoved main, and the merge's own outcome.legacy_columns::,warm_read_cost::by_id_reads,--test forbidden_apis(35) and the threerfc_0067_*_diff_*failpoint tests.main,changes::andmerge_truth_table::again (58 passed).--test integration:data_routes::,auth_policy::andopenapi::(221 passed).openapi.jsonis regenerated.--test integration:parity_matrix::andcli_data::(119 passed), includingparity_diff_over_shared_historyandparity_branch_review.omnigraph-gqt-core(202 passed), andomnigraph-gqtgq_logic_tests graph_diff(4 cases, engine and DST), before and after mergingmain.cargo fmt --all --check, workspace clippy (also after mergingmain),omnigraph-gqtclippy,check-docs.py,typos,check-agents-md.shandcheck-ci-cells.py.Under local load (other builds held the load average near 20 on 18 cores), each new GQT case took 8.6–9.8 s against the 10 s budget, close to comparable existing cases such as
unpublished_commit_merge(8.5 s); I split the merge-proposal half out of the branch-review case for that reason. In the same conditions,runner_dispatchtimed out in 3 to 18 of its 26 tests. Each of those tests passes when run alone, and none exercises the diff steps.Review follow-up (#964)
The review of the RFC amendment found two contract gaps; both corrections are merged in from #964 (5169758), with their tests here (3bb0775).
--start after:<merge-base>works only while the source has never merged the target in. After that the base is off the source's first-parent chain and the feed refuses it. The user guide now says so and points atbranch diff.change_feed_merge_commit_is_first_parent_relative_with_merged_parent_on_causeasserts the refusal and the branch diff from that base.readon source and target.policy_branch_review_needs_read_on_the_targetpins the case where both diff endpoints are readable but the target is not, on the first page, a continuation andmerge-base.Rerun: engine
changes::(56), serverauth_policy::anddata_routes::(128), clippy for the engine and server targets, and the docs checks.