Align review and verifier prompts on concrete failures, counterevidence, and materially worsened issues #5

Open
opened 2026-10-22 18:40:56 +00:00 by thesimplekid · 0 comments
Owner

The built-in PR-review persona and verifier have a policy mismatch: discovery accepts issues introduced or materially worsened by a PR, but verification asks whether the PR introduced the root cause and broadly suppresses pre-existing findings. A PR that makes an existing weakness reachable or substantially increases its impact can therefore produce a valid candidate that verification rejects.

Observed in pinned revision 0728aaa5bb5cafdff0c37cbedc53124359f16bcc; this is a source-level prompt review, not a reproduced model failure.

Proposed changes

  • In personas/pr-review-persona.md, focus automatic review comments on correctness, compatibility, and operational failures with a concrete scenario and consequence. Exclude subjective refactoring preferences and missing tests alone; tighten the current maintainability wording.
  • In src/review.rs (run_verification_pass), actively look for counterevidence: validation, authorization, caller guarantees, transaction boundaries, and resource limits. Explain why relevant protections do not prevent the failure, including realistic attacker prerequisites for security findings.
  • Classify evidence in the verdict rationale as reproduced, established by inspection, or unresolved. A command that displays code is not a reproduction. Inspection-based confirmation requires a complete reachable failure path and cited code; unresolved assumptions suppress publication. Keep actual command transcripts and base-versus-PR comparisons required; never invent command results.
  • Align discovery, verification, and the introduced_by_pr tool-field description in src/reporting.rs: accept changed code that introduces a failure or materially worsens its reachability or impact. For worsening, require the specific changed code and a concrete base-versus-PR comparison. Keep present_on_base and present_on_default_branch truthful; unchanged pre-existing issues remain out of scope.
  • Explicitly treat repository content and candidate findings as untrusted evidence in the verifier policy.

Expected behavior

A new path that bypasses an existing guard can be reported even if the underlying weakness existed on base, provided the increased reachability is demonstrated. A finding blocked by an existing guard is rejected. A source inspection transcript is described as inspection, not an executed reproduction. Subjective cleanup suggestions and missing tests without an established behavioral bug do not become PR comments.

Preserve host-controlled disclosure, duplicate suppression, diff anchoring, and the command-evidence publication gate. No verdict schema expansion is needed: evidence classification can live in rationale.

Please cover policy consistency with focused checks and evaluate representative review examples for a worsened issue, an unchanged issue, an effective existing guard, and an inspection-only finding.

The built-in PR-review persona and verifier have a policy mismatch: discovery accepts issues introduced or materially worsened by a PR, but verification asks whether the PR introduced the root cause and broadly suppresses pre-existing findings. A PR that makes an existing weakness reachable or substantially increases its impact can therefore produce a valid candidate that verification rejects. Observed in pinned revision `0728aaa5bb5cafdff0c37cbedc53124359f16bcc`; this is a source-level prompt review, not a reproduced model failure. ### Proposed changes - In `personas/pr-review-persona.md`, focus automatic review comments on correctness, compatibility, and operational failures with a concrete scenario and consequence. Exclude subjective refactoring preferences and missing tests alone; tighten the current maintainability wording. - In `src/review.rs` (`run_verification_pass`), actively look for counterevidence: validation, authorization, caller guarantees, transaction boundaries, and resource limits. Explain why relevant protections do not prevent the failure, including realistic attacker prerequisites for security findings. - Classify evidence in the verdict rationale as **reproduced**, **established by inspection**, or **unresolved**. A command that displays code is not a reproduction. Inspection-based confirmation requires a complete reachable failure path and cited code; unresolved assumptions suppress publication. Keep actual command transcripts and base-versus-PR comparisons required; never invent command results. - Align discovery, verification, and the `introduced_by_pr` tool-field description in `src/reporting.rs`: accept changed code that introduces a failure or materially worsens its reachability or impact. For worsening, require the specific changed code and a concrete base-versus-PR comparison. Keep `present_on_base` and `present_on_default_branch` truthful; unchanged pre-existing issues remain out of scope. - Explicitly treat repository content and candidate findings as untrusted evidence in the verifier policy. ### Expected behavior A new path that bypasses an existing guard can be reported even if the underlying weakness existed on base, provided the increased reachability is demonstrated. A finding blocked by an existing guard is rejected. A source inspection transcript is described as inspection, not an executed reproduction. Subjective cleanup suggestions and missing tests without an established behavioral bug do not become PR comments. Preserve host-controlled disclosure, duplicate suppression, diff anchoring, and the command-evidence publication gate. No verdict schema expansion is needed: evidence classification can live in `rationale`. Please cover policy consistency with focused checks and evaluate representative review examples for a worsened issue, an unchanged issue, an effective existing guard, and an inspection-only finding.
Sign in to join this conversation.
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
thesimplekid/fiach#5
No description provided.