[Review] Adversarial correctness sweep across the whole codebase #440

New issue

Open

opened 2026-08-29 18:17:38 +00:00 by claude-bot · 0 comments

claude-bot commented

2026-08-29 18:17:38 +00:00

Contributor

Part of the v4.6.0 release-readiness gate.

What

A deliberate, hostile read of the entire codebase looking for defects — not a style pass. Multiple independent angles, because single-reviewer sweeps find single-reviewer bugs.

The v4.0.0 acceptance-criteria pass is the model: it went back over 30 issues already marked done and found a dead validator check whose test could not fail, a dedupe that preferred hallucinations over verified beats, an unbounded recursion that killed the test runner, and the default transcription path having no regression coverage at all. Work believed finished was not. That pass paid for itself several times over, and this is the same discipline applied repo-wide.

Angles to cover

Each is a separate reading, not a checklist item:

Correctness under failure — what happens on a partial write, a lost worker, a failed commit after a side effect, a network timeout mid-pipeline
Concurrency — two GMs, two tabs, a retry racing a sweep, Beat tasks overlapping their own previous run
Data lifecycle — anything that deletes, overwrites, or anonymises; every such path re-examined against "is this recoverable, and does the user know"
Trust boundaries — the bot API, OIDC, admin surfaces, anything taking a path or an id from outside
Resource behaviour at scale — unbounded queries, N+1s, unpaginated lists, memory growth on long sessions

Rules for the sweep

Verify by mutation, not by reading. A claimed defect is not confirmed until the code has been changed to prove the behaviour. Several findings in the v4.0.0 pass were only real because they were mutation-tested; at least one plausible hypothesis was refuted by the data.
Cite file and line. A finding without a location is an opinion.
Separate confirmed from suspected, and say which is which.
A finding with no failure scenario is not a finding. State the inputs and the wrong output.

Acceptance criteria

Every backend service, router, task and bot cog has been read adversarially at least once
Findings are filed as individual issues with file:line, a concrete failure scenario, and a confirmed/suspected verdict
Each confirmed finding has a regression test that was run against the unfixed code and seen to fail
Areas reviewed and found correct are recorded too, so the next sweep knows what was already covered and does not re-litigate it
Anything deliberately not filed is written down with the reason, rather than silently dropped

Part of the v4.6.0 release-readiness gate. ## What A deliberate, hostile read of the entire codebase looking for defects — not a style pass. Multiple independent angles, because single-reviewer sweeps find single-reviewer bugs. The v4.0.0 acceptance-criteria pass is the model: it went back over 30 issues already marked done and found a dead validator check whose test could not fail, a dedupe that preferred hallucinations over verified beats, an unbounded recursion that killed the test runner, and the default transcription path having no regression coverage at all. **Work believed finished was not.** That pass paid for itself several times over, and this is the same discipline applied repo-wide. ## Angles to cover Each is a separate reading, not a checklist item: - **Correctness under failure** — what happens on a partial write, a lost worker, a failed commit after a side effect, a network timeout mid-pipeline - **Concurrency** — two GMs, two tabs, a retry racing a sweep, Beat tasks overlapping their own previous run - **Data lifecycle** — anything that deletes, overwrites, or anonymises; every such path re-examined against "is this recoverable, and does the user know" - **Trust boundaries** — the bot API, OIDC, admin surfaces, anything taking a path or an id from outside - **Resource behaviour at scale** — unbounded queries, N+1s, unpaginated lists, memory growth on long sessions ## Rules for the sweep - **Verify by mutation, not by reading.** A claimed defect is not confirmed until the code has been changed to prove the behaviour. Several findings in the v4.0.0 pass were only real because they were mutation-tested; at least one plausible hypothesis was refuted by the data. - **Cite file and line.** A finding without a location is an opinion. - **Separate confirmed from suspected**, and say which is which. - **A finding with no failure scenario is not a finding.** State the inputs and the wrong output. ## Acceptance criteria - [ ] Every backend service, router, task and bot cog has been read adversarially at least once - [ ] Findings are filed as individual issues with file:line, a concrete failure scenario, and a confirmed/suspected verdict - [ ] Each confirmed finding has a regression test that was **run against the unfixed code and seen to fail** - [ ] Areas reviewed and found correct are recorded too, so the next sweep knows what was already covered and does not re-litigate it - [ ] Anything deliberately not filed is written down with the reason, rather than silently dropped

claude-bot referenced this issue

2026-08-29 18:18:37 +00:00

[Review] Simplification pass — make the code as tight as it can honestly be #443

claude-bot added this to the v4.6.0 - Release Readiness: Review, Simplification & Documentation milestone

2026-08-29 18:19:25 +00:00