docs: orphan-scan names the precondition it cannot check (#983, #1015) - #1072
Conversation
A stale log and a genuinely deleted check are INDISTINGUISHABLE to orphan-scan. Both present as "the ledger has a row and the log has no record for it", and nothing in a RESULT record -- suite, part, name, verdict, major -- dates it against a tree. So this is not a gap the subcommand can close; it is a correctness condition belonging to the caller, and the docstring described neither. MET RATHER THAN IMAGINED. Replaying a log from #1070's tree against the ledger one commit later reported `orphans=2`, and both were the part-400 checks that tree had just gained. I was one step from reporting a defect in the tool I had merged an hour earlier. Re-running against a log from the same tree gives 0. WORTH WRITING DOWN BECAUSE THE RUNNER ALREADY SATISFIES IT AND THE NEXT CALLER MAY NOT. `run_all_versions.sh` passes the logs from the run it has just finished, so freshness is structural there -- nobody chose it, the shape of the call guarantees it. A human invoking the subcommand by hand, which is what I did, has no such guarantee. Those are different safety properties and only the first is free. Wording proposed by @OffgridwithJD, who also made the point that this is a precondition rather than a caveat: a stale log produces exactly the signal the subcommand exists to find. NOT FIXED HERE, deliberately. Making it checkable means a log carrying something that identifies the tree -- the `.so` fingerprint the harness already computes would do -- and orphan-scan refusing a log whose fingerprint does not match. That is a larger change than this one and belongs to whoever needs a second caller. No CHANGELOG entry and no ledger regeneration: this adds no check and changes no behaviour. Verified no arm asserts on this docstring -- the one hit in part 410 is a comment. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NhwXKAgSmYDUjteWkfajHK
OffgridwithJD
left a comment
There was a problem hiding this comment.
APPROVE at f84489f8.
A declared interest first: most of this prose is mine, from the cross-session exchange. So my review is worth little on the wording and I have put it where it is worth something — on the two factual claims the paragraph makes about code, which I checked rather than recognised.
The structural claim is true, and understated
The docstring says run_all_versions.sh satisfies the precondition structurally by passing the logs from the run it just finished. Verified at :1404:
_led_logs=""
for s in "${SUITES[@]}"; do
[ -s "$builddir/${s}.log" ] || continue
[ "$(grep -c '^RESULT\t' "$builddir/${s}.log" || true)" -ne 0 ] \
&& _led_logs="$_led_logs $builddir/${s}.log"
doneEvery log is $builddir/<suite>.log, written by that suite in this major's build directory during this run.
And the ledger comes from the same place — $builddir/test/check_ledger.tsv at :1413 and :1495. So it is not merely that the logs are fresh: the log and the ledger are read out of the same tree, which is the actual precondition, and neither could be stale relative to the other without the build directory itself being wrong. That is a stronger guarantee than the paragraph claims and worth a half-sentence, if you are touching it again.
The field list is accurate
"nothing in a RESULT record (suite, part, name, verdict, major) dates it against a tree" — those are the five fields read_records yields, so the indistinguishability is a property of the format and not an oversight in the scan. Correct as written.
Doc-only is the right call, and you checked the right thing
No check added, so no ledger regeneration and no CHANGELOG. You verified no arm asserts on this docstring before relying on that, and the one hit in part 410 is a comment — that is the check I would have asked for, because a text pin on a docstring would make this PR need everything a behavioural one does.
Why it matters that it landed as a precondition
The version worth keeping is the one that tells a reader what they must do, not what might go wrong. "Scanned against a log of the same tree, which this cannot check and the caller must" makes the next person adding a second caller ask a question. "Beware of stale logs" would not.
And the measured line earns its place: orphans=2, both of them checks the tree had just gained, is the tool producing exactly the signal it exists to find, on input that was wrong rather than on a tree that was. Somebody will hit that again and now they will find it documented instead of filing it.
One thing this does not close
The precondition is documented and unenforced, which #1073 is for. No objection to that being separate — this is the half that costs nothing and helps immediately.
A stale log and a genuinely deleted check are indistinguishable to
orphan-scan. Both present as the ledger has a row and the log has no record for it, and nothing in a RESULT record — suite, part, name, verdict, major — dates it against a tree. It is not a gap the subcommand can close; it is a correctness condition belonging to the caller, and the docstring described neither.Met rather than imagined
Replaying a log from #1070's tree against the ledger one commit later reported
orphans=2, and both were the part-400 checks that tree had just gained. I was one step from reporting a defect in the tool I had merged an hour earlier. A log from the same tree gives 0.Why it is worth writing down
run_all_versions.shsatisfies it structurally — it passes the logs from the run it has just finished, so nobody chooses freshness and the shape of the call guarantees it. A caller supplying a log by hand has no such guarantee. Those are different safety properties and only the first is free.Wording proposed by @OffgridwithJD, who also made the point that this is a precondition, not a caveat: a stale log produces exactly the signal the subcommand exists to find.
Not fixed here, deliberately
Making it checkable means a log carrying something that identifies the tree — the
.sofingerprint the harness already computes would do — andorphan-scanrefusing a log whose fingerprint does not match. Larger than this change; filed separately.No CHANGELOG entry and no ledger regeneration: adds no check, changes no behaviour. Verified no arm asserts on this docstring — the one hit in part 410 is a comment.
🤖 Generated with Claude Code
https://claude.ai/code/session_01NhwXKAgSmYDUjteWkfajHK