fix(reviewer): bind failed checks to actionable source evidence

docs/noema-agent-sandbox-plan.md

@@ -51,10 +51,17 @@ The driver returns JSON:
   "findings": [
     {
       "severity": "critical | high | medium | low | info",
+      "priority": "P1 | P2 | P3",
       "path": "relative/path",
       "line": 1,
-      "evidence": "log, SARIF, test, or source reference",
-      "recommendation": "specific fix"
+      "check_name": "exact current-head failed check name | null",
+      "evidence": "log, SARIF, test, source, or other independently checkable reference",
+      "evidence_type": "nearby_implementation | matching_existing_example | cross_file_counterpart | current_official_docs | failed_check_or_log",
+      "observable_impact": "specific user or operator consequence",
+      "trigger": "concrete condition that exposes the issue",
+      "recommendation": "smallest specific fix",
+      "regression_command": "one exact single-line command or test target",
+      "suggested_diff": "optional replacement text | null"
     }
   ],
   "suggested_patch_ref": "optional artifact path or branch",
@@ -63,6 +70,22 @@ The driver returns JSON:
 }
 ```
 
+`check_name` is optional for ordinary source, SARIF, dependency, and review-thread
+findings. When a finding is offered as the causal RCA for a failed current-head
+check, it must equal that exact check name. A failed check remains `blocked`
+unless it has its own blocking-severity finding on a current-head changed path
+with a positive source line; one finding cannot authorize multiple failed
+checks.
+
+Every finding is actionable data rather than prose-only advice. Priority,
+evidence type, observable impact, trigger, smallest fix, and an exact regression
+command are required. A `regression_command` cannot contain a newline or Markdown
+backtick. `suggested_diff` is optional, but when present it cannot contain a
+Markdown fence and must anchor to a right-side line in the exact PR diff before
+publication. Valid replacement text is published through GitHub's inline review
+`comments` payload as a suggestion rather than only being displayed in the
+top-level review body.
+
 Noema-issued installation tokens are used only after the sandboxed agent has a
 bounded verdict to publish. The token scope is limited to the target repository
 and central review workflow permissions.
@@ -150,6 +173,12 @@ failure and blocks strict approval.
   a failure came from missing evidence, dependency vulnerability, image
   verification, image vulnerability, CodeGraph failure, sandbox timeout,
   attestation creation/verification, model exhaustion, or GitHub API rejection.
+- Each ordinary failed current-head check either has its own exact-name,
+  changed-path, positive-line blocking RCA or keeps the verdict `blocked`;
+  another failed check's finding cannot satisfy that evidence requirement.
+- Each finding carries priority, evidence type, observable impact, trigger,
+  smallest fix, and one exact regression command; any proposed replacement text
+  must be fence-safe and exact-diff-anchorable before GitHub receives it.
 - Medium-or-higher dependency and sandbox-image findings from OSV, Trivy, and
   dependency-review are remediated by package/image bump or source change, not
   by gate weakening.
@@ -186,10 +215,12 @@ privileged publication plane.
 
 The judgement plane is implemented as the Python package
 `reviewer/noema_reviewer` (a PydanticAI `ReviewAgent` driver). It returns the
-JSON verdict contract above, enforces strict-evidence blocking and
-MEDIUM-or-higher dependency downgrade around the model, preserves reviewed PR
-comments and current check conclusions, records containerized CodeGraph status,
-and publishes only against the live exact head after attested manifest
-verification. The Noema Worker (`src/`) remains the token-exchange boundary
-only. Reviewer code ships with 100% line and branch coverage and 100% docstring
-coverage; the Worker release gate remains `npm run release:verify`.
+JSON verdict contract above, enforces strict-evidence blocking, exact per-check
+failed-check RCA binding, actionable finding validation, exact-diff suggestion
+anchoring, and MEDIUM-or-higher dependency downgrade around the model. It
+preserves reviewed PR comments and current check conclusions, records
+containerized CodeGraph status, and publishes only against the live exact head
+after attested manifest verification. The Noema Worker (`src/`) remains the
+token-exchange boundary only. Reviewer code is required to retain 100% line and
+branch coverage and 100% docstring coverage; the Worker release gate remains
+`npm run release:verify`.

reviewer/README.md

@@ -17,19 +17,46 @@ Division of responsibility:
 
 ## Contract
 
-The verdict shape is the JSON contract from the sandbox plan:
+The verdict shape is the JSON contract from the sandbox plan. Each finding
+carries structured actionability rather than relying on free-form prose:
 
 ```json
 {
   "verdict": "approve | request_changes | blocked",
   "summary": "…",
-  "findings": [{"severity": "critical|high|medium|low|info", "path": "…", "line": 1, "evidence": "…", "recommendation": "…"}],
+  "findings": [{
+    "severity": "critical|high|medium|low|info",
+    "priority": "P1|P2|P3",
+    "path": "…",
+    "line": 1,
+    "check_name": "exact failed check name | null",
+    "evidence": "…",
+    "evidence_type": "nearby_implementation|matching_existing_example|cross_file_counterpart|current_official_docs|failed_check_or_log",
+    "observable_impact": "…",
+    "trigger": "…",
+    "recommendation": "smallest fix",
+    "regression_command": "one exact single-line command",
+    "suggested_diff": "optional replacement text | null"
+  }],
   "suggested_patch_ref": null,
   "blocked_reasons": [],
   "confidence": "high | medium | low"
 }
 ```
 
+`check_name` is optional for ordinary source, SARIF, dependency, and review-thread
+findings. A finding offered as the RCA for a failed current-head check must bind
+to that exact check name. The deterministic gate requires each ordinary failed
+check to have its own blocking-severity finding on a current-head changed path
+with a positive line; one unrelated or differently bound finding cannot clear
+another failed check.
+
+`regression_command` cannot contain newlines or Markdown backticks. A
+`suggested_diff` cannot contain a Markdown fence and is accepted only when its
+`path:line` is a right-side anchor in the exact PR diff. Accepted replacement
+text is sent through GitHub's inline review `comments` payload as a suggestion,
+not merely printed in the top-level review body.
+
 The following guarantees are enforced deterministically around the LLM
 (`gating.py`), so they hold regardless of what the model says:
 
@@ -70,7 +97,7 @@ The following guarantees are enforced deterministically around the LLM
    a repository probe. The primary explore query preserves each selected changed
    path in full instead of truncating individual path identities; it admits at
    most 80 changed files and 24,079 aggregate characters. The manifest retains
-   bounded current-head file content for every selected file through that same
+   bounded current-head file context for every selected file through that same
    80-file canonical scope; above 80 files both semantic scope and changed-file
    context fail closed rather than reviewing a historical 12-file prefix.
    Exceeding either exact-scope budget fails closed instead of querying a prefix.
@@ -91,40 +118,46 @@ The following guarantees are enforced deterministically around the LLM
    unchanged lookalike path become a retrieval seed. The node output never
    counts as review evidence by itself; deleted, unresolved, symlinked-component,
    unindexed, or symbol-less paths leave the original empty result fail closed.
-   The local host-process CodeGraph fallback also builds a closed execution
-   environment instead of copying the parent environment: only `PATH` and locale
-   discovery variables may be propagated; `HOME`, `TEMP`, `TMP`, and `TMPDIR`
-   are replaced by one fresh per-command private temporary directory and
-   `NO_COLOR=1` is set explicitly. Process injection, host user configuration/
-   credentials, ambient temporary-directory capabilities, credential-helper/
-   socket, container/Kubernetes, proxy, arbitrary workflow, and provider
-   variables such as `NODE_OPTIONS`, `GIT_ASKPASS`, `SSH_AUTH_SOCK`,
+   The local host-process CodeGraph fallback builds a closed execution
+   environment instead of copying the parent environment: only `PATH` and
+   locale discovery variables may be propagated; `HOME`, `TEMP`, `TMP`, and
+   `TMPDIR` are replaced by one fresh per-command private temporary directory
+   and `NO_COLOR=1` is set explicitly. Process injection, host user
+   configuration/credentials, ambient temporary-directory capabilities,
+   credential-helper/socket, container/Kubernetes, proxy, arbitrary workflow,
+   and provider variables such as `NODE_OPTIONS`, `GIT_ASKPASS`, `SSH_AUTH_SOCK`,
    `DOCKER_CONFIG`, `KUBECONFIG`, and `HTTPS_PROXY` are not ambient CodeGraph
-   authority. Production central review still uses the separately attested no-
-   network sandbox; this host fallback does not replace that isolation boundary.
-   The production `DockerCodeGraphRunner` now owns the same semantic wrapper and
-   passes both the exact symbol probe and any symbol-seeded second `explore`
-   through its verified no-network container boundary. It extracts only the
-   trusted sandbox copy receipt and sole explore stdout section before semantic
-   classification, so setup/status bytes cannot satisfy the strict gate and an
-   empty production explore cannot silently fall back to a host CodeGraph
+   authority. Production central review still uses the separately attested
+   no-network sandbox; this host fallback does not replace that isolation
+   boundary. The production `DockerCodeGraphRunner` owns the same semantic
+   wrapper and passes both the exact symbol probe and any symbol-seeded second
+   `explore` through its verified no-network container boundary. It extracts
+   only the trusted sandbox copy receipt and sole explore stdout section before
+   semantic classification, so setup/status bytes cannot satisfy the strict gate
+   and an empty production explore cannot silently fall back to a host CodeGraph
    process.
 2. **MEDIUM-or-higher dependency findings can't ride out on an approve.** An
    unresolved OSV/Trivy/dependency-review finding at MEDIUM+ downgrades an
    approval to `request_changes` with the finding attached — the org rule is
    "remediate by bump, not gate weakening".
-3. **Current-head failures remain blocking.** Failed GitHub Checks and
-   MEDIUM-or-higher code-scanning/SARIF alerts deterministically downgrade an
-   approval and retain their exact job, rule, path, and bounded log evidence.
-4. **Reviewer independence cannot deadlock.** The exact reviewer check names
+3. **Current-head failures remain blocking until causally mapped.** Every
+   ordinary failed GitHub Check remains `blocked` unless its exact check name is
+   bound to its own current-head changed-file, positive-line blocking RCA.
+   Check-run names or workflow URLs are not synthesized into source findings.
+   MEDIUM-or-higher code-scanning/SARIF alerts remain deterministic findings.
+4. **Suggestions must be executable review artifacts.** Suggested replacement
+   text is rejected before publication if GitHub cannot attach it to the exact
+   right side of the reviewed diff; fence injection and multiline regression
+   commands fail schema validation.
+5. **Reviewer independence cannot deadlock.** The exact reviewer check names
    `noema-review` and `opencode-review`, plus the downstream
-   `metadata-only gate evaluation`, are excluded from Noema's deterministic
-   failed-check gate because they cannot be prerequisites for the review that
-   produces them. This cycle exception cannot satisfy strict evidence by itself:
-   at least one current-head check outside that reviewer-dependent set must be
-   observed. Similarly named checks remain blocking, as do every other failed
-   check and unresolved non-outdated inline thread.
-5. **Long reviews stay useful.** The production provider request timeout
+   `metadata-only gate evaluation`, are excluded from Noema's failed-check RCA
+   gate because they cannot be prerequisites for the review that produces them.
+   This cycle exception cannot satisfy strict evidence by itself: at least one
+   current-head check outside that reviewer-dependent set must be observed.
+   Similarly named checks remain blocking, as do every other failed check and
+   unresolved non-outdated inline thread.
+6. **Long reviews stay useful.** The production provider request timeout
    defaults to 5,400 seconds and provider 429/5xx responses receive bounded SDK
    retries. Production failover belongs inside `contextual-orchestrator`; Noema
    does not sequentially try the next model. Publication re-reads the live PR
@@ -133,8 +166,11 @@ The following guarantees are enforced deterministically around the LLM
 The GitHub manifest fetch covers all inline review threads (including resolved
 and outdated state), submitted review bodies, conversation comments, failed
 current-head workflow logs, current-head code-scanning alerts, and open
-Dependabot package advisories. Evidence-fetch errors are part of the manifest,
-not silent empty lists.
+Dependabot package advisories. Failed-check log collection derives an Actions
+Job id only from an exact repository-bound GitHub `details_url`; a Check Run id
+is never reused as a Job id. If the Actions log cannot be obtained, collection
+falls back to the same Check Run's bounded annotations. Evidence-fetch errors
+are part of the manifest, not silent empty lists.
 
 The driver sits behind the small `ReviewAgent` protocol, so the sandbox plan's
 "Codex, OpenCode, PydanticAI, or another driver" swap is a one-line change.

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

@@ -30,9 +30,16 @@
     "regressions from that evidence only. Approve when no blocking issue is "
     "supported by the evidence. Use request_changes only for concrete, "
     "evidence-backed blocking issues, and cite the log, SARIF, test, or source "
-    "line for each finding. Use blocked when required evidence is missing rather "
-    "than guessing. Never approve while an unresolved MEDIUM-or-higher "
-    "dependency finding is present; require a package bump instead."
+    "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 "
+    "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 "
+    "MEDIUM-or-higher dependency finding is present; require a package bump instead."
 )