Add internal-contradictions check as #1 in design review prompt#740
Merged
Conversation
Surface contradictions between two parts of a spec/PRD/task list as the first thing the design reviewer looks for. When two parts of a spec conflict, the implementation cannot satisfy all of it, and downstream agents (codex, claude, etc.) may resolve the conflict differently — producing inconsistent implementations across commits. This was prompted by a real tug-of-war between roborev and codex on a spec contradiction that neither side flagged as the underlying issue. Also updates the corresponding golden file. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
roborev: Combined Review (
|
| 3. **Task scoping**: Are implementation stages small enough to review incrementally? Are dependencies ordered correctly? | ||
| 4. **Missing considerations**: Security, performance, backwards compatibility, error handling | ||
| 5. **Clarity**: Are decisions justified and understandable? | ||
| 1. **Internal contradictions**: Flag places where two parts of the spec, PRD, or task list conflict — even if each is individually clear. Top priority, because downstream agents (codex, claude, etc.) may resolve them differently, producing inconsistent implementations. |
Collaborator
There was a problem hiding this comment.
Suggested change
| 1. **Internal contradictions**: Flag places where two parts of the spec, PRD, or task list conflict — even if each is individually clear. Top priority, because downstream agents (codex, claude, etc.) may resolve them differently, producing inconsistent implementations. | |
| 1. **Internal contradictions**: Flag places where parts of the spec, PRD, or task list conflict — even if each is individually clear. Top priority, because implementers could resolve these differently, resulting in inconsistencies. |
Collaborator
There was a problem hiding this comment.
no need to name implementers, it really shouldn't matter
Contributor
Author
There was a problem hiding this comment.
Agreed on both — applied verbatim in abde8de.
- Dropped "two" since contradictions can span more than two parts of the spec.
- Dropped the named agents; "implementers" is the right level of abstraction here.
mariusvniekerk
requested changes
May 23, 2026
Per @mariusvniekerk's review on kenn-io#740: - "two parts of the spec" → "parts of the spec" (contradictions can span more than two) - "downstream agents (codex, claude, etc.) may resolve them differently, producing inconsistent implementations" → "implementers could resolve these differently, resulting in inconsistencies" (no need to name specific implementers; the concern holds regardless of who/what is implementing) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
roborev: Combined Review (
|
Member
|
LGTM |
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
3 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.
What
Add a new top-priority bullet to the design review prompt that asks the reviewer to flag internal contradictions between two parts of a spec, PRD, or task list.
Why
I hit a case where roborev and codex effectively played tug-of-war over a contradiction between two parts of the spec — each side resolved the conflict differently, and neither flagged the underlying contradiction as the issue. The existing prompt covers Completeness ("are things defined?") and Clarity ("are decisions justified?"), but doesn't explicitly cover the case where two parts are each individually clear and complete, yet contradict each other.
Putting it at position #1 reflects the priority: when a spec contradicts itself, no implementation can satisfy all of it, and downstream agents will diverge in how they resolve the conflict — which compounds across commits.
Changes
internal/prompt/templates/default_design_review.md.gotmpl: new bullet inserted as Addroborev install-hook uninstallto uninstall the hook #1, existing bullets renumbered 2–6.internal/prompt/testdata/golden/design_review.golden: golden updated to match.Verification
🤖 Generated with Claude Code