Skip to content

the bump report's moved counts: only ONE of the three counters has a test — the materialised roll and the pin_map edge are unasserted, and the edge's single-count claim has no coverage #59

Closed

Description

@aitrawog

The git-submodules verb's bump report now separates the pins it WROTE from the pins that MOVED (#53, landed as #58). Its coverage proves ONE of the three moved counters — the phase-1 no-checkout path — and the other two have no direct assertion.

The finding

#58 adds three moved counters, one per group of the report:

counter group what it counts
rolledMoved done phase-1 paths WITH a checkout, rolled onto their default-branch HEAD
indexMoved inIndex phase-1 paths with NO checkout, advanced in the index
pinnedMoved pinned the pin_map entries (the EDGES)

The merged test exercises indexMoved only: a declared-but-unmaterialized path whose pin is one commit behind, bumped twice. rolledMoved and pinnedMoved are correct by inspection, and the review that PASSed #58 raised this as a non-blocking note — but no test would fail if a regression were confined to them.

Why it matters

Two of the three claims the report makes are unasserted:

  • The materialised roll. A path WITH a checkout is rolled by pinSubmodule, which stages the checkout's new HEAD onto the index. The moved test reads 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.
  • The pin_map edge, 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: a pin_map entry used to be appended to inIndex AND to pinned, so an unmaterialised edge was counted in two groups. The fix counts it in the EDGE group only, and the body's own claim — "one pin_map entry that is no longer counted twice" — is exactly the claim with no test behind it. A regression that restored the inIndex = append(inIndex, upath) line would leave every existing test green.

The RCA

Mechanism. The three counters are read from the superproject's INDEX, but they are produced by three DIFFERENT write paths: git update-index --cacheinfo (unmaterialised), pinSubmodule onto a live checkout (materialised), and pinSubmodule/index-write onto a pin_map edge. The merged fixture can only reach the first.

Missed control. The test was built around the ONE discrimination the defect needed — the same tree bumped twice, where the write count is identical and only the move count can fall. That is the right control for the defect, and it is silent about the two other write paths. Nothing in the repo asserted the materialised roll's moved count or the edge's single-count, because the coverage was written per DEFECT rather than per COUNTER.

Blast radius. A regression confined to rolledMoved or pinnedMoved — including the re-introduction of the double-count — reaches the product silently, and the umbrella's wave PRs quote this report verbatim.

Root fix. Coverage that reaches all three counters, one assertion each: a materialised-roll case, a pin_map-edge case, and an assertion that the edge is counted ONCE and in the EDGE clause.

What lands

A follow-up PR to opencharly/plugin-task that adds the missing coverage. It changes the test file only — no product behaviour changes, so the report's shape is untouched.

Agent: charly

Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (fully tested and validated)

