docs: add the working-practice retro for the 12/13 September issue wave - #109
Conversation
The derivation behind the rules added to CLAUDE.md: chronology, the eight claims that turned out to be wrong, and the measurements that settled each one. CLAUDE.md carries what applies; this carries why, for anyone who wants to check the reasoning rather than take the rules on trust. Placed in docs/reviews/ next to the existing review, which mkdocs excludes from the website via exclude_docs, so it is an internal working document and needs no nav entry. Written in German to match the document already in that folder and the reader it is addressed to; CLAUDE.md stays English like the rest of the tree. The uncomfortable parts are in it on purpose. Six public self-corrections, two occasions where the maintainer had to push before anything happened, and a control experiment the wave produced by accident: the periodic test was mutation-checked and held, the claim test was not and was worthless -- same day, same hand. It also separates what was genuine exploration (grid alignment is not resolvable at a 120 ms period against ~50 ms observation jitter, which no amount of care would have predicted) from rework on claims made before measuring, which is all eight rows of the table. Markdown only, in a directory excluded from the site build; no code, project or workflow file touched, so no build, test or format result is claimed. Refs #92, #102, #103. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Zd6AyAtcZApgfRC2Rkitj
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
…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
PR SummaryLow Risk Overview The doc positions itself as the derivation behind those rules (with a distilled checklist and open follow-ups such as #92, #106, and the proposed „FERTIG — mergebar" signal). It lives under Reviewed by Cursor Bugbot for commit f4b12bb. 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: 9e050091f8
ℹ️ 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! |
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
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 008a971e76
ℹ️ 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".
The review found the introduction announcing "zwei echte Fehler im Produktivcode" over a list whose second item describes a wrong test assertion about documented scheduler behaviour. The list is right and the heading was too wide: it now reads two defects, one in production code and one in the suite, and the second bullet says explicitly where the defect sat. Re-reading every summary against the evidence it points at turned up two more of the same shape: - "Was in dieser Welle tatsaechlich Klarheit gebracht hat, war ohne Ausnahme Messung" is contradicted three sections later, where the rule contradictions were found by reading, not by measuring. Scoped to the code half, with the exception named. - "neun oeffentliche Selbstkorrekturen" was not checkable against anything in the document. Replaced by the eleven the chronology below it counts. The section on this error class records both instances found on this branch, and separates them: the count slips are the diff-blind variant already described, while the production-code heading was wrong from the first draft with nothing having changed under it. That widens the rule - a summary gets read against its intent rather than against its evidence on first writing too, not only after an edit. 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: 36eb789753
ℹ️ 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".
The review is right that "elf oeffentlich korrigierte Fehlbehauptungen - die Chronologie unten zaehlt sie einzeln" is false: entry 11 bundles four contradictions, so the table holds fourteen claims in eleven entries. Both numbers are now stated, with the reason for the difference. This came out of the previous commit, which replaced an uncheckable figure with a checkable one and got the checkable one wrong. The section on this error class records it as the fifth instance and what it adds: the correction of a summary is itself a summary and needs the same check against the evidence as the original, or an unsupported number is merely swapped for a false one. 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: 702f98680e
ℹ️ 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".
The review found the overview still saying #109 confirmed the pattern "noch zweimal" while the section it points at had grown to three cases. That is the sixth instance of the class in this document, and the third introduced by a fix for the previous one, which makes the cause clearer than any single case: a summary that repeats a number proved elsewhere goes stale on every change to the evidence, silently, because both places stay internally consistent. So the fix is structural rather than another sync. Every duplicated count is gone: the overview refers to the section instead of counting it, the price paragraph says each chronology row is one corrected claim and the last bundles several, the cause section says "every case above that involved a measurement" instead of naming a row range, and the exploration-versus-rework line says no row rather than eleven rows. The numbers now live once, in the chronology and in the section that analyses it. Also drops the row count from the open-items entry for the remaining clock-dependent tests, which the failure seen on this branch today would have made wrong as well. 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: a0732905d6
ℹ️ 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".
macOS is red on this branch, and it is not this pull request'sRecording it here once, per the rule, rather than fixing it in flight. What is failing. Three completed macOS runs, three different outcomes:
Why it is not this branch's. The diff is one file: What I did instead of fixing them. Both are recorded on #92, the second with an analysis rather than only a log: its collection budget assumes a period of twice the measured send time, and the 600 ms upper clamp removes that premise above a 300 ms send — at the 475 ms measured here the send fills 79 % of the period, so ticks are dropped at a rate the No fix ported. There is a prepared branch for the single-frame sibling ( I re-ran the failed job once; the other legs were still running at the first attempt, so the re-run follows once they finish. This comment stands for the whole macOS situation on this branch — I will not repeat it per head. Generated by Claude Code |
…instances The review is right that "This rule has no exception" above "The exception above permits that" does not belong in the list of inter-paragraph contradictions: c5519db classifies that pair explicitly as a juxtaposition that only reads as one, both statements being true about different cases. Checking the rest of the list against the history rather than against memory found the same problem one entry further up. Two of the three examples were findings of mine, not the review's - 1e2f3e3 and c5519db - so the list evidenced neither the count nor the class it stood for. It now names the four commits whose messages credit the review as the source: a1d4323, 72a572d, e30b632 and b087366, the last being the one the paragraph after it describes. The instances of this error class found while writing the document are no longer numbered. A running count forced two more edits per addition - the introduction and the total - which is exactly the coupling one of those instances establishes a rule against, and it went stale twice in the same session. They are now an unnumbered list, each case self-describing, so adding one costs one edit. 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: 9e64e18246
ℹ️ 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".
The review is right that chronology row 9 contradicted the rule the document derives. It recorded "a second pull request for a problem the docs branch did not cause" as the failure, while the rule summary in the same document says that is exactly what the exception permits when the maintainer wants it. CLAUDE.md states the real failure at its worked example: the out-of-scope identification was correct, and what turned information into work was deciding mid-task that it deserved a pull request rather than the issue that is the default - a call the exception does not hand to the author. The row now says that, so the evidence supports the rule instead of contradicting it. The cause is the same as the previous finding: I summarised from memory of the rule as it stood before c25dc89 inverted it, rather than from the rule as it now reads two pages further down. Recorded in the list of instances. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Zd6AyAtcZApgfRC2Rkitj
What does this change?
Adds
docs/reviews/2026-09-13-working-practice-retro.md— the derivation behind the rules that landed inCLAUDE.mdvia #104, #107 and #108.CLAUDE.mdcarries what applies; this carries why, for anyone who wants to check the reasoning rather than take the rules on trust.Placed next to the existing review, which
mkdocsexcludes from the site viaexclude_docs, so it is an internal working document and needs no nav entry. German, to match the document already in that folder;CLAUDE.mdstays English like the rest of the tree.It was rewritten before opening, because it had gone stale
The document was written mid-wave and then held on a branch while #104, #107 and #108 happened — deliberately, under the one-pull-request rule. Opening it unchanged would have committed a stale derivation, and stale in the worst possible place: its summary of the rules quoted the one-PR rule in the version you rejected, with an exception for problems the pull request itself caused.
Corrected, and extended with what the second half of the wave produced — which turned out to be the more useful half.
The finding that only the second half exposed
Seven Codex findings across #101, #104, #107 and #108. All seven upheld. Four were contradictions between paragraphs rather than defects in any one of them — the document names them by commit rather than by paraphrase, so the claim is checkable:
a1d4323— the section opened with "causality decides" and closed with "only urgency decides whether it happens now";72a572d— the sequencing exception let a branch-caused change too large to fold in become its own pull request, while the scope rule forbade any follow-up for exactly that case;e30b632(P1) — after the exception was inverted, the paragraph below it still read "The exception is for findings on the pull request's own content", i.e. the opposite of the new rule;b087366— the out-of-scope procedure sent such a problem to an issue unconditionally, while the exception four sections above allows it its own pull request.The last is the most telling. You had asked me to re-read the whole file specifically hunting contradictions. I found five. Codex then found a sixth — in a sentence I had not edited, which had been correct until a rule two commits earlier was inverted.
That generalises: 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 — the same principle this repository states for code, applied to prose. I had written that principle four commits before failing to apply it here.
The review of this pull request proved the point again, six more times
Every finding on this branch was upheld, and several were introduced by the fix for the previous one. They are recorded in the document itself, unnumbered, because a running count was one of them. The two worth naming here:
This description carried the superseded version of that list until the branch was finished. Same class, same document, one level up.
Also recorded
9b5248f/9f172d6lost with docs: record the verification rules the last wave was missing, and serialise pull requests #104,8ebced1lost with docs: the two working-agreement commits #104 merged past #107 — both pushed shortly before a merge, both announced in chat, both merged past. Plus the signal agreed in response.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 only, in a directory excluded from the site builddotnet test CanKit.Pro.sln -c Releasepasses — sameChecked instead: the stale rule summary is gone, the branch merges cleanly onto current
main, and the file needs no nav entry becausereviews/is inexclude_docs.macos-latestis red, and it is not this pull request'sThe diff is one markdown file in a directory
mkdocs.ymlexcludes from the site — it reaches no test through any mechanism. Five completed macOS runs on this branch, over identical product code, no two alike:9e05009008a971J1939TpTests.Two_Channels_Sharing_One_Service_…— J1939-TP BAM T1 timeouta073290J1939NodeTests.StartPeriodicSend_MultiFrame_…— 7 of 8 emissions inside an 8.4 s budgetf4b12bbattempt 1IsoTpChannelIntegrationTests.DiscardPendingPdus_…,IsoTpStminTimingTests.Sender_Paces_…,J1939TpTests.Two_Channels_…f4b12bbattempt 2 (re-run)UdsClientTests.TimedOut_Request_Does_Not_Poison_…— disjoint from attempt 1All are recorded on #92, two of them with analysis rather than only a log: the multi-frame budget assumes a period of twice the measured send time, and its 600 ms upper clamp removes that premise above a 300 ms send. The one permitted re-run is spent.
🤖 Generated with Claude Code
https://claude.ai/code/session_011Zd6AyAtcZApgfRC2Rkitj
Generated by Claude Code