feat(reviewer): enforce actionable finding contract

reviewer/noema_reviewer/__init__.py

@@ -12,7 +12,7 @@
 
 from .agent import PydanticAIReviewAgent, ReviewAgent, build_agent
 from .manifest import ReviewManifest
-from .models import Confidence, Finding, ReviewVerdict, Severity, Verdict
+from .models import Confidence, EvidenceType, Finding, Priority, ReviewVerdict, Severity, Verdict
 from .patch_image_validation import (
     DockerPatchValidatorImageRunner,
     PatchValidatorImageProfile,
@@ -35,6 +35,7 @@
     "Confidence",
     "DockerPatchValidationRunner",
     "DockerPatchValidatorImageRunner",
+    "EvidenceType",
     "Finding",
     "PatchValidationProfile",
     "PatchValidationRequest",
@@ -45,6 +46,7 @@
     "PatchValidatorImageResult",
     "PatchValidatorImageStatus",
     "PydanticAIReviewAgent",
+    "Priority",
     "ReviewAgent",
     "ReviewManifest",
     "ReviewVerdict",

reviewer/noema_reviewer/agent.py

@@ -32,8 +32,10 @@
     "evidence-backed blocking issues, and cite the log, SARIF, test, or source "
     "line for each finding. For every failed check, read its current-head log or "
     "annotation, trace the failure to an exact repository path and positive line, "
-    "set finding.check_name to that exact current-head check name, and state the "
-    "root cause, smallest fix, and regression test in the finding; one finding "
+    "set finding.check_name to that exact current-head check name, and state "
+    "P1/P2/P3 priority, evidence type, observable impact, trigger, smallest fix, "
+    "and an exact regression command in the finding. Include minimal replacement "
+    "text in suggested_diff when the cited line can be fixed directly; one finding "
     "must not stand in for multiple failed checks. A check name, workflow URL, or "
     "synthetic .github/checks path is not actionable. Use blocked when logs cannot "
     "support that mapping rather than guessing. Never approve while an unresolved "

reviewer/noema_reviewer/gating.py

@@ -15,11 +15,15 @@
 
 from __future__ import annotations
 
+import re
+
 from .manifest import ReviewManifest
 from .models import (
     BLOCKING_SEVERITIES,
     Confidence,
+    EvidenceType,
     Finding,
+    Priority,
     ReviewVerdict,
     Severity,
     Verdict,
@@ -34,6 +38,42 @@
 REVIEW_DEPENDENT_CHECK_NAMES = frozenset(
     {"opencode-review", "metadata-only gate evaluation"}
 )
+HUNK_HEADER_RE = re.compile(r"^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@")
+
+
+def _right_side_diff_lines(diff: str) -> set[tuple[str, int]]:
+    """Return right-side path/line anchors accepted by GitHub review comments."""
+    anchors: set[tuple[str, int]] = set()
+    path: str | None = None
+    line_number: int | None = None
+    for line in diff.splitlines():
+        if line.startswith("+++ b/"):
+            path = line[6:]
+            line_number = None
+            continue
+        hunk = HUNK_HEADER_RE.match(line)
+        if hunk:
+            line_number = int(hunk.group(1))
+            continue
+        if path is None or line_number is None or not line:
+            continue
+        if line[0] in {" ", "+"}:
+            anchors.add((path, line_number))
+            line_number += 1
+        elif line[0] != "-":
+            line_number = None
+    return anchors
+
+
+def invalid_suggestion_reasons(manifest: ReviewManifest, verdict: ReviewVerdict) -> list[str]:
+    """Reject suggestions GitHub cannot attach to this exact PR diff."""
+    anchors = _right_side_diff_lines(manifest.diff)
+    return [
+        "suggested diff is not anchored to a current-head right-side diff line: "
+        f"{finding.path}:{finding.line or 'missing'}"
+        for finding in verdict.findings
+        if finding.suggested_diff and (finding.path, finding.line) not in anchors
+    ]
 
 CODEGRAPH_EXPLORE_MARKER = "## codegraph explore"
 
@@ -154,12 +194,17 @@ def dependency_findings_as_review(manifest: ReviewManifest) -> list[Finding]:
         findings.append(
             Finding(
                 severity=dependency.severity,
+                priority=Priority.P1 if dependency.severity is Severity.CRITICAL else Priority.P2,
                 path=dependency.package_name,
                 evidence=(
                     f"{dependency.tool} reported {dependency.package_name}"
                     f"@{dependency.installed_version or 'current'}{identifier}"
                 ),
+                evidence_type=EvidenceType.FAILED_CHECK,
+                observable_impact="The pull request would retain a known vulnerable dependency.",
+                trigger="Installing the dependency set recorded by the current lockfile.",
                 recommendation=f"Bump {dependency.package_name} to {fixed} and refresh the lockfile.",
+                regression_command="uv run pip-audit",
             )
         )
     return findings
@@ -174,13 +219,18 @@ def security_findings_as_review(manifest: ReviewManifest) -> list[Finding]:
         findings.append(
             Finding(
                 severity=security.severity,
+                priority=(Priority.P1 if security.severity in {Severity.CRITICAL, Severity.HIGH} else Priority.P2),
                 path=security.path or ".github/code-scanning",
                 line=security.line,
                 evidence=(
                     f"{security.tool} reported {security.identifier}: {security.message}"
                     + (f" ({security.url})" if security.url else "")
                 ),
+                evidence_type=EvidenceType.FAILED_CHECK,
+                observable_impact="The current-head security gate remains failed.",
+                trigger=f"Running the {security.tool} scanner against the current head.",
                 recommendation="Remediate the current-head scanner finding and rerun code scanning.",
+                regression_command="gh pr checks --watch",
             )
         )
     return findings
@@ -223,10 +273,15 @@ def unresolved_threads_as_review(manifest: ReviewManifest) -> list[Finding]:
     return [
         Finding(
             severity=Severity.HIGH,
+            priority=Priority.P1,
             path=comment.path or ".github/review-threads",
             line=comment.line,
             evidence=f"Unresolved review thread by {comment.author}: {comment.body}",
+            evidence_type=EvidenceType.NEARBY_IMPLEMENTATION,
+            observable_impact="The current head retains a reviewer-confirmed defect.",
+            trigger="Merging while the current inline review thread remains unresolved.",
             recommendation="Resolve the cited review thread with a current-head fix or response.",
+            regression_command="gh pr checks --watch",
         )
         for comment in manifest.review_comments
         if comment.kind == "thread" and comment.state == "open"
@@ -301,6 +356,9 @@ def apply_gates(
     The dependency gate always runs so an approval can never bury an unresolved
     MEDIUM-or-higher vulnerability.
     """
+    suggestion_reasons = invalid_suggestion_reasons(manifest, verdict)
+    if suggestion_reasons:
+        return blocked_verdict(suggestion_reasons)
     if strict:
         reasons = missing_evidence(manifest)
         if reasons:

reviewer/noema_reviewer/github_io.py

@@ -697,12 +697,26 @@ def _fetch_codegraph_status(
 
 def render_review_body(verdict: ReviewVerdict, head_sha: str, token_source: str) -> str:
     """Render the PR review body, including the interop marker the central gate detects."""
-    finding_lines = [
-        f"- [{finding.severity.value}] {finding.path}"
-        + (f":{finding.line}" if finding.line else "")
-        + f": {finding.recommendation} ({finding.evidence})"
-        for finding in verdict.findings
-    ] or ["- No blocking findings."]
+    finding_lines: list[str] = []
+    for finding in verdict.findings:
+        location = finding.path + (f":{finding.line}" if finding.line else "")
+        finding_lines.extend(
+            [
+                f"#### [{finding.priority.value}] {location}",
+                f"- Severity: {finding.severity.value}",
+                f"- Evidence type: {finding.evidence_type.value}",
+                f"- Evidence: {finding.evidence}",
+                f"- Observable impact: {finding.observable_impact}",
+                f"- Trigger: {finding.trigger}",
+                f"- Smallest fix: {finding.recommendation}",
+                f"- Regression: `{finding.regression_command}`",
+            ]
+        )
+        if finding.suggested_diff:
+            finding_lines.extend(["", "```suggestion", finding.suggested_diff, "```"])
+        finding_lines.append("")
+    if not finding_lines:
+        finding_lines = ["- No blocking findings."]
     blocked_lines = [f"- {reason}" for reason in verdict.blocked_reasons]
     body = [
         "## Noema PydanticAI review",
@@ -765,6 +779,16 @@ def publish_verdict(
         "commit_id": head_sha,
         "event": event,
         "body": render_review_body(verdict, head_sha, token_source),
+        "comments": [
+            {
+                "path": finding.path,
+                "line": finding.line,
+                "side": "RIGHT",
+                "body": f"```suggestion\n{finding.suggested_diff}\n```",
+            }
+            for finding in verdict.findings
+            if finding.suggested_diff and finding.line
+        ],
     }
     runner(
         ["gh", "api", "-X", "POST", f"repos/{repo}/pulls/{pr_number}/reviews", "--input", "-"],

reviewer/noema_reviewer/models.py

@@ -11,7 +11,7 @@
 
 from enum import Enum
 
-from pydantic import BaseModel, Field, model_validator
+from pydantic import BaseModel, Field, field_validator, model_validator
 
 
 class Verdict(str, Enum):
@@ -40,6 +40,24 @@ class Confidence(str, Enum):
     LOW = "low"
 
 
+class Priority(str, Enum):
+    """Review priority compatible with actionable PR-review conventions."""
+
+    P1 = "P1"
+    P2 = "P2"
+    P3 = "P3"
+
+
+class EvidenceType(str, Enum):
+    """The source that independently supports a finding."""
+
+    NEARBY_IMPLEMENTATION = "nearby_implementation"
+    MATCHING_EXAMPLE = "matching_existing_example"
+    CROSS_FILE_COUNTERPART = "cross_file_counterpart"
+    OFFICIAL_DOCS = "current_official_docs"
+    FAILED_CHECK = "failed_check_or_log"
+
+
 # Severities at or above which an unresolved dependency finding must block an
 # approval (the org rule: remediate MEDIUM-or-higher by bump, never by gate
 # weakening). Ordered worst-first for deterministic comparisons.
@@ -54,6 +72,7 @@ class Finding(BaseModel):
     """A single reviewer-facing issue tied to concrete evidence."""
 
     severity: Severity = Field(description="How serious the issue is.")
+    priority: Priority = Field(description="P1, P2, or P3 review priority.")
     path: str = Field(description="Repository-relative path the issue lives in.")
     line: int | None = Field(
         default=None,
@@ -67,11 +86,47 @@ class Finding(BaseModel):
         ),
     )
     evidence: str = Field(
+        min_length=1,
         description="Log, SARIF, test, or source reference proving the issue is real.",
     )
+    evidence_type: EvidenceType = Field(description="The kind of source evidence supporting the finding.")
+    observable_impact: str = Field(
+        min_length=1,
+        description="The user- or operator-visible failure caused by the issue.",
+    )
+    trigger: str = Field(
+        min_length=1,
+        description="The concrete condition or workflow that exposes the issue.",
+    )
     recommendation: str = Field(
+        min_length=1,
         description="The specific fix the author should apply.",
     )
+    regression_command: str = Field(
+        min_length=1,
+        description="One exact command or test target that verifies the fix.",
+    )
+    suggested_diff: str | None = Field(
+        default=None,
+        max_length=8000,
+        description="Minimal replacement text for a GitHub suggestion block, when possible.",
+    )
+
+    @field_validator("regression_command")
+    @classmethod
+    def require_single_line_command(cls, value: str) -> str:
+        """Keep the published command exact and safe inside inline-code markup."""
+        if any(character in value for character in "\r\n`"):
+            raise ValueError("regression command must be one plain-text command")
+        return value
+
+    @field_validator("suggested_diff")
+    @classmethod
+    def reject_suggestion_fence_injection(cls, value: str | None) -> str | None:
+        """Prevent model output from escaping the GitHub suggestion fence."""
+        if value is not None and "```" in value:
+            raise ValueError("suggested diff cannot contain a Markdown fence")
+        return value
 
 
 class ReviewVerdict(BaseModel):

reviewer/tests/test_agent.py

@@ -87,7 +87,8 @@ def test_build_prompt_includes_all_sections() -> None:
     assert "SARIF summary:" in prompt
     assert "Workflow log excerpts:" in prompt
     assert "exact repository path and positive line" in SYSTEM_PROMPT
-    assert "root cause, smallest fix, and regression test" in SYSTEM_PROMPT
+    assert "P1/P2/P3 priority" in SYSTEM_PROMPT
+    assert "exact regression command" in SYSTEM_PROMPT
     assert "Prior review comments:" in prompt
     assert "Changed-file context:" in prompt
 

reviewer/tests/test_failed_check_causal_binding.py

@@ -4,7 +4,7 @@
 
 from noema_reviewer.gating import apply_gates
 from noema_reviewer.manifest import ChangedFile, CheckConclusion, ReviewManifest
-from noema_reviewer.models import Finding, ReviewVerdict, Severity, Verdict
+from noema_reviewer.models import EvidenceType, Finding, Priority, ReviewVerdict, Severity, Verdict
 
 
 def _manifest(*check_names: str) -> ReviewManifest:
@@ -28,11 +28,16 @@ def _finding(*, check_name: str | None) -> Finding:
     """Build one otherwise-actionable source finding for failed-check tests."""
     return Finding(
         severity=Severity.HIGH,
+        priority=Priority.P1,
         path="a.py",
         line=1,
         check_name=check_name,
         evidence="current-head log reports the failing assertion at a.py:1",
+        evidence_type=EvidenceType.FAILED_CHECK,
+        observable_impact="The current-head check fails.",
+        trigger="Running the bound check.",
         recommendation="Fix the regression and retain this assertion as a test.",
+        regression_command="uv run pytest reviewer/tests/test_failed_check_causal_binding.py",
     )