fix(dd045): item 2 — a stable pointer to the run of record - #105
Conversation
bench-results/RUN-OF-RECORD is a tracked pointer FILE (not a symlink — this checkout has core.symlinks=false, on which a tracked symlink materialises as a plain-text file naming its target rather than resolving) holding one non-comment line naming the current run-of-record directory. scripts/check-citations.py's resolve() now substitutes a cited bench-results/RUN-OF-RECORD/... prefix with the directory the pointer names before matching it against the tracked tree (resolve_run_of_record()), reusing the existing FAILED/DEAD-PATH reporting path rather than adding a parallel one. A dangling, missing, or malformed (0 or 2+ non-comment-line) pointer is never a silent pass or a guess: each was proven empirically to produce exit=1 with a DEAD PATH finding naming the reason, and restoring the pointer returned exit=0 again every time. Repointed all 14 occurrences across the 5 files that named verify-20260730T215842Z (counted with grep -o | wc -l, not grep -c, per the lesson 16da079 shipped) to bench-results/RUN-OF-RECORD/; check-citations.py's disposition counts are byte-identical before and after. Removed the now-moot "proposed, not yet built" allowlist exemption for the pointer path. scripts/verify-dd043-pr3.sh was not touched and does not write this file — promoting a run stays a deliberate, manual edit in the same commit that adds the new run directory, per the pointer file's own comment header. Closes DD-045 item 2 in TODO.md and docs/ROADMAP.md, describing what was actually delivered. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LHBLwqfM1acog2bHRS8WCb
…ring resolve() substitutes the RUN-OF-RECORD pointer; the separate pinned-row resolver for */citations.txt does not. Moot as written — neither tracked citations.txt names the run of record — and it fails closed rather than open. Recording it because an undocumented gap in a gate reads as coverage, which is the defect class this whole thread exists to remove; it surfaced in the implementer's report and would otherwise have lived only there. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LHBLwqfM1acog2bHRS8WCb
|
@claude please review this PR. This one edits
Also worth your attention:
Do not take the PR description at face value — I wrote it, and it argues for the change rather than evidencing it. |
|
Claude finished @basquin-bot[bot]'s task in 7m 15s —— View job Review: PR #105 — DD-045 item 2 (RUN-OF-RECORD pointer)
Note on tooling: I could not execute Finding 1 (blocking): a bare
|
…ore ever resolving it — a bare RUN-OF-RECORD/ citation bypassed validation Review finding: `resolve()` matched `path in tracked_set` before the RUN_OF_RECORD_PREFIX branch. bench-results/RUN-OF-RECORD is itself a tracked FILE, and the caller strips a citation's trailing slash before calling resolve() — so a bare `bench-results/RUN-OF-RECORD/` citation (nothing after the slash) arrived as exactly that tracked path and returned "exact" on the first line, never reaching resolve_run_of_record(). Three citations (TODO.md:1200, TODO.md:1293, docs/ROADMAP.md:102) are written that way, and a pointer deliberately pointed at a nonexistent directory verified clean for all three while sub-path citations of the same pointer correctly failed. Fix: check path == RUN_OF_RECORD_FILE (or the prefix) FIRST in resolve(), ahead of the generic short-circuit. A bare citation now resolves to the pointer's target DIRECTORY, not the pointer FILE, so file_cands is empty for it exactly as for any other directory citation — a nearby quoted value has nothing to check against, instead of silently matching the pointer's own one-line text. Proved empirically: all four break modes (dangling, missing, 0-line, 2-line pointer) now produce a DEAD PATH FAILED finding and exit=1 for all three bare citation sites; restoring the pointer returns exit=0. Corrected the docstring's and TODO.md/ROADMAP.md's claims that a broken pointer "fails the run whenever a RUN-OF-RECORD/ citation exists" / "proved empirically for all three modes" — both were true only for the sub-path shape. Sibling shape found, NOT fixed here (reported for a deliberate decision): the citing-dir-relative branch of resolve() (`rel = normpath(cite_dir/path)`, checked directly against tracked_set/tracked_dirs) has the same ordering bug. A citation like `../RUN-OF-RECORD/` from a file one level under bench-results/ normalises to the bare pointer path and can match tracked_set there too, bypassing resolve_run_of_record(). Not currently exploited (no tracked citation is written that way), but structurally present. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LHBLwqfM1acog2bHRS8WCb
…acked_set before ever resolving the RUN-OF-RECORD pointer — same bug 793c18e fixed, one branch further down 793c18e fixed resolve()'s literal-path short-circuit but explicitly deferred the sibling: the citing-dir-relative branch (`rel = normpath(cite_dir/path)`) still checked `rel in tracked_set or rel in tracked_dirs` directly, with no pointer substitution. `bench-results/dd043-pr3-restvillains-2026-07-26/README.md` sits one level under bench-results/ and already cites the run of record, so a future edit there in relative form (`../RUN-OF-RECORD/`) would normalise to `bench-results/RUN-OF-RECORD` — the pointer FILE's own tracked path — and return "exact" without resolve_run_of_record() ever running. Not exploited today (no citation is written that way), but structurally present and one edit away. Fix: fold both candidate shapes resolve() is willing to call an exact match — the literal path and its citing-dir-relative normalisation — through one shared helper, tracked_or_pointer(), which checks the pointer substitution FIRST and the generic tracked-tree membership second, for whichever candidate it is given. One place expresses the ordering instead of two copies that can drift independently, so a third citation shape added later inherits the correct order automatically. Verified no third "check tracked_set before resolving the pointer" site exists in resolve(): the basename/tail-guess branches never reach RUN-OF-RECORD by accident, because a bare `RUN-OF-RECORD` token (no bench-results/ prefix, no extension) never matches TOKEN_RE or DIR_TOKEN_RE and so is never extracted as a citation at all. Proved empirically: added a temporary `../RUN-OF-RECORD/` citation to the restvillains README (one level under bench-results/), broke the pointer four ways (dangling, missing, 0-line, 2-line) — each produced a DEAD PATH FAILED finding on that citation and exit=1; the intact pointer round-tripped exit=0 with byte-identical output to the pre-change baseline. Removed the scratch citation before committing; git diff on that README is empty. Updated the module docstring's RUN-OF-RECORD POINTER section and resolve()'s own docstring to describe the guarantee across all three citation shapes (literal, bare, citing-dir-relative) instead of the literal-only claim that had grown wider than the check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LHBLwqfM1acog2bHRS8WCb
|
Confirmed and fixed — and the finding was better than one bug. Verified before acting. With the pointer set to a nonexistent directory, all eight FAILED lines named sub-path forms ( Worth naming what that means: the docstring sentence promising a broken pointer "is never a silent pass" was wider than the check it described. That is this thread's entire defect class, appearing inside the sentence claiming the class was closed. Fixed in Then your finding generalised. Asked to hunt the same shape rather than the same line, it turned up a second instance: Both fixes measured against a citation form that was the only thing able to catch a broken pointer — dangling, missing, zero non-comment lines and two non-comment lines all exit 1; restored exits 0. One correction to my own verification: my first relative-form test was invalid. The pre-existing sub-path citations in that README masked it, so the FAILED lines it produced were not the ones under test — the same masking you identified as hiding the original bug. Re-run with the scratch citation isolated at a known line, it reports Your note about |
|
@claude re-review please — the code changed since your last pass. You found that What I want from this pass, in priority order:
Note one thing I got wrong, so you can judge the evidence accordingly: my first attempt to verify the relative-form fix was invalid, because pre-existing sub-path citations in that README masked it — the same masking that hid the original bug. I re-ran it with the scratch citation isolated at a known line and it reports FAIL correctly, but treat my verification claims as things to check rather than facts. Do not take the PR description at face value — I wrote it. |
|
Claude finished @basquin-bot[bot]'s task in 4m 26s —— View job Re-review: PR #105 —
|
The review found TODO.md:1293 stale in the docstring added by 793c18e: that same commit inserted ~17 lines earlier in the file, so the citation rotted inside the commit that fixed citation rot. A second line number in the same sentence had moved too. Replaced with the grep that finds the sites, not a list that goes stale. This file is outside check-citations.py's own corpus — it scans *.md, */citations.txt and the '#'-comments of *.sh and workflow files, so a Python docstring's citations have no guard and must not carry line numbers. A figure that cannot drift beats one that is currently right. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LHBLwqfM1acog2bHRS8WCb
|
Independent approver review of PR #105 (dd045-run-of-record, head 5fdcf9e). Reviewed the artifact directly — code, docstring, tracked files, and empirical behavior — not the PR description. Decision: sound. Approving. What I verified1. All four pointer failure modes actually fail, with exit codes captured explicitly (
In every broken case, all 14 tracked citation sites go FAILED — critically including the bare-form sites ( 2. Masking-aware test of the citing-dir-relative branch. No 3. Repoint faithfulness. Recounted the 14 occurrences by diff hunk against 4. Scope boundary + no auto-write. Read 5. Claims vs. code. Traced 6. False debt. The removed allowlist entry ( What I did not verifyDid not run the Full working notes: |
DD-045 item 2 — the cheapest structural fix on the thread, and the only one that removes the cause of citation rot rather than detecting the symptom.
The problem
Verification evidence lives in
bench-results/verify-<UTC timestamp>/. Because the directory name changes every time evidence is regenerated, producing a new run of record stales every citation to the previous one at once. Four supersession commits had to repoint 10, 10, 13 and 18 occurrences (16da079,865ba35,1c3ce88,572282a) — and one miscounted its own repoint:16da079recordsgrep -creporting eight where there were ten, becausegrep -ccounts matching lines, not occurrences.The fix
bench-results/RUN-OF-RECORD, a tracked pointer file naming the current run directory on its single non-comment line.check-citations.py's existingresolve()substitutes a citedbench-results/RUN-OF-RECORD/...prefix with that directory before matching against the tracked tree. A future supersession edits one line instead of hunting occurrences across the tree.Not a symlink. This checkout has
core.symlinks=false, so a tracked symlink materialises on a fresh clone as a plain text file containing its target path. The pointer has to be a real file whose content is read.The harness deliberately does not write it. Promoting a run to run-of-record is a deliberate act; an automatic update would let any ad-hoc local run silently become the cited evidence. The pointer's own comment header states that moving it is manual and belongs in the same commit that adds the new run directory.
Proven able to fail
This edits
check-citations.py— the repo's own citation gate — so a resolver that silently accepted an unresolvable pointer would be the exact "check that cannot fail" defect this thread exists to remove. All four failure modes were broken deliberately and measured:check-citations.pyRUN-OF-RECORD/citations existA broken pointer is emitted as a FAILED DEAD PATH like any other broken citation — never a guess, never a silent pass. I re-ran the dangling case independently of the implementer: 8 FAILED, exit 1, and exit 0 again once restored.
Scope
grep -o … | wc -lrather thangrep -c, per the lesson above.verify-20260730T215842Zremains inTODO.md, deliberately: it names the string that was repointed, in the entry describing this work. It is not a citation.scripts/check-citations-allowlist.txtloses an entry.bench-results/RUN-OF-RECORDhad been exempted as "proposed, not yet built — forward-looking reference"; now that it exists the exemption is false, so removing it puts the path under real checking.The
@claudereview found a real hole in the above, twiceThe failure-mode table was true and still incomplete.
resolve()short-circuited onpath in tracked_setbefore reaching the pointer-substitution branch — and the pointer file is itself tracked. So a barebench-results/RUN-OF-RECORD/citation (trailing slash, nothing after it) stripped to the pointer's own path, matched as an exact file, and returned verified without ever validating the pointer. Confirmed empirically: with the pointer dangling, all eight FAILED lines named sub-path forms and not one of the bare sites was flagged.That shape already existed in this PR's own tree. The docstring sentence promising a broken pointer "is never a silent pass" was therefore wider than the check it described — this thread's defect class, inside the sentence claiming it closed.
Fixed in
793c18e. Then, asked to hunt for the same shape rather than the same line, the fix surfaced a second instance:resolve()'s citing-dir-relative branch checked the normalised path against the tracked tree with no substitution either, so../RUN-OF-RECORD/written from a file underbench-results/took the identical bypass — reachable, sincebench-results/dd043-pr3-restvillains-2026-07-26/README.mdalready cites the run of record and sits one level down. Fixed in4a65bb3, and deliberately not by patching the second branch: both candidates now go through one sharedtracked_or_pointer()helper that checks pointer substitution first and tracked-tree membership second, so the ordering is expressed once instead of drifting across independently-patched branches.resolve()was then read in full to confirm no third site.Both fixes measured, each against a citation form that was the only thing able to catch a broken pointer:
RUN-OF-RECORD/../RUN-OF-RECORD/I re-ran both independently of the implementers. The first attempt at the relative-form test was invalid — the pre-existing sub-path citations in that README masked it, so the FAILED lines it produced were not the ones under test. Re-run with the scratch citation isolated at a known line, it reports
FAIL ...README.md:226: DEAD PATH ../RUN-OF-RECORD/.What this does not establish
resolve()knows about the pointer. The separate pinned-row resolver that reads*/citations.txtdoes not, so aRUN-OF-RECORD/...path written into a citations.txt would be reported DEAD rather than resolved. Moot as written — neither tracked citations.txt names the run of record — and it fails closed, not open. Recorded in the script's docstring rather than left in an implementer's report, because an undocumented gap in a gate reads as coverage.RESULTS.md's own NON-CITABLE stamp remains the check for that.🤖 Generated with Claude Code
https://claude.ai/code/session_01LHBLwqfM1acog2bHRS8WCb