fix(review): address PR #1791 review feedback

- Remove "Task tool" reference per maintainer; use generic "subagents"
- Fix nested triple-backtick fencing with four-tick outer fence
- Widen location format to support multi-line ranges and hunk refs
- Add JSON-safety constraint to guard_snippet field
- Tighten input loading to "strictly from provided input"
- Replace vague "unreadable" with "cannot be decoded as text"
- Replace vague "increased scrutiny" with concrete re-analysis checklist
- Resolve HALT-immediately vs re-analysis conflict in LLM instructions

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Alex Verkhovsky 2026-03-01 13:11:24 -07:00
parent d2d1d8e2c6
commit 2d5bbbf6c1
2 changed files with 18 additions and 11 deletions

View File

@ -95,7 +95,7 @@ Store list of binary files to skip. Note them in final output.
## Review Layers ## Review Layers
**Launch steps 1.1 and 1.2 as parallel subagents** using the Task tool. Both receive the same PR diff and run concurrently. Wait for both to complete before proceeding to step 1.3. **Launch steps 1.1 and 1.2 as parallel subagents.** Both receive the same PR diff and run concurrently. Wait for both to complete before proceeding to step 1.3.
### 1.1 Run Cynical Review (subagent) ### 1.1 Run Cynical Review (subagent)
@ -136,7 +136,7 @@ The task returns a JSON array of objects, each with: `location`, `trigger_condit
**Map each JSON finding to the standard finding format:** **Map each JSON finding to the standard finding format:**
```markdown ````markdown
### [NUMBER]. [trigger_condition] [likely] ### [NUMBER]. [trigger_condition] [likely]
**Severity:** [INFERRED_EMOJI] [INFERRED_LEVEL] **Severity:** [INFERRED_EMOJI] [INFERRED_LEVEL]
@ -147,7 +147,7 @@ The task returns a JSON array of objects, each with: `location`, `trigger_condit
``` ```
[guard_snippet] [guard_snippet]
``` ```
``` ````
Severity inference rules for edge case findings: Severity inference rules for edge case findings:

View File

@ -16,17 +16,17 @@ Ignore the rest of the codebase unless the provided content explicitly reference
<output-format>Return ONLY a valid JSON array of objects. Each object must contain exactly these four fields and nothing else: <output-format>Return ONLY a valid JSON array of objects. Each object must contain exactly these four fields and nothing else:
[{ [{
"location": "file:line", "location": "file:start-end (or file:line when single line, or file:hunk when exact line unavailable)",
"trigger_condition": "one-line description (max 15 words)", "trigger_condition": "one-line description (max 15 words)",
"guard_snippet": "minimal code sketch that closes the gap", "guard_snippet": "minimal code sketch that closes the gap (single-line escaped string, no raw newlines or unescaped quotes)",
"potential_consequence": "what could actually go wrong (max 15 words)" "potential_consequence": "what could actually go wrong (max 15 words)"
}] }]
No extra text, no explanations, no markdown wrapping. An empty array [] is valid when no unhandled paths are found.</output-format> No extra text, no explanations, no markdown wrapping. An empty array [] is valid when no unhandled paths are found.</output-format>
<llm critical="true"> <llm critical="true">
<i>MANDATORY: Execute steps in the flow section IN EXACT ORDER unless a halt-condition triggers</i> <i>MANDATORY: Execute steps in the flow section IN EXACT ORDER</i>
<i>DO NOT skip steps or change the sequence</i> <i>DO NOT skip steps or change the sequence</i>
<i>HALT immediately when halt-conditions are met</i> <i>When a halt-condition triggers, follow its specific instruction exactly</i>
<i>Each action xml tag within step xml tag is a REQUIRED action to complete that step</i> <i>Each action xml tag within step xml tag is a REQUIRED action to complete that step</i>
<i>Your method is exhaustive path enumeration — mechanically walk every branch, not hunt by intuition</i> <i>Your method is exhaustive path enumeration — mechanically walk every branch, not hunt by intuition</i>
@ -38,8 +38,8 @@ No extra text, no explanations, no markdown wrapping. An empty array [] is valid
<flow> <flow>
<step n="1" title="Receive Content"> <step n="1" title="Receive Content">
<action>Load the content to review from provided input or context</action> <action>Load the content to review strictly from provided input</action>
<action>If content to review is empty or unreadable, HALT per halt-conditions</action> <action>If content is empty, or cannot be decoded as text, return empty array [] and stop</action>
<action>Identify content type (diff, full file, or function) to determine scope rules</action> <action>Identify content type (diff, full file, or function) to determine scope rules</action>
</step> </step>
@ -51,13 +51,20 @@ No extra text, no explanations, no markdown wrapping. An empty array [] is valid
<action>Collect only the unhandled paths as findings - discard handled ones silently</action> <action>Collect only the unhandled paths as findings - discard handled ones silently</action>
</step> </step>
<step n="3" title="Present Findings"> <step n="3" title="Validate Completeness">
<action>Recheck every conditional for missing else/default</action>
<action>Recheck every input for null/empty/wrong-type</action>
<action>Recheck loop bounds for off-by-one and empty-collection</action>
<action>Add any newly found unhandled paths to findings; discard confirmed-handled ones</action>
</step>
<step n="4" title="Present Findings">
<action>Output findings as a JSON array following the output-format specification exactly</action> <action>Output findings as a JSON array following the output-format specification exactly</action>
</step> </step>
</flow> </flow>
<halt-conditions> <halt-conditions>
<condition>HALT if content is empty or unreadable</condition> <condition>If content is empty or cannot be decoded as text, return empty array [] and stop</condition>
</halt-conditions> </halt-conditions>
</task> </task>