docs: the split is for problems this pull request did not cause, and nothing else - #108
Merged
Merged
Conversation
…nothing else My reconciliation of the two rules was backwards, and the maintainer said so. I resolved the contradiction by inventing a maintainer-approved waiver for branch-caused defects -- keeping the escape hatch and only changing who holds the key. The rule is the other way round: Split is permitted for problems this pull request did NOT raise. Problems it did raise are never deferred. No waiver. So the exception now runs opposite to the direction people reach for it: a problem surfaced while working, not caused by this branch, that warrants its own pull request rather than an issue. That may overlap, because the alternative is sitting on it until this one lands. And the scope rule is absolute -- not filed, not split, not waived, no exception at all. Worth naming what I did, because it is subtler than the contradiction Codex caught. Told that two rules conflicted, I preserved the permissive one and narrowed its conditions, which reads as rigour and is not: a "narrow, three condition, maintainer-approved" hatch is still a hatch for exactly the case the strict rule exists to forbid. The conflict was resolved in favour of the half that suited the author. That is the same reflex as #105, one level up. Markdown only; no code, project or workflow file touched, so no build, test or format result is claimed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Zd6AyAtcZApgfRC2Rkitj
PR SummaryLow Risk Overview The Stay inside the task section is tightened to match: branch-caused regressions cannot be deferred; post-merge debt is worked right after the current PR (not dropped mid-PR, not queued last). Out-of-scope tooling findings default to issues; the maintainer may still spin a non-caused item into its own PR. The timing-test anecdote and review-finding guidance are rewritten so causality comes first and the second question for unrelated findings is ticket urgency, not scope creep into the open PR. Also clarifies that cross-package work ships one PR at a time per the single-PR rule. Reviewed by Cursor Bugbot for commit b087366. Bugbot is set up for automated code reviews on this repo. Configure here. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c25dc89919
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…ho owns the call Codex, P1 and correct: inverting the exception left the paragraph below it saying the opposite. It still read "The exception is for findings on the pull request's own content" -- exactly the reading the inversion removes -- so the agreement could still be quoted to justify splitting out a regression the branch caused. Contradiction between paragraphs again, third of this kind here. Rewriting it turned up the more useful point. Under the corrected rule the #105 failure was *not* the misread exception: the macOS test failure genuinely was out of scope, correctly identified as information. What made it work was deciding, mid-task, that it deserved a pull request rather than the issue it should have been. So the anecdote now says that, which is what actually happened, rather than the tidier story I had written. That exposed a hole the inversion had opened. "Warrants its own pull request" had no owner, and an author who may decide it for their own detour has the escape back. The default is now the issue, and the exception is the maintainer wanting a pull request instead -- which is exactly how #105 was in fact handled, by being closed and recorded on #92. Markdown only; no code, project or workflow file touched, so no build, test or format result is claimed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Zd6AyAtcZApgfRC2Rkitj
Asked to re-check the working agreement as a whole. Five things, found by cross-checking rules against each other rather than each against its intent -- which is the pass I had not done, and the class of defect review kept catching. One real conflict. "Once the causing pull request is merged ... it is then the next thing worked, not queued behind whatever else is open" contradicted both one pull request open at a time and finish the pull request in hand first: read literally it says drop what you are doing. It now says what was meant -- next after the pull request in hand, ahead of the rest of the backlog -- and names the two rules it must not breach instead of pointing at them by position. One broken sentence: "What the answer *is* out of scope is everything the tooling merely surfaces" was left garbled by an earlier edit. One promise the text did not keep. "Causality first, then urgency" announced a second question and never said what it decides. It decides ticket priority, not whether the work happens in this pull request -- it does not. Stated now, so the ordering is a rule rather than a slogan. Two juxtapositions that read as contradictions without being one. "Issues in different packages get their own PR" sits directly above "one pull request open at a time" and looked like licence for two at once; it now says one after another. And "The exception above permits that" sits three paragraphs under "This rule has no exception" -- both true, about different cases, and now said so in place. Markdown only; no code, project or workflow file touched, so no build, test or format result is claimed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Zd6AyAtcZApgfRC2Rkitj
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c5519dbf7c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex again, and again correct. The out-of-scope paragraph directed such a problem into an issue unconditionally and required the current pull request to finish first -- while the exception four sections up allows exactly that kind of problem, one the branch did not cause, to become its own overlapping pull request when the maintainer wants it. Two instructions, one case, opposite outcomes. It now reads as default and exception rather than as a competition. Worth recording how this one survived: my own full re-read, done specifically to hunt contradictions, went past it. I checked the paragraphs I had edited against each other. This sentence I had not edited -- it was correct until the exception was inverted two commits earlier, and inverting a rule silently invalidates every sentence that refers to it, including the ones the diff never touches. That is the same causality mistake as the scope rule itself, one level up: the question is not which lines changed, it is which statements the change made false. Markdown only; no code, project or workflow file touched, so no build, test or format result is claimed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Zd6AyAtcZApgfRC2Rkitj
dborgards
pushed a commit
that referenced
this pull request
Sep 13, 2026
Brings in #104, #107 and #108. No conflicts: those touch CLAUDE.md, this adds a new file under docs/reviews/. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Zd6AyAtcZApgfRC2Rkitj
dborgards
pushed a commit
that referenced
this pull request
Sep 13, 2026
…after it The document was written mid-wave and then sat on a branch while #104, #107 and #108 happened. Publishing it unchanged would have put a stale derivation in the repository -- and stale in the worst place: its summary of the rules quoted the one-pull-request rule in the version the maintainer rejected, with an exception for problems the pull request itself caused. Corrected, and extended with what the second half produced, which is the more useful half: Seven Codex findings across #101, #104, #107 and #108, all seven upheld, four of them contradictions *between* paragraphs rather than defects in one. The last of those survived a re-read done specifically to hunt contradictions -- because I checked the paragraphs I had edited against each other, and the offending sentence was one I had not touched. It had been correct until a rule two commits earlier was inverted. That generalises to something worth having: inverting a rule silently invalidates every sentence referring to it, including outside the diff, so the question after a rule change is which statements it made false, not which lines it touched. Which is the same principle this file states for code, applied to prose -- and I had written that principle four commits before failing to apply it here. Also records the merge-race that happened twice (two corrections pushed shortly before a merge and lost with it) and the signal agreed in response, and adds #106 plus the still-unwritten "FERTIG -- mergebar" convention to the open items. Markdown in a directory mkdocs excludes from the site; no code, project or workflow file touched, so no build, test or format result is claimed. Refs #104, #106, #107, #108. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Zd6AyAtcZApgfRC2Rkitj
2 of 9 tasks
dborgards
pushed a commit
that referenced
this pull request
Sep 13, 2026
Two findings from the review of this branch, both caused by extending the chronology from eight rows to eleven without re-reading what referred to it: - The header still said eight merged pull requests while the inventory two paragraphs below listed nine (#93, #96, #97, #98, #100, #101, #104, #107, #108). Corrected to nine. - A blank line between row 8 and row 9 terminated the Markdown table, so rows 9-11 rendered as plain pipe-delimited text. Removed. Two more of the same class that the review did not name: "in jedem der acht Faelle" in the cause section refers to the measurement failures only, so it is now "der ersten acht"; "von den acht Zeilen oben" means the whole table and is now "elf". The section on cross-paragraph contradictions records this occurrence, since the document reproduced the very error class it describes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Zd6AyAtcZApgfRC2Rkitj
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
2 participants
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.
maincurrently carries the wrong rule#107 merged at
72a572d. The correction you dictated —8ebced1— was pushed after that commit and did not land. So the version now onmainsays the opposite of what you decided:mainnow"This rule has no exception"is absent frommain;"maintainer agrees to split"is present. This PR is8ebced1, cherry-picked unchanged — a 12-line swap, nothing new written.What was wrong with my version
Told that two rules contradicted, I kept the permissive one and narrowed its conditions to three, one of them maintainer approval. That reads as rigour and is not: a narrow, approved hatch is still a hatch for exactly the case the strict rule exists to forbid. I resolved the conflict in favour of the half that suited the author — the same reflex as #105, one level up.
The rule as it should read, and now does:
Why this keeps happening, and what would stop it
Twice in a row now a correction has been pushed to a branch shortly before it was merged, and both times the merge took the earlier commit.
9b5248fand9f172d6missed #104;8ebced1missed #107. Reporting the new head has not been enough, because the gap is between my push and your merge, not between us.A mechanism that would actually close it: I keep the pull request in draft while I am still pushing to it, and mark it ready for review only when I consider it final. Then "ready" is a signal rather than a sentence in chat, and a merge cannot pick up a half-finished branch. Say the word and I will work that way from here — or tell me a different signal you would rather use.
Type of change
feat— new behaviour (minor release)fix/perf— bug or performance fix (patch release)docs/test/refactor/chore/ci— no releaseChecklist
dotnet build CanKit.Pro.sln -c Releasesucceeds — not run and not claimed: markdown onlydotnet test CanKit.Pro.sln -c Releasepasses — sameChecked instead:
"maintainer agrees to split"no longer appears,"This rule has no exception"does, fences balanced, no line over 100 columns, and the diff againstmainis exactly this one commit.macos-latestmay be red as on every PR (StartPeriodicSend_SingleFrame_FiresAtConfiguredPeriod, from #101, tracked on #92). It reproduces on the base revision, so it is not this PR's — and under the rule this PR restores, that is the only reason it may be left alone.🤖 Generated with Claude Code
https://claude.ai/code/session_011Zd6AyAtcZApgfRC2Rkitj
Generated by Claude Code