diff --git a/README.md b/README.md index 061537d..6b50ddc 100644 --- a/README.md +++ b/README.md @@ -185,7 +185,7 @@ prompts/ codex-first-pass.md # Stage 1 prompt (template) claude-synthesize.md # Stage 2 control arm (template) claude-synthesize-thesis-first.md # Stage 2 thesis-first arm (template) - _shared/{completeness,privacy,migration-data,perf,parsing,navigation}-rules.md # shared rule blocks + _shared/{completeness,privacy,migration-data,perf,parsing,navigation,rename-compatibility,spec-value}-rules.md # shared rule blocks ``` - **Templates + shared blocks.** Each prompt references the shared rule blocks via diff --git a/prompts/_shared/spec-value-rules.md b/prompts/_shared/spec-value-rules.md new file mode 100644 index 0000000..278ac3c --- /dev/null +++ b/prompts/_shared/spec-value-rules.md @@ -0,0 +1,24 @@ +A spec is useless if its failure would neither reveal a regression worth fixing nor +point at what to fix. + +Report it only for one of these reasons: +- It asserts the implementation back to itself, such as by stubbing the object under + test and checking the stub, comparing a literal with itself, checking only + `respond_to?` or `be_a`, or checking that a method calls its stubbed collaborator. +- It tests something whose failure would already break louder coverage, such as factory + validity, a widely traversed route, or a framework guarantee. +- It has no assertion tied to its named behavior, so unrelated changes can fail it + without identifying the cause. Request a narrower assertion, not deletion. +- It pins private internals instead of an observable outcome, so safe refactors fail it + while behavior regressions can pass. BiggerPockets does not test private methods. + +Do not flag: +- A spec that catches a plausible bug, however small or obvious the behavior looks. +- A regression spec for a fixed bug. +- Edge, boundary, nil, or error-path coverage, or required flag-enabled and + flag-disabled pairs. Repeated setup is not redundancy. +- Missing coverage. That is a completeness finding. +- A spec the diff only moves, renames, or reindents. + +State the bug the spec fails to catch and the assertion that would catch it. This finding +never blocks a PR by itself. Report it and approve unless another finding blocks. diff --git a/prompts/claude-synthesize-thesis-first.md b/prompts/claude-synthesize-thesis-first.md index c4ce31b..eb7e6f0 100644 --- a/prompts/claude-synthesize-thesis-first.md +++ b/prompts/claude-synthesize-thesis-first.md @@ -46,14 +46,18 @@ Synthesize a single review decision for pull request #{{PR}}. 12. Check whether the diff renames, moves, or deletes a name that is persisted outside the codebase and read back after deploy, per these rules: {{@prompts/_shared/rename-compatibility-rules.md}} -13. Validate which of Codex's findings are real (discard false positives), add any genuine +13. Judge the value of the specs the diff adds or changes, per these rules, and report a + useless spec with a file/line reference and the assertion that would make it fail: + {{@prompts/_shared/spec-value-rules.md}} +14. Validate which of Codex's findings are real (discard false positives), add any genuine issues Codex missed, and (when a ticket is available) judge genuine misses of the ticket's intent or clear scope creep — but give credit when the author went beyond the literal acceptance criteria in a sound way rather than flagging it as non-compliant. -14. Decide ONE verdict. Be pragmatic: use "request_changes" only when there is at least one +15. Decide ONE verdict. Be pragmatic: use "request_changes" only when there is at least one genuine, blocking issue (such as a bug, regression, privacy violation, half-finished task, - placeholder, or deferred work); otherwise "approve". A change that exceeds the AC without - breaking the ticket's intent is a reason to approve, not to block. + placeholder, or deferred work); otherwise "approve". A useless spec is reported, never + blocking. A change that exceeds the AC without breaking the ticket's intent is a reason + to approve, not to block. Write a file named verdict.json in the current working directory with EXACTLY this shape: {"verdict": "approve" | "request_changes", "summary": ""} diff --git a/prompts/claude-synthesize.md b/prompts/claude-synthesize.md index 0ab0416..93fe9f0 100644 --- a/prompts/claude-synthesize.md +++ b/prompts/claude-synthesize.md @@ -46,14 +46,18 @@ Synthesize a single review decision for pull request #{{PR}}. 12. Check whether the diff renames, moves, or deletes a name that is persisted outside the codebase and read back after deploy, per these rules: {{@prompts/_shared/rename-compatibility-rules.md}} -13. Validate which of Codex's findings are real (discard false positives), add any genuine +13. Judge the value of the specs the diff adds or changes, per these rules, and report a + useless spec with a file/line reference and the assertion that would make it fail: + {{@prompts/_shared/spec-value-rules.md}} +14. Validate which of Codex's findings are real (discard false positives), add any genuine issues Codex missed, and (when a ticket is available) judge genuine misses of the ticket's intent or clear scope creep — but give credit when the author went beyond the literal acceptance criteria in a sound way rather than flagging it as non-compliant. -14. Decide ONE verdict. Be pragmatic: use "request_changes" only when there is at least one +15. Decide ONE verdict. Be pragmatic: use "request_changes" only when there is at least one genuine, blocking issue (such as a bug, regression, privacy violation, half-finished task, - placeholder, or deferred work); otherwise "approve". A change that exceeds the AC without - breaking the ticket's intent is a reason to approve, not to block. + placeholder, or deferred work); otherwise "approve". A useless spec is reported, never + blocking. A change that exceeds the AC without breaking the ticket's intent is a reason + to approve, not to block. Write a file named verdict.json in the current working directory with EXACTLY this shape: {"verdict": "approve" | "request_changes", "summary": ""} diff --git a/prompts/codex-first-pass.md b/prompts/codex-first-pass.md index 3004c09..11b8921 100644 --- a/prompts/codex-first-pass.md +++ b/prompts/codex-first-pass.md @@ -40,8 +40,11 @@ verbatim and handed to a second reviewer, so do not add conversational preamble. 11. Check whether the diff renames, moves, or deletes a name that is persisted outside the codebase and read back after deploy, per these rules: {{@prompts/_shared/rename-compatibility-rules.md}} -12. Report concrete issues — bugs, regressions, security problems, member-privacy - violations, incomplete tasks/half-measures/placeholders/deferred work, and genuine misses - of the ticket's intent or clear scope creep — each with a file/line reference and a brief - rationale. Do not list "doesn't match acceptance criteria" as an issue by itself; only +12. Judge the value of the specs the diff adds or changes, per these rules, and report a + useless spec with a file/line reference and the assertion that would make it fail: + {{@prompts/_shared/spec-value-rules.md}} +13. Report concrete issues — bugs, regressions, security problems, member-privacy + violations, incomplete tasks/half-measures/placeholders/deferred work, specs that cannot + fail usefully, and genuine misses of the ticket's intent or clear scope creep — each with + a file/line reference and a brief rationale. Do not list "doesn't match acceptance criteria" as an issue by itself; only raise it when the deviation harms the intent. If nothing is blocking, say so briefly. \ No newline at end of file