fix(claude-lane-incident): state the real counting condition, drop the false recovery promise, and gate the body budget at the shape that binds - #369
Conversation
…o meet, and stop promising recovery where there is none
The counting rule named two of the three things `coverageGap` gates on. When the
tracked index names nothing both of those read as satisfied — polling the empty
set, an empty index accounting for zero seen — so the copy told an operator cycles
were counting exactly while the gate withheld every one of them. Reachable, not
theoretical: a hand-edited `"repositories":{}` block parses, and the gate returns
`namesNothing`. The rule now leads with the index naming repositories at all, and
says why that clause is there rather than leaving it to look redundant.
The permanent-hold list promised each cause would be "closed by hand once you have
confirmed recovery". Two of the three have nothing to recover — a repository that
is gone is not coming back, and an unreadable index is a corrupted record, not an
outage — and the earlier fix widened the third to any hand-edited or older-schema
index, so the promise covered strictly more causes it could not keep. It now closes
on the failure being resolved or no longer applying, and says outright that two
leave nothing to recover.
The pinned counting-rule string moves with the copy in the same commit. That seam
is the design: the pin exists so copy cannot change without someone re-reading it
against the gate assertions beside it.
Closes #360
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Claude finished @kyle-sexton's task in 6m 13s —— View job Review complete
Reviewed the two copy fixes in Inline finding: left a comment on Test-coverage gap (no changed line anchors this — it spans both files): the PR's summary states "Both copy fixes asserted against the rendered body at the reachable empty-index state," but only the first copy fix (the counting-rule sentence, item 1 in the description) is actually pinned and asserted — via Everything else checks out:
Verification note: I do not have permission to execute |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count. |
… recover, not two Review caught the new sentence claiming "two of them leave nothing to recover" without saying which two. Checking it against this file's own `coverageGap` docstring, the count is simply wrong: it describes the wider-than-tracked case as one "whose `unlisted` remainder never resolves", the same permanence as the other two. All three leave nothing to recover, so naming a subset was the error, not the ambiguity about which subset. Dropping the count instead of spelling it out. The sentence now says none of the three recovers on its own and closes on the operator having addressed the lane failures behind the incident — true of all three, and consistent with both the docstring and the three permanent clauses that follow it. The same wrong count is in #360's body, which is corrected in a comment there rather than edited away.
…olds for, and plain-word the vacuum sentence Verification found the comment claiming the character budget "always bites before the 40-row cap does" at the longest legal name. Measured: at 40 repositories it does not bite until seven pull requests each - 40x1 through 40x6 render all 40 rows. The assertion is still sound, because the loop only drives shapes where it does hold, so this was prose overclaiming again. Scoped to those shapes, with the counterexample recorded so the next reader does not have to re-derive it. The epilogue sentence also read machine-written: "by vacuum" appears nowhere else in this repo, and "which is why the first one is stated at all" is authoring rationale an operator has no use for. Reworded to say what actually happens - an index that names nothing has nothing to poll and nothing to account for - which also drops the positional "the last two conditions" a reader had to count back through. Plain words rather than the codebase's "vacuously", because every other use of that word is in a comment and this is text an operator reads mid-incident.
1 participant
Summary
Drains all three residuals from #360 — the last of the #350 rework list. Two are production copy in
renderIssueBody; one is the body-budget test that never drove the shape that binds.1. The counting rule was vacuously satisfiable
coverageGapgates on three things. The copy named two: every tracked repository polled, and the index accounting for everything seen. When the tracked index names nothing, both read as satisfied — polling the empty set, an empty index accounting for zero seen — so the copy told the operator cycles were counting exactly while the gate withheld every one.Reachable, not theoretical. A hand-edited
"repositories":{}block parses, and the gate returnsnamesNothing: true:The rule now leads with the condition that was missing, and the sentence after it says why that clause exists rather than leaving it looking redundant beside the other two.
2. "once you have confirmed recovery" promised recovery that does not exist
None of the three permanent-hold causes has anything to recover. A repository that is gone (archived, or removed from the installation) is not coming back; an index this watchdog cannot read is a corrupted record rather than an outage; and this file's own
coverageGapdocstring describes the third as an incident "wider thanMAX_TRACKED_REPOSITORIES, whoseunlistedremainder never resolves". #350's own rework widened the unreadable-index cause, so the promise covered strictly more causes it could not keep.It now says none of the three recovers on its own, and closes on the operator having addressed the lane failures behind the incident — true of all three, and consistent with the docstring.
My first attempt at this got the count wrong, claiming "two of them leave nothing to recover" without saying which two. Review flagged the ambiguity; checking it against the docstring showed there was no exception to name.
f0fbeaddrops the count instead of spelling out a subset that does not exist. #360's body carries the same wrong count and is corrected in a comment there rather than quietly edited.3. The body-budget test never drove the shape that binds
The test drove 40×10, 120×40, and 200×300 at the longest legal name — all roughly 700–1400 characters clear of the limit. A sweep finds the real bound at ~49 characters of headroom.
A fixed tight shape would not have fixed this, and this PR is the proof. The two shapes measured as tightest on
origin/main— 40×20 at 65418 (118 spare) and 54×14 at 65487 (49 spare) — now sit at 63760 and 63672, or 1776 and 1864 characters of headroom, because the budget converts epilogue growth into dropped rows and the binding shape moves. Hardcoding them would have looked like a budget gate while measuring slack — the same inert-fixture failure this lane has been unwinding since #357.So the new test sweeps a 144-shape grid bracketing the tight region (146 ms) and asserts the maximum fits. It also asserts at least one shape lands in the row-dropping regime, because a grid where everything sat comfortable would be proving nothing about the bound.
The existing three-shape test keeps its remainder-semantics role and gains two assertions: that the body fits by dropping rows rather than by happening to be short (fewer rows render than the 40-row cap allows, so the character budget bit), and that rows plus remainder account for every repository the incident holds.
What the budget gate does and does not catch — measured, not assumed
fixedLengthdeductionREMAINDER_LINE_RESERVEThe last row is the important one: copy growth alone cannot breach this bound, because rows drop to pay for it. That is the bound being structural, not the test being weak, and the comments now say so instead of crediting the test with protection it does not provide.
The pin seam worked as designed
The pinned counting-rule string moves with the copy in the same commit. That is what the pin is for — copy cannot change without someone re-reading it against the
coverageGapassertions beside it. Updated deliberately, both sides together.Verification
node --test .github/scripts/claude-lane-incident.test.cjs— 46 pass, 0 fail, the file this PR changes.claude-security-review-fail-closed.test.cjsand 2 in the retry-resolution suite fail locally, and they fail identically atorigin/maincontent, so the cause is this machine (subprocess spawning degraded partway through the session), not this change. Reported rather than papered over; CI on GitHub runners is unaffected and green.origin/main: 65485 of 65536 at 54×9, 51 spare. This branch: 65487 of 65536 at 48×17, 49 spare. Zero over-limit across the swept space on either side.Closes #360
Related