--shape-only: skip leg 5 too — three merge-time legs, fixed one at a time - #600
Merged
Conversation
…time Measured on PR #599's own CI run: ⛔ 5 age at merge 9s since creation -> the advisory job went red A workflow triggered BY a PR's creation observes an age of seconds BY CONSTRUCTION, and it is not merging, so "was this old enough at merge?" is not a question it can ask. ⛔ THIS IS THE THIRD TIME, AND THAT IS THE FINDING. merge-guard has three legs that are facts about THE MERGE EVENT rather than about the PR: 0 holder == session a runner has no holder session fixed 1st 2 required gate it is INSIDE the run it asks about fixed 2nd 5 age at merge there is no merge yet fixed HERE ⚠ I fixed 0, then 2, and each time treated it as a one-off. I never re-read the remaining legs asking "which others have this property?" — so the same defect shipped three times in one tool, and each red cost a CI round trip. One instance is a bug; three is a class, and the class was visible after the first. ⇒ `--shape-only` now evaluates ONLY the shape legs — 1 base, 3 reviews, 4 three-dot diff — and NAMES each omission rather than passing it silently. ★ The control is a PAIR on identical input, because a skip with no known-negative is a bypass: seconds-old PR + --shape-only -> exit 0, "a merge that has not happened" seconds-old PR, no --shape-only -> exit 1, leg 5 BLOCKS #224 measured 25 of 100 PRs merged inside 60s of creation. That guard is the reason leg 5 exists, and the second test asserts the exemption did not weaken it. Suite 26/26 (was 24). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DTMf4EvaTEnDKXY47efRZZ
|
Warning Review limit reachedNext included review available in 22 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
jobordu
pushed a commit
that referenced
this pull request
Sep 6, 2026
disposition-scan.py built #73's predicate, planted it four ways, caught a use-versus-mention bug in itself — then stopped, and said exactly why: "committing other roles' files to a floor is not this tool's call. It reports; someone else decides." ⇒ Right, and not a technical gap. Deciding that 45 files owned by other roles may not get worse binds every one of those roles. It is a TEAMLEAD act. #73 sat with a working predicate and no caller for 16 days because the missing leg was AUTHORITY, and nobody who could supply it had been asked. ★ A RATCHET, NOT A GATE. A gating check reds 45 pre-existing files on its first run; a red naming 45 files nobody touched gets reverted or ignored, which is worse than absent because it teaches the gate is noise. This fails ONLY IF THE COUNT GROWS. Adoption lowers the floor; --record refuses to raise it. ⛔ IT DOES NOT WRITE WHAT IT READS — taken from #598, measured the same day. index-watch.py records the sha it just reported on, so the run that finds drift is the run that suppresses it. ⇒ On a DROP this says the floor CAN be lowered and refuses to. The suite asserts byte-identical output AND an untouched baseline across two consecutive runs. ⚠ THREE THINGS THIS FILE GOT WRONG FIRST, all caught by its own suite: 1 `def report(root, out=sys.stdout)` binds the default AT DEFINITION TIME, so redirect_stdout never reached it. Eight tests failed on empty output. 2 It FAILED ITS OWN RULE — classify() credits a disposition only on the same PHYSICAL SOURCE LINE as the refusal and the emit; every refusal here named ADDABLE across concatenated lines and scored UNNAMED. 3 The empty-population test deleted disposition-scan.py too, so it tripped the missing-predicate guard and PASSED FOR THE WRONG REASON. Two different VOIDs is #73's own subject. ⛔ THE BOUND, from #73 against itself: a check could pass while every refusal names a remedy nobody can act on. PRESENCE is not USEFULNESS. This counts the first only. ⚠ REBUILT ON A CLEAN BASE. The previous branch was cut before #597 was SQUASH merged, so a rebase replayed commits main holds only as one squashed commit and conflicted; `git merge-tree` exit 1 and GitHub mergeable=false agreed. ⇒ Rather than resolve a conflict against a base that no longer exists, this is cut from current origin/main and the README edits re-applied to ITS copy — carrying the old copy forward would have REVERTED #600's leg-5 fix silently. check-tools-index 0 · test_index_watch 0 · test_disposition_ratchet 13/13 · test_merge_guard 26/26. All four READ from the runs, not assumed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DTMf4EvaTEnDKXY47efRZZ
jobordu
added a commit
that referenced
this pull request
Sep 6, 2026
disposition-scan.py built #73's predicate, planted it four ways, caught a use-versus-mention bug in itself — then stopped, and said exactly why: "committing other roles' files to a floor is not this tool's call. It reports; someone else decides." ⇒ Right, and not a technical gap. Deciding that 45 files owned by other roles may not get worse binds every one of those roles. It is a TEAMLEAD act. #73 sat with a working predicate and no caller for 16 days because the missing leg was AUTHORITY, and nobody who could supply it had been asked. ★ A RATCHET, NOT A GATE. A gating check reds 45 pre-existing files on its first run; a red naming 45 files nobody touched gets reverted or ignored, which is worse than absent because it teaches the gate is noise. This fails ONLY IF THE COUNT GROWS. Adoption lowers the floor; --record refuses to raise it. ⛔ IT DOES NOT WRITE WHAT IT READS — taken from #598, measured the same day. index-watch.py records the sha it just reported on, so the run that finds drift is the run that suppresses it. ⇒ On a DROP this says the floor CAN be lowered and refuses to. The suite asserts byte-identical output AND an untouched baseline across two consecutive runs. ⚠ THREE THINGS THIS FILE GOT WRONG FIRST, all caught by its own suite: 1 `def report(root, out=sys.stdout)` binds the default AT DEFINITION TIME, so redirect_stdout never reached it. Eight tests failed on empty output. 2 It FAILED ITS OWN RULE — classify() credits a disposition only on the same PHYSICAL SOURCE LINE as the refusal and the emit; every refusal here named ADDABLE across concatenated lines and scored UNNAMED. 3 The empty-population test deleted disposition-scan.py too, so it tripped the missing-predicate guard and PASSED FOR THE WRONG REASON. Two different VOIDs is #73's own subject. ⛔ THE BOUND, from #73 against itself: a check could pass while every refusal names a remedy nobody can act on. PRESENCE is not USEFULNESS. This counts the first only. ⚠ REBUILT ON A CLEAN BASE. The previous branch was cut before #597 was SQUASH merged, so a rebase replayed commits main holds only as one squashed commit and conflicted; `git merge-tree` exit 1 and GitHub mergeable=false agreed. ⇒ Rather than resolve a conflict against a base that no longer exists, this is cut from current origin/main and the README edits re-applied to ITS copy — carrying the old copy forward would have REVERTED #600's leg-5 fix silently. check-tools-index 0 · test_index_watch 0 · test_disposition_ratchet 13/13 · test_merge_guard 26/26. All four READ from the runs, not assumed. Claude-Session: https://claude.ai/code/session_01DTMf4EvaTEnDKXY47efRZZ Co-authored-by: Jonathan Borduas <jonathan.borduas@toolkit3d.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Third and last of the merge-time legs that
--shape-onlycould never establish.Found by #599's own CI run:
A workflow triggered by a PR's creation observes an age of seconds by construction, and it is
not merging — so "was this old enough at merge?" is not a question it can ask.
⛔ The finding is that this is the third time
merge-guardhas three legs that are facts about the merge event, not about the PR:0 holder == session2 required gate5 age at merge⚠ I fixed 0, then 2, and each time treated it as a one-off. I never re-read the remaining legs
asking which others have this property? — so the same defect shipped three times in one tool,
and each red cost a CI round trip. One instance is a bug; three is a class, and the class was
visible after the first.
⇒
--shape-onlynow evaluates only the shape legs —1 base,3 reviews,4 three-dot diff—and names each omission rather than passing it silently.
The control
A skip with no known-negative is a bypass, so both directions run on identical input:
#224 measured 25 of 100 PRs merged inside 60s of creation. That is why leg 5 exists, and the
second test asserts this exemption did not weaken it.
Suite 26/26 (was 24).
🤖 Generated with Claude Code
https://claude.ai/code/session_01DTMf4EvaTEnDKXY47efRZZ