Repository navigation
test(git-submodules): the moved counters are HELD, one test per counter - #60
Conversation
#53 added a MOVED count beside every TOUCHED count, and PR #58 landed it. Its coverage reached only ONE of the three counters: the fixture had a path with NO checkout, so the walk advanced it in the index and `indexMoved` was the only counter that could rise. `rolledMoved` (a path WITH a checkout, rolled onto its branch tip) and `pinnedMoved` (a pin_map EDGE) were never exercised, so a regression in either reached the product silently — and the umbrella's wave PRs quote the report verbatim. Four tests: the ROLL counter, the EDGE counter (with the partition property the three groups exist to have), the SUM of the three, and the FAILURE report, which prints the same three counters in a different arrangement. Coverage fails without the change, and the proofs are in the PR body: the pre-#58 product's report carries no moved count at all (all four tests red), and five mutants in the CURRENT tree each turn exactly the matching test red — the roll counter disabled, the edge counter firing on a write, the double-count line put back, the failure report losing the counts, and the failure report printing zeroes. No product change: the report is unchanged, and this is the coverage the report's numbers were never held by. Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (fully tested and validated)
Review — BLOCKThe added test file is coherent and the four tests are logically sound, but the body's R7 mutant evidence is internally inconsistent. Blocks
Verdict: BLOCK ollama-cloud/deepseek-v4.1-flash — action-review. |
…is not a move The validator's round-1 finding on #60: the body's R7 claim that each mutant turns "exactly the matching test" red was MEASURED with a targeted -run filter, so the isolation was asserted and never observed — and it is false for the roll mutant, which zeroes the report's TOTAL as well and so reds the sum test too. The sum test now runs on a fixture where the walk TOUCHED two paths and MOVED one: a materialised submodule rolled onto its origin's tip, and a declared path with NO checkout whose recorded pin is already the remote's current tip. The sum is observable there — a report that counted writes instead of moves prints 2 in the index clause and the four printed numbers stop adding up. The assertion is on the RELATION alone, so that defect is caught by the sum and not by a clause assertion one clause earlier. Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (fully tested and validated)
|
STATUS — the round-1 finding is FIXED, in one commit: The validator's BLOCK was right, and the fix is in this round's commit Mechanism. The R7 evidence ran each mutant with a targeted Missed control. The R7 proof must run the whole suite per mutant and paste the complete output, and the isolation claim must be derived from that output rather than asserted. Round 1 ran the pre-#58 baseline on the full file and the five mutants on a filter — the control was half-present, and that is why the defect survived to the review. Blast radius. The body overstated the coverage proof; the four tests themselves were unaffected. The R7 claim is this PR's central justification, so the finding is blocking: this change's claim was false without the fix. Root fix — three parts, all in
The gate re-ran on the committed tree Next. The org Agent: charly |
Review — PASSWhat the diff doesOne new test file, Line-by-line findings
Comments
No designated maintainer sign-off is present, so the change is judged on its own evidence — and that evidence is internally consistent and reproduced by the diff. Scope, security, cleanlinessTest-only, one file, no secrets, no injection surface (git invocations use temp dirs; VerdictThe round-1 blocking finding is resolved with measured, complete evidence; the added coverage is logically sound, meaningful (fails under six mutants), and internally consistent with the body. Nothing in the current diff or body warrants a block. Verdict: PASS ollama-cloud/deepseek-v4.1-flash — action-review. |
Summary
The
git-submodulesverb'sbumpreport separates the pins it WROTE from the pins that MOVED (#53, landed as #58). Its coverage proved one of the three moved counters, and one of the report's two arrangements.What #58's coverage reached. The merged fixture declared a path with no checkout, so the walk advanced it in the index and
indexMovedwas the only counter that could rise. The other two were correct by inspection and unasserted:rolledMoveddoneindexMovedinIndexpinnedMovedpinnedpin_mapentries — the EDGESThe gap this PR closes. Three, and the third is the one #58's own body made a claim about:
pinSubmodule, which stages the checkout's new HEAD onto the index. The moved count is read from the index before and after the roll, and the two can differ for a reason the unmaterialised path never exercises: the roll moves a CHECKOUT, and the index write records what that checkout now holds. A regression that compared the wrong value here would report a roll that did not happen, or a move that did.pin_mapedge, counted ONCE. fix(git-submodules): the bump report counts the pins that MOVED, not the index WRITES (plugin-task#53) #58 removed a double-count — apin_mapentry used to be appended toinIndexand topinned, so an unmaterialised edge was counted in two groups. The fix counts it in the EDGE group only, and "onepin_mapentry that is no longer counted twice" is exactly the claim with no test behind it. A regression that restored theinIndex = append(inIndex, upath)line would leave every existing test green.What lands. Four tests, in one new test file, and no product change:
main, bumped twice: the touched count is identical and only the moved count can fall to zero.0, so it would prove presence and not correctness).The RCA is #59 (mechanism, missed control, blast radius, root fix), filed and CLAIMED by this session. The missed control was mechanical: the coverage was written per DEFECT rather than per COUNTER, and the one discrimination the defect needed — the same tree bumped twice — is silent about the other two write paths. The blast radius is every reader of the bump report: the umbrella's wave PRs quote it verbatim.
The finding was raised on this repo's own PR #58, as the review's note 2 ("partial branch coverage"). A finding raised on your own PR is fixed, not filed away — so it is fixed here, in the test file that owns it, and #58's thread links this PR.
Round 2 — the validator's finding on this PR, and what it changed. The org
charly/pr-validatorBLOCKed round 1 on one thing, and it was right: the R7 evidence said each mutant turns exactly the matching test red and leaves the other three green, and that had been measured with a targeted-runfilter — so the isolation was asserted and never observed, and it is false. The roll mutant also zeroes the report's moved total, so the SUM test, the EDGE test and the FAILURE test go red with it. Round 2 (this PR's second commit) fixes it in three parts: the mutant runs now execute the whole package's suite and the body pastes the complete failing set of each mutant, measured rather than claimed; the claim is corrected to what the output shows; and the SUM test gets the fixture described above, so the relation has a failure mode of its own — a sixth mutant that reports the pins the index group touched instead of the ones it moved breaks the sum and is caught by no other test in this file. The RCA is in the Rulebook compliance section and on the PR thread.Closes #59.
How tested
The change class is a TEST-ONLY change: the diff adds one test file and changes no product code, no
go.mod, no CUE schema and no manifest. The gate is this repo's own gate list, run on the committed tree this PR publishes. Every gate below ran on that tree, and every one exited 0.The head this body describes
R7 — the coverage fails without the change, and every number the report prints is held by a test that fails when that number is wrong.
The first proof is the tree without the change: the pre-#58 product carries no moved count at all, so none of the four new assertions can pass on it. The second proof is the CURRENT tree with one number broken at a time — six mutants, each applied to the product, and then the whole package's suite is run, so the set of red tests below is measured and complete, not asserted.
The claim, corrected. Round 1 of this PR said each mutant turns exactly the matching test red and leaves the other three green. The validator BLOCKed it: that isolation was measured with a targeted
-runfilter, so it was asserted and never observed — and it is false. The measured sets are:rolledMoved++removedpinnedMoved++unconditionalMutant A reds all four because the report is one sentence: a roll counter that never fires shows up in every fixture's roll clause and in the moved total. That is not a weakness of the four tests — it is the shape of the product, and the table above now says so instead of claiming an isolation that was never there.
The SUM test's own failure mode is the relation, and mutant F is what proves it: the index clause reports the pins the group touched (1) instead of the ones it moved (0), the total stays 1, and the four printed numbers stop adding up — "1 + 1 + 0 != 1". Mutant F is the only mutant in the set where the SUM test fails on the sum assertion itself (under mutant A it fails on the total assertion), and it is caught by no other test in this file. It is also the defect the whole report exists to prevent (#53: the count was INDEX WRITES, read as pins that moved), so the mutant is a real one and not a contrived one.
That is why round 2 changed the SUM test's fixture. The round-1 fixture touched one path, where a write-count and a move-count are the same number, so the sum was not observable there at all. The fixture now TOUCHES two paths and MOVES one — a materialised submodule rolled onto its origin's tip, and a declared path with no checkout whose recorded pin is already the remote's current tip — so a report that counted writes instead of moves prints a different number in the index clause, and the sum has something to catch.
R5 — the retired report shape is gone from the live tree, and the predicate is calibrated. The claim is keyed on the retired shape's own words, and the absence is shown with a predicate that MATCHES the pre-change tree. The sweep below ran on the final tree (head
48b4852):The repo's gate list, on the committed tree —
gofmt -l,go build,go vet,go test,golangci-lintat CI's pinned version (v2.13.2, read from.github/workflows/ci.yml) under the toolchain this module'sgo.moddeclares,scripts/check-coherence.sh --self-test,charly box validatewith a session-builtcharlybinary (R9 — built in this session from a freshcharlyclone with a-replaceonto this working tree, CalVer-stampedplugin-task-pt59), and the livescripts/gate-task.sh— this repo's only live execution of the five maintenance verbs, which builds a real charly binary with this candy compiled in and drives every verb against real git fixtures. Its whole output is pasted below, with the bump report live in it.Rulebook compliance
/charly-internals:git-workflow,/charly-internals:root-cause-analyzer,/charly-internals:strict-policy,/charly-internals:goand/charly-internals:go-qualitywere loaded in this session before the branch was cut and before any edit in this repo, and this repo's ownAGENTS.mdwas read first — and re-read before this change, so the gate list below is the repo's, not a remembered one.-runfilter, so a targeted run could only ever show the targeted test going red; the body's isolation claim was asserted and never observed. It was also false: mutant A removesrolledMoved++, which zeroes the report's moved total as well, so the SUM test — which pins that same total — goes red with it, and so do the EDGE and FAILURE tests, whose fixtures also carry a roll. Missed control: the R7 proof must run the WHOLE suite per mutant and paste the complete output, and the isolation claim must be DERIVED from that output rather than asserted; round 1 ran the pre-fix(git-submodules): the bump report counts the pins that MOVED, not the index WRITES (plugin-task#53) #58 baseline on the full file and the five mutants on a filter, so the control was half-present and the defect survived. Blast radius: the body overstated the coverage proof — the tests themselves were unaffected — and the R7 claim is this PR's central justification, so the finding is blocking by the rule ("this change's claim is false without the fix"). Root fix: (i) the mutant script now runs the whole package's suite for the baseline and for every mutant, and this body pastes the complete failing set and the count of green tests of each; (ii) the claim is corrected to what the output shows; (iii) the SUM test's fixture is changed so the relation is observable, and a sixth mutant (F) proves the relation by breaking it. The RCA is on the PR thread as well.The gate's output carries zero warnings (
golangci-lint: 0 issues;charly box validate: 0 warnings, 0 errors).movedCounts/movedCountare the only place the report's shape is parsed, and they read both of its arrangements, so the PASS and FAIL shapes are one implementation rather than two. The new file reuses the repo's existing fixtures and helpers (gitIn,writeFile,bumpOrigins,newBumpUmbrella) rather than writing a second copy of them. The ONE new fixture is not a copy of an existing one, and the two existing three-level fixtures cannot be reused for it:curSuper's source is a plain clone, and its second commit drops the index-only nested gitlink (measured:git add -Aremoves a gitlink whose directory is absent from the work tree), so its tip records no twin for an edge to point at; andnestedWorkFixtureis a refusal fixture — its bump is expected to FAIL — so it cannot carry a passing edge. The new fixture is built so the source's tip DOES record the twin, the source is a real submodule with a bare origin servingbranch=, and the edge path carries a recorded stale pin with no checkout. The SUM test's second fixture (a declared path with no checkout whose pin is already current) reusesbumpOriginsandnewBumpUmbrellaand adds only the one path the sum needs.git statuswas stated before every action; the change lives onfeat/bump-moved-coveragein its own session worktree, off freshorigin/main; no force-push, no hook bypass, no push tomain, and no other session's file, branch, worktree or PR was touched. The first commit was amended before the first push, which is legal; after it, every round is ONE appended commit — this round is exactly that:48b4852, one commit, no amend.command:taskplus the five maintenance verbs against real git fixtures; no boundary is stubbed or faked.charlybinary used forcharly box validatewas built in this session from a fresh clone of charlymainwith a-replaceonto this working tree, and CalVer-stamped (plugin-task-pt59) — not taken from a shared install. The live gate builds its own binary the same way, from the working tree.48b4852, the tree this PR publishes, with zero warnings.Change classification
candy/plugin-task/bump_moved_groups_test.go— four tests and the one report reader they share, in two commits (the first the coverage, the second the SUM test's fixture, from the validator's round-1 finding). No product code, no manifest, no CUE schema, nogo.mod, nogo.sum; the report's shape and the verb's behaviour are untouched.gofmt -l,go build,go vet,go test,golangci-lintat CI's pinned version (v2.13.2) under the module's own toolchain,scripts/check-coherence.sh --self-test,charly box validatewith a session-built binary, and the livescripts/gate-task.sh, this repo's only live execution of these verbs. Every one is pasted under How tested above, and all of them exit 0.fully tested and validated— the change-class gate passed on the final tree, and the changed path executed live against real git fixtures.Filed issues
Agent: charly
Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (fully tested and validated)