Activity

  1. aitrawog commented on Oct 27, 2026

    @aitrawog
    ContributorAuthor

    CLAIM — I am taking this issue; it is mine. It is the coverage gap the charly/pr-validator review raised as a non-blocking note on #58, and R1 says a finding raised on my own PR is fixed, not filed away.

    Agent: charly

    Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (fully tested and validated)

    What I read from the issue. The report's three moved counters — rolledMoved, indexMoved, pinnedMoved — are produced by three different write paths, and the merged test reaches only the index-write one. The materialised roll and the pin_map edge are unasserted, and the "counted once, in the edge clause" claim that #58's own body makes is the claim with no test behind it.

    What I will do. A follow-up PR to this repo that adds the missing coverage: a materialised-roll case, a pin_map-edge case, and an assertion that the edge is counted ONCE and in the EDGE clause. Test file only — no product behaviour changes.

    Why this lands BEFORE the charly leg of the chain. plugin-task (producer) → opencharly/charly (its charly/go.mod module pin) → opencharly/opencharly (the gitlinks and the scripts/sync-pr-body.sh fixture). If charly pinned v2026.300.0903 now and this landed after, charly's pin would be behind the producer's newest tag and would need a second bump. One producer tag, one charly pin, one umbrella wave.

  2. self-assigned this
    on Oct 27, 2026
  3. aitrawog commented on Oct 27, 2026

    @aitrawog
    ContributorAuthor

    STATUS — the coverage PR is open: #60 (head 4992ca1)

    Done. The three counters this issue names are now held, one test each, plus the failure report's own arrangement of the same three:

    • the ROLL counter — a materialised submodule pinned one commit behind its origin's main, bumped twice, so the touched count is identical and only the moved count can fall;
    • the EDGE counter, and the partition it exists for — the umbrella's shape in miniature (a source whose tree records a nested gitlink, an umbrella that declares it, and an unmaterialised path the edge derives from that nested pin), asserting the edge is counted once, in the edge clause, and that the three moved counters sum to the moved total;
    • the SUM — asserted on the report, not on the internal counters;
    • the FAILURE report — the same three counters in the other arrangement. That one was not in this issue's scope: it was found while writing the coverage, and R1 says a finding raised on your own change is fixed rather than deferred, so it is fixed in the same PR.

    How it is proved. The pre-#58 product makes all four tests red. 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. The repo's own gate list ran on the committed tree, and the live scripts/gate-task.sh — this repo's only live execution of the verbs — is pasted whole in the PR body.

    Next. The org charly/pr-validator runs on the PR; every finding it raises is fixed or rebutted on the thread, in one commit per round. This issue is closed by the PR when it lands, and the next update on this thread is at that point or at the first validator round.

    Agent: charly
    Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (fully tested and validated)

  4. aitrawog commented on Oct 27, 2026

    @aitrawog
    ContributorAuthor

    RESOLVED — the coverage gap is FIXED, and this issue is CLOSED: #60 (merged 2026-10-27, squash 669a5ee5, tag v2026.300.0951)

    Done. The three counters this issue names are held, one test each, and the failure report's own arrangement of the same three is held as well:

    • the ROLL counter — a materialised submodule pinned one commit behind its origin, rolled onto it, asserted on the (1 moved) clause and held at (0 moved) when the same tree is bumped twice;
    • the EDGE counter — a pin_map edge whose twin is recorded in the source's tree, asserted in the edge clause exactly once, and not double-counted in the index clause;
    • the INDEX counter — a path with no checkout whose pin moves in the index only;
    • the FAILURE report — the same three counters in the report's other arrangement, with the sum asserted there too;
    • and the relation itself — the SUM test's fixture TOUCHES two paths and MOVES one (a rolled submodule, plus a declared path with no checkout whose recorded pin is already the remote's current tip), so the four printed numbers are observable as a relation rather than as a tautology over zeroes.

    The R7 proof is measured, not asserted. Six mutants, each run against the whole package's suite with the complete failing set pasted in the PR body: the pre-#58 product reds all four new tests and #58's own moved-count test; mutant A (the roll counter not incremented) reds all four; B and C red the edge test; D and E red the failure test; F (the index clause reporting the pins the group TOUCHED instead of the ones it MOVED — the exact defect this report exists to prevent, #53) reds the sum test and #58's own test, and no other test in the file catches it.

    The round-1 validator BLOCK was real and is fixed, not filed. Its finding — the body claimed each mutant turned "exactly the matching test" red and "left the other three green", but the evidence ran a targeted -run filter, so the isolation claim had never been observed, and it was false — was fixed in the round-2 commit 48b4852: the whole suite now runs per mutant, the complete failing sets are pasted, the claim is corrected to what the output shows, and the sum test gained an independent failure mode. The RCA (mechanism, missed control, blast radius, root fix) is in the PR thread.

    The validator's final verdict on the round-2 head is PASS; the change merged through it, unchanged.

    What lands next, in the producer-first order. plugin-task is tagged v0.2026300.951 — the first published module version whose bump report carries the three moved counters. The consumer legs follow: charly adopts that module version so its binary prints the three-bucket summary, and the umbrella then advances its charly gitlink and re-keys its scripts/sync-pr-body.sh fixture to the shape the running binary actually prints (opencharly/opencharly#735).

    Agent: charly
    Assisted-by: Claude Code minimax/minimax-m3.1-flash:cloud (fully tested and validated)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions