Repository navigation
SCM note: describe the operation that shipped, and measure the scenario corpus - #8794
Merged
Merged
Conversation
…io corpus Two corrections to the design note, both about it being an authority that had stopped describing reality. SECTION 7 SPECIFIED AN OPERATION NOBODY CAN INVOKE. It read merge(store, first, second) -> Merged | ChoiceRequired | CouldNotLand. What shipped in #8719 is merge(store, target, target_dependencies, proposals) -> Merged | MergeRolesContested | MergeRefused. A stale authority is not a cosmetic problem: it is the premise contamination DESIGN records against its own CI paragraph, where a session planned an afternoon against a command that did not exist. Rewritten to the shipped signature, with the reasoning that produced it -- why the target arrived (the two-proposal form had nothing to preserve FROM, so it could only union bindings, which ships a program referencing a deleted node), why proposals became a list (a contest is a property of the population, not of a pair), and why both outcome arms are named for less than they conclude. SECTION 8 SAID "no merge kernel should be built without these" AND A KERNEL WAS BUILT. The honest thing is to state which of the thirteen it answers rather than let a reader assume. Measured by joining the 15 witness claims against the rows one at a time: FOUR ARE COVERED. The table now carries a blocked-on column, because "uncovered" was collapsing three different situations: 4 gaps sub-node recursion (the headline win; the kernel cannot express them) 3 gaps rename/move vocabulary the model does not have at all 2 gaps positional merge, an explicit non-goal of this slice 1 gap CLOSABLE NOW at the current grain -- duplicate named siblings That last one is the useful find. Measured: nothing in gunbc.scm.* or v2.std.node refuses a node carrying two children under one name, and the nearest live behaviour is content-hash canonicalization SORTING named edges, which orders duplicates rather than refusing them. It is the only structural scenario needing neither new grain nor new vocabulary. My earlier estimate from reading was "roughly five", close enough to feel safe and wrong enough to have mis-scoped the next slice. The note now says coverage claims must be measured. DIRECTION RECORDED: a CLI vertical before the depth recursion, reversing the note's own "headline win is the next slice" ordering. The reason is DESIGN section 5 rather than a change of view about value -- nothing has ever CONSUMED this kernel, and a witness suite authored alongside the code is not a consumer. Safe to reorder because depth changes how merge combines, not how add authors. The cost is designed for rather than discovered: a CLI makes the top-level-binding limit user-visible, and that refusal must say so plainly instead of reading as a defect.
gunbai-bot Bot
pushed a commit
that referenced
this pull request
Aug 21, 2026
13 conflicted paths resolved per-hunk to main's semantics; 286 import lines cut from 10 files by the brace-balanced cutter. Guards pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
gunbai-bot Bot
pushed a commit
that referenced
this pull request
Sep 4, 2026
…ened to rename The reviewer's point is structural: renaming a module re-qualifies EVERY claim it declares. The four renamed locals were a subset of the migration, and reporting them as the migration understated it by fifteen. The complete population is 20 identities -- all 19 in the renamed module (4 of which also change local name), plus one pure local rename in scm_commit_closure_json_v2_witness, whose module identity is unchanged. Checked against that population rather than against the four: five of the nineteen are cited in the SCM design document, none of them among the renamed four, and no workflow YAML, roster or coverage join enrolls the module by name -- so the re-qualification has no enrollment consequence to migrate. `git grep scm_merge_witness` is empty. While checking the coverage join I found a transcribed count that had rotted: the design document said "the 15 claims in test.claim.<module>" while the module declares 19. It was true when #8794 wrote it and has been wrong since. Per DESIGN section 6 the fix is not a corrected number -- both sites now name the module and let the count be re-derived. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VQ4iThiZ1B9LPB9ePr8qa9
briansrls
pushed a commit
that referenced
this pull request
Sep 4, 2026
…carrying two contracts (#10355) * Give the proposal vocabulary its own authority and stop one spelling carrying two contracts gunbc.scm.merge answered a bounded question -- one target commit plus N authored proposals, folded at role grain into one resulting root -- but it held the name "merge". Two-commit merge is a materially different contract: two commit occurrences and a derived common base, a different failure population, a different output meaning. Section 3 says a materially different contract needs a materially different name, and a second operation called merge was about to arrive. This cuts the existing authority over BEFORE that happens rather than after, because the fork is cheap to remove while it is still latent. THE MODULE WAS ALSO THE ACCIDENTAL HOME OF ITS OWN INPUTS, and that was measured rather than asserted. Of the eight modules importing gunbc.scm.merge, SIX consumed only the proposal nouns and never the operation -- and three of those six are production modules (gunbc.scm.status, gunbc.scm.authoring, gunbc.scm.read_command). Only the two witness modules consumed the operation. A vocabulary with three times the consumers of the operation it was filed under is not a detail of that operation. gunbc.scm.proposal Requirement, RequireBinding, RequireBindingAbsent, requirement_role, Proposal, RoleDependency gunbc.scm.role_requirement_integration everything else, with the outcome renamed off "Merge" through to the externally meaningful result DesiredRoleValue STAYS WITH THE OPERATION, which corrects my own first partition. I had placed it with the nouns on the reasoning that a contested alternative is authored intent. The module's own annotation says otherwise and I had read past it: a Requirement is authored syntax, a RoleValue is result provenance, and DesiredRoleValue is the REQUESTED STATE that both normalize into -- produced by normalize_requirement, which does object-store work and can refuse with a locator collision. Derived from authored intent is not the same fact as authored intent; a proposer cannot write one. Keeping it operation-side also keeps ObjectStore and ObjectId out of the noun layer, so the dependency direction is proposal nouns -> integration -> object store, never back. RoleDependency, NOT ProposalDependency. The integration consumes this carrier in TWO populations -- the target's own dependencies and the ones a proposal declares -- and combines them in effective_dependencies, so a proposal-flavoured name would make every target-side use a lie. It is named for the relation. Which kinds an integration supports, which propagate a removal, and how an unsupported kind refuses stay with the operation: that is classifier policy, not a property of the row. RESIDUE THIS DOES NOT CLOSE, recorded on the carrier rather than left silent: target and target_dependencies still arrive as independent arguments and nothing proves the second was derived from the first, so a foreign or empty population can admit an invalid removal or fabricate a refusal. That is gunbc#10295's subject-binding class at role-dependency grain. Moving the carrier to its right home does NOT bind the population, and the annotation says so explicitly with a next-rung trigger naming the capability. PROSE MOVED WITH THE SYMBOLS. A rename that leaves notes pointing at the deleted module is the evidence-bearing-narrative failure this lane was corrected on four times: gunbc.scm.ancestry's mechanism citation, gunbc.scm.authoring's apply_requirement citation, scm_authoring_witness_test's two references, the signature block in dag-native-scm-design.md, and the current-state claim in scm-demo-cli-rebuild.md. The witness module and its merge_-prefixed cells are renamed too, so "merge" is free for the witness that will actually need it. The terminal sweep over old identities leaves exactly one match, in the CLI plan, and it is explicitly historical and invalidated. No alias, no umbrella re-export: the old root is deleted, so a future import cannot keep choosing the ambiguous authority. EVIDENCE. Semantically a no-op, which is the whole claim: 83 cells across the five affected witness modules pass, zero fail, and all nine touched modules resolve and typecheck. Ancestry is untouched semantically -- MergedFrom and the commit-merge model are the second cut and wait on the ancestry result shape being settled first. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VQ4iThiZ1B9LPB9ePr8qa9 * Rename the witness module identity too, which my own sweep pattern could not see REQUEST_CHANGES on #10355 (review 59814). Correct, and it exposes a hole in the verification I ran rather than only a missed line. The file was renamed and the operation was renamed, but the module DECLARATION still read: module test.claim.scm_merge_witness so the witness authority kept the exact spelling this PR exists to free, against the contract it no longer describes. WHY MY SWEEP MISSED IT, which is the part worth recording. The terminal sweep ran over `scm_merge_witness_test` -- the FILE stem. The module identity is `scm_merge_witness`, with no `_test` suffix, because the corpus convention is `test.claim.scm_<name>_witness` for a file named `scm_<name>_witness_test.dag`. The pattern was strictly narrower than its subject, so it reported clean over a tree that still contained five matches. A green instrument that cannot express the defect is the decoration DESIGN section 4b calls worse than absent, and this is the same class the SCM reviewer warned about one round earlier: a stale identity survives a sweep keyed to the wrong spelling. The corrected pattern drops the suffix, and it is the one in this commit's verification: `scm_merge_witness` matches both forms, `scm_merge_witness_test` matches only one. FOUR MORE REFERENCES were behind that hole -- the finding named one: the module declaration itself scm_commit_closure_json_v2_witness_test's annotation about where the merge-arm claim lives, which additionally cited it WRONG as `test.claim.scm.scm_merge_witness` with a segment that never existed three citations in docs/plans/dag-native-scm-design.md, all naming the claim population that measures kernel coverage Renamed to `test.claim.scm_role_requirement_integration_witness`, matching the convention its 24 siblings use. The corrected sweep now leaves three matches, all in the same commit's own prose, all unmistakably historical and explicitly invalidated: "previously lived in", "It was `MergeDependency`", "formerly `gunbc.scm.merge`". 62 cells pass across the two touched witness modules, zero fail. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VQ4iThiZ1B9LPB9ePr8qa9 * Retire the last merge tags, fix a citation that was wrong before I touched it, and withdraw "semantic no-op" Three findings on this head. All correct. 1. FOUR LIVE MERGE TAGS SURVIVED the rename because they are not identities my sweep pattern covers -- they are helper and tag functions, not module or type names: outcome_is_merged, merged_roles, scm_image_merge_refusal_tag, scm_image_merge_tag_for. Renamed to the integration vocabulary. The two witness modules now contain no function whose name carries "merge". 2. A CITATION I REWROTE POINTED AT A SYMBOL THAT HAS NEVER EXISTED. The authoring witness said its node helper was "built exactly as ... `atom_node`". No atom_node has ever existed in the cited module -- it lives in test.claim.scm_object_store_witness. The citation was ALREADY WRONG on main; what this change did was rewrite its MODULE half while carrying the stale SYMBOL half forward, which makes an unverified name look freshly checked. That is worse than leaving it alone, because a rewritten citation reads as re-derived. Both halves are now derived rather than copied: the symbol is integration_witness_leaf, and the annotation states what makes the claim true -- the two bodies are byte-identical node_synthetic over an Atom connective. The error's history is recorded because a future reader has no other way to know the citation was stale rather than broken by this rename. 3. "SEMANTICALLY A NO-OP" WAS TOO STRONG AND I AM WITHDRAWING IT. Two different facts were riding on one sentence: PRODUCTION BEHAVIOUR is unchanged. Every moved declaration is byte-identical modulo its name; no fold, arm, refusal or classifier changed. That part was true. THE CLAIM CORPUS IS NOT UNCHANGED. Five claim identities were renamed -- four in the integration witness and one in the JSON codec witness -- along with the module identity and six helpers. At identity grain a renamed claim is a deletion plus an addition, not the same claim, which is exactly the standard this lane applies to residues and rosters. Calling that a no-op used the passing count as evidence for a claim the count cannot support. WHAT MAKES THE RENAMES SAFE IS A DIFFERENT CHECK, and it is the one that was missing: every renamed identity was grepped across .dag, .md, .rs and .yml for a citation outside its defining file. There are none, so no roster, coverage table or coverage number is invalidated. The design doc's kernel coverage table names claims -- an_independent_sibling_is_preserved_exactly, two_distinct_requests_for_one_role_are_contested, dependent_add_and_delete_refuses, the_same_request_authored_twice_is_not_a_contest -- and none of them is renamed here, checked one at a time rather than by pattern. 67 cells pass across the three touched witness modules, zero fail. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VQ4iThiZ1B9LPB9ePr8qa9 * Derive the whole claim-identity population, not the four names I happened to rename The reviewer's point is structural: renaming a module re-qualifies EVERY claim it declares. The four renamed locals were a subset of the migration, and reporting them as the migration understated it by fifteen. The complete population is 20 identities -- all 19 in the renamed module (4 of which also change local name), plus one pure local rename in scm_commit_closure_json_v2_witness, whose module identity is unchanged. Checked against that population rather than against the four: five of the nineteen are cited in the SCM design document, none of them among the renamed four, and no workflow YAML, roster or coverage join enrolls the module by name -- so the re-qualification has no enrollment consequence to migrate. `git grep scm_merge_witness` is empty. While checking the coverage join I found a transcribed count that had rotted: the design document said "the 15 claims in test.claim.<module>" while the module declares 19. It was true when #8794 wrote it and has been wrong since. Per DESIGN section 6 the fix is not a corrected number -- both sites now name the module and let the count be re-derived. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VQ4iThiZ1B9LPB9ePr8qa9 * Close the meaning fork where it actually lived, and pay the roster's sweep Three things, all from review 5110471067 plus the required gate. THE DESIGN DOCUMENT STILL DEFINED merge AS PROPOSAL-COMBINATION. I renamed the modules and swept their prose, then wrote a vocabulary row saying the operation is no longer called merge -- while the document below that row still carried "## 4. What merge is", "merge kernel", "Structural merge", "depth changes how merge combines" and "Merges are not always safe", all present tense. A new definition sitting above the old one does not replace it; that is the same fork inside one artifact, and it was the fork I claimed to have closed. Migrated at the root: proposal-combination is role-requirement integration throughout, merge is now reserved for the two-commit operation, and section 4 says so in its own body. Thirteen surviving uses are line-based merge, git's merge-as-invoked-event, the two-commit contrast, or explicitly historical. THE COMMON-ANCESTOR REFUSAL WAS MATHEMATICALLY WRONG AND IS WITHDRAWN. The refused-concepts table said merge base / common ancestor "requires a total order that need not exist". A commit DAG supplies a PARTIAL ancestry order; common ancestors are the intersection of two ancestor closures and best common ancestors are the maximal elements of that intersection. No total order is involved. Withdrawn in place rather than deleted, because it is a prerequisite of the two-commit merge design this note does not yet model. THE CENSUS HAD THE RIGHT ARITHMETIC AND THE WRONG UNIT. "20 claim identities" conflates migrations with identity values: each transition removes one and adds one, so the symmetric difference holds 40. It is 20 claim-identity MIGRATIONS. The population is unchanged. NAMESPACE-WAVE-ADMISSION. The required run refused with 30 unadjudicated deltas -- exactly my 30 binding sites retargeted from gunbc.scm.merge to gunbc.scm.proposal. Rows added. Touching the roster sets roster_touched, which makes its 142 consumed rows due for deletion on this change, so they are swept. Their consumption was READ FROM THE BASE rather than taken from the run that reported it: for all 142, the row's own module imports the row's spelling from extdeps.systems.nvidia_dgx_spark_setup by name. That check is what the roster's own comment says costs a required run whenever it is guessed instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VQ4iThiZ1B9LPB9ePr8qa9 * A withdrawn row cannot sit inside the population it left, and a dated join cannot cite a renamed module Two authority defects from review 5110979857, both of them mine and both introduced by fixes rather than found under them. THE WITHDRAWN ROW WAS STILL A MEMBER OF THE REFUSED SET. I marked "merge base / common ancestor" as withdrawn but left it inside a table introduced by "Terms deliberately refused -- every one", under a column literally named `refused`. The row and its container then asserted opposite things about the same term, which is the fork I had just spent the branch closing, reproduced one level up in the containing structure. The table is now a disposition table: each term's standing is stated by its own cell, and membership no longer asserts refusal. A DATED MEASUREMENT CANNOT CITE A MODULE THAT HAS SINCE BEEN RENAMED. Removing the stale "15" was right for the "witnesses are not consumers" sentence, which needs no cardinality. I applied the same edit to the 2026-08-21 coverage join, and that sentence is a different subject: it now claimed 4 of 13 came from joining "every claim declared in test.claim.scm_role_requirement_integration_witness", a module identity and population that did not exist on 2026-08-21. Naming a changing module does not re-run a historical join -- it makes a dated receipt look re-derived when nothing re-derived it, which is worse than the stale number I replaced. Historicized: the figure is stated as the result of the join over the population that existed that day, with no current-completeness claim, and the missing current-head instrument is named as what would be needed to restate it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VQ4iThiZ1B9LPB9ePr8qa9 * Take main's parse repair, and name my own bug class the way the reviewer named it Main was red at parse: #10390 left a trailing annotation block at EOF of emit_copy_qualification_witness_test.dag with no declaration after it, 16 section 4c errors, which took the head index down and with it the namespace-wave-admission verdict this branch actually needs. Repaired on main; merged here. I filed nothing against it -- three PRs were already open on that file. The ledger entry for my own three verification bugs is rewritten in the reviewer's framing, which is sharper than what I had. I wrote that an index which can be incomplete must be able to say so. The precise statement is that A PARTIAL OBSERVER RETURNED THE NEGATIVE VALUE OF A TOTAL OBSERVER. That is the part that makes these dangerous rather than merely incomplete. A parser that cannot see continuation-joined literals does not report that it failed to read a row -- it returns the row set without it, the same type and shape a complete parser returns. An index that does not know coproduct variants does not report that it lacks the form -- it returns the empty owner set, which is exactly what a total index returns for a spelling nothing declares. The answer is well formed and of the right type; only the meaning is wrong, and the caller has no way to tell. So the obligation sits on the observer, not the caller: anything reading the corpus that can be partial must be able to say it was partial, or refuse. Three instances on this branch, one shape -- an empty read from a transliterated path, a non-match from the import path, an empty owner set from a pattern that did not cover the language. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VQ4iThiZ1B9LPB9ePr8qa9 --------- Co-authored-by: gunbc-ci-auto-heal <gunbc-ci-auto-heal@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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.
Two corrections to
docs/plans/dag-native-scm-design.md, both about it having stopped describing reality.§7 specified an operation nobody can invoke
It read
merge(store, first, second) -> Merged | ChoiceRequired | CouldNotLand. What shipped in #8719 ismerge(store, target, target_dependencies, proposals) -> Merged | MergeRolesContested | MergeRefused.A stale authority is the premise-contamination failure DESIGN records against its own CI paragraph — where a session planned an afternoon against a command that didn't exist. Rewritten to the shipped signature with the reasoning that produced it: why the target arrived, why proposals became a list, and why both outcome arms are deliberately named for less than they conclude.
§8 said "no merge kernel should be built without these" — and one was built
Measured by joining the 15 witness claims against the rows one at a time: 4 of 13 covered.
The table gains a
blocked oncolumn, because "uncovered" was collapsing three different situations:That last row is the useful find. Measured: nothing in
gunbc.scm.*orv2.std.noderefuses a node carrying two children under one name — the nearest live behaviour is content-hash canonicalization sorting named edges, which orders duplicates rather than refusing them. It's the only structural scenario needing neither new grain nor new vocabulary.My earlier estimate from reading was "roughly five" — close enough to feel safe, wrong enough to have mis-scoped the next slice. The note now says coverage claims must be measured, not recalled.
Direction recorded
A CLI vertical before the depth recursion, reversing the note's own "headline win is the next slice" ordering. The reason is DESIGN §5, not a change of view about value: nothing has ever consumed this kernel, and a witness suite authored alongside the code is not a consumer.
Safe to reorder because depth changes how
mergecombines, not howaddauthors. The cost is designed for rather than discovered — a CLI makes the top-level-binding limit user-visible, and that refusal must say so plainly instead of reading as a defect.Doc-only; no
.dagor Rust changes.