fix(review): make the code-review second pass unconditional - #937
fix(review): make the code-review second pass unconditional#937tend-agent wants to merge 2 commits into
Conversation
tend-agent
left a comment
There was a problem hiding this comment.
The premise holds up: the removed back-reference did tie the pass's applicability to a scale whose bottom rung is "skim, not the full checklist", and the two bullets below already carry the depth-scaling on their own, so the deleted clause was load-bearing only for the reading you're trying to close. The trivial-skip path in step 1 (Do NOT proceed to steps 2–6) never reaches step 4, so "every review that reaches this step" is the right scope rather than an over-claim, and draft mode still lands here.
One suggestion on placement. The mandate is now sentence three, behind a sentence describing what the skill is — and CLAUDE.md's skill-authoring rule is Lead with the rule or recipe; cut motivation. Since the failure you're fixing is partly one of salience (four sessions never mentioned the skill at all), a rule sitting mid-paragraph after two sentences of description is the shape most likely to get skimmed past. Moving it to the front costs no words and puts the imperative where a scanning reader hits it first.
Since #819 made
/tend-ci-runner:code-reviewreachable again, everytend-reviewsession onnumbagg/numbagghas still skipped the mandated second pass — 4 of 4 reviews that reached step 4, zero counter-examples. The reviews still get posted and still read as complete, so the missing pass is invisible from the outside; only the session logs show it.The reachability bug (#817) is fixed — the skill is in the available-skills listing in every one of these sessions, and no session got a tool-use error. The agent simply never reaches for it, and in all four sessions never even mentions it in its reasoning.
Why the text permits the skip
Step 4 opens with
Scale depth to the change… Don't over-analyze trivial changes.Three lines later the second-pass mandate saysScale its depth to how core the change is, **the same way you scale the manual depth above**. That back-reference ties the pass to a scale whose bottom rung is explicitly "skim, not the full checklist" — so for a lockfile-only dependency bump, "scale it to nothing" is a defensible reading of the instruction as written. The pass is never stated to be unconditional.This change says it is: the depth-scaling sets how deep the pass goes, never whether it happens. The peripheral bullet already names dependency bumps and gives them the short angle set in one pass, so the always-run reading is what the rest of the paragraph already assumes.
Evidence
Run 31365030481. Full log: https://gist.github.com/19b5ab297bb7ac7e1e9a44d595ccde0f
tend-ci-runner:code-reviewfirst appears in a numbagg session's available-skills listing between 2026-08-06 and 2026-08-08 (absent in 31113573735, present in 31246101561). Sessions before that are excluded — the skill was genuinely unreachable, and one of them (31088428562) says so in as many words: "I skipped the/code-reviewpass the workflow calls for. It isn't in this environment's available-skills list." That agent checked, found it missing, and named the deviation — which is the behavior we want, and the reason the four sessions since are worth acting on.code-reviewreachableEach row is a session that read the diff (
gh pr diff) and so reached step 4; none contains aSkillcall fortend-ci-runner:code-review, and none mentions it in assistant reasoning.Gate assessment
Honest limit: the evidence is from one adopter repo, and I can't prove wording alone fixes what is partly a salience problem. If a later cycle finds sessions still skipping the pass with this text in place, the next step is a harder mechanism (a distinct numbered step, or a step-5 pre-post check that the pass ran) rather than more wording.