Problem
praxis's review-pr flow and its reviewer agents report findings as individual instances anchored to a single file:line. But many real defects are one instance of a broader class -- a naming-convention mismatch, a missing accessibility attribute, doc-code drift, a duplicated pattern. When a reviewer surfaces only the single site it happened to land on, fixing that site leaves the siblings elsewhere in the codebase untouched. The next review round then finds one of those siblings and reports it as a "new" Important finding -- when it is really the same class recurring at a different line.
The result: multi-round reviews feel unbounded. Each round keeps turning up Important findings, not because the code is getting worse, but because we keep fixing instances instead of retiring classes. There are effectively infinite instances but a finite number of defect classes; the review contract optimizes for the former.
Concrete anchor: a recent 6-round frontend review (Jartan-LLC/merlin#299) found, in round 6, three Important findings, each an instance of a class rather than a one-off:
- prop-to-
data-* naming consistency across a ~14-component family (one component diverged; the reviewer cited only that one -- the sweep of the other 13 was left to the human).
- accessibility on scrollable/interactive regions (one container missing
tabindex/role; siblings not enumerated).
- a convention enforced in CI but absent from the doc that states conventions (one doc gap cited; "what other CI-enforced conventions are undocumented?" not asked).
Each finding had a plausible set of siblings the single-instance report never named.
Current Workaround
After each finding, manually ask "what class is this an instance of?" and grep/audit the codebase for other occurrences before fixing. This is ad-hoc, easy to forget, and re-derived by hand every round instead of being part of the reviewer contract.
Proposed Solution / Alternatives
Add a bounded class-sweep step to the review contract: for each confirmed finding at or above a severity threshold (suggest Important+), the reviewer names the general class the finding instantiates, then searches -- changed surface first, wider codebase where cheap -- for other instances of that same class, and reports them together as one grouped finding ("class + all sites") instead of a lone instance.
Sketch:
- Report shape: a finding gains an optional
Class: label and an Other sites: list. review-format.md gets a short note on grouping sibling instances under one finding.
- Boundedness: cap the sweep (top-N sites, changed files first) and log-not-drop when truncated. This is consistent with review-severity's anti-aggregation rule -- a pile of instances of one systemic defect rolls up to one systemic finding, tiered at the underlying bug, never inflated to Critical by count.
- Where it lives (open question): baked into each reviewer agent's prompt, a shared step in the review-pr skill applied after findings return, or a dedicated pass. A shared post-findings step keeps behavior identical across all reviewer categories and avoids each agent re-implementing the sweep.
Explicit non-goal -- keep the iterative multi-round review as-is. Re-running review-pr across rounds is valuable precisely because a fresh pass catches things earlier passes missed; that recurrence stays. Class-sweep is additive: it makes each round retire a whole category, so later rounds spend attention on genuinely new territory rather than re-finding the same class at a new line. The two together should make rounds converge (each retires a class) without capping how many rounds a maintainer chooses to run.
Problem
praxis's review-pr flow and its reviewer agents report findings as individual instances anchored to a single
file:line. But many real defects are one instance of a broader class -- a naming-convention mismatch, a missing accessibility attribute, doc-code drift, a duplicated pattern. When a reviewer surfaces only the single site it happened to land on, fixing that site leaves the siblings elsewhere in the codebase untouched. The next review round then finds one of those siblings and reports it as a "new" Important finding -- when it is really the same class recurring at a different line.The result: multi-round reviews feel unbounded. Each round keeps turning up Important findings, not because the code is getting worse, but because we keep fixing instances instead of retiring classes. There are effectively infinite instances but a finite number of defect classes; the review contract optimizes for the former.
Concrete anchor: a recent 6-round frontend review (Jartan-LLC/merlin#299) found, in round 6, three Important findings, each an instance of a class rather than a one-off:
data-*naming consistency across a ~14-component family (one component diverged; the reviewer cited only that one -- the sweep of the other 13 was left to the human).tabindex/role; siblings not enumerated).Each finding had a plausible set of siblings the single-instance report never named.
Current Workaround
After each finding, manually ask "what class is this an instance of?" and grep/audit the codebase for other occurrences before fixing. This is ad-hoc, easy to forget, and re-derived by hand every round instead of being part of the reviewer contract.
Proposed Solution / Alternatives
Add a bounded class-sweep step to the review contract: for each confirmed finding at or above a severity threshold (suggest Important+), the reviewer names the general class the finding instantiates, then searches -- changed surface first, wider codebase where cheap -- for other instances of that same class, and reports them together as one grouped finding ("class + all sites") instead of a lone instance.
Sketch:
Class:label and anOther sites:list.review-format.mdgets a short note on grouping sibling instances under one finding.Explicit non-goal -- keep the iterative multi-round review as-is. Re-running review-pr across rounds is valuable precisely because a fresh pass catches things earlier passes missed; that recurrence stays. Class-sweep is additive: it makes each round retire a whole category, so later rounds spend attention on genuinely new territory rather than re-finding the same class at a new line. The two together should make rounds converge (each retires a class) without capping how many rounds a maintainer chooses to run.