feat: Improved the code-reviewer suite so it doesn't need to loop as frequently for failed reviews based on the finding-verifier (which may need many round trips to verify large PRs)
This commit is contained in:
7 files changed
+117
-6
No files matched your search
@@ -56,6 +56,7 @@ settings:
|
|||||||
|
|
||||||
initial_state:
|
initial_state:
|
||||||
project_dir_in: ''
|
project_dir_in: ''
|
||||||
|
verify_retries: 0
|
||||||
diff_spec: auto
|
diff_spec: auto
|
||||||
intent: ''
|
intent: ''
|
||||||
rigor_in: ''
|
rigor_in: ''
|
||||||
@@ -522,6 +523,14 @@ nodes:
|
|||||||
state_updates:
|
state_updates:
|
||||||
verifier_output: '{{output}}'
|
verifier_output: '{{output}}'
|
||||||
fallback: verdict
|
fallback: verdict
|
||||||
|
next: verify_gate
|
||||||
|
|
||||||
|
verify_gate:
|
||||||
|
id: verify_gate
|
||||||
|
type: script
|
||||||
|
description: Local verifier-fault retry — when the verifier completed but its output carries PIPELINE-FAULT markers (or the verifier died), re-run the cheap finding-verifier ONCE instead of letting the fault escalate into a full lane re-run at the gauntlet. Falls through to verdict when clean or out of retries; a gate crash falls forward on the declared edge.
|
||||||
|
script: scripts/verify_gate.py
|
||||||
|
timeout: 60
|
||||||
next: verdict
|
next: verdict
|
||||||
|
|
||||||
verdict:
|
verdict:
|
||||||
|
|||||||
@@ -165,14 +165,32 @@ def main():
|
|||||||
if v.get("id") == "pipeline-fault"
|
if v.get("id") == "pipeline-fault"
|
||||||
or (isinstance(v.get("note"), str) and v["note"].lstrip().startswith("PIPELINE-FAULT:"))
|
or (isinstance(v.get("note"), str) and v["note"].lstrip().startswith("PIPELINE-FAULT:"))
|
||||||
]
|
]
|
||||||
if fv_faults:
|
# parse_fault's sentinel = the verifier verified NOTHING (hard fault, lane-
|
||||||
first = fv_faults[0].get("note")
|
# degrading). Per-finding verify_fault / crash-guard notes = those findings
|
||||||
|
# simply stand UNVERIFIED (soft: proportionate NEEDS-HUMAN attention — a
|
||||||
|
# ~1 h lane re-run to re-check a few findings is the wrong trade, and the
|
||||||
|
# verify_gate node already retried the verifier locally once).
|
||||||
|
hard_fv = [v for v in fv_faults if v.get("id") == "pipeline-fault"]
|
||||||
|
soft_fv = [v for v in fv_faults if v.get("id") != "pipeline-fault"]
|
||||||
|
|
||||||
|
def _fv_detail(entries):
|
||||||
|
first = entries[0].get("note")
|
||||||
detail = first.strip().replace("\n", " ") if isinstance(first, str) else ""
|
detail = first.strip().replace("\n", " ") if isinstance(first, str) else ""
|
||||||
if len(detail) > MAX_DETAIL_CHARS:
|
if len(detail) > MAX_DETAIL_CHARS:
|
||||||
detail = detail[: MAX_DETAIL_CHARS - 1] + "…"
|
detail = detail[: MAX_DETAIL_CHARS - 1] + "…"
|
||||||
|
return detail
|
||||||
|
|
||||||
|
soft_notes = []
|
||||||
|
if hard_fv:
|
||||||
faults.append(
|
faults.append(
|
||||||
f"PIPELINE-FAULT: finding verification degraded — {len(fv_faults)} verifier "
|
"PIPELINE-FAULT: finding verification degraded — the verifier verified "
|
||||||
f"verdict(s) carry a pipeline fault ({detail})"
|
f"nothing ({_fv_detail(hard_fv)})"
|
||||||
|
)
|
||||||
|
if soft_fv:
|
||||||
|
soft_notes.append(
|
||||||
|
f"Verifier: {len(soft_fv)} finding(s) could not be verified (verifier "
|
||||||
|
f"branch fault: {_fv_detail(soft_fv)}) — they stand above UNVERIFIED; "
|
||||||
|
"verify them manually"
|
||||||
)
|
)
|
||||||
# Keyed off synth_failure + a non-empty diff, NEVER off findings == [] —
|
# Keyed off synth_failure + a non-empty diff, NEVER off findings == [] —
|
||||||
# a clean review legitimately has zero findings and stays MERGE-READY.
|
# a clean review legitimately has zero findings and stays MERGE-READY.
|
||||||
@@ -190,7 +208,7 @@ def main():
|
|||||||
# faults go FIRST and uncapped so the display cap can never hide one;
|
# faults go FIRST and uncapped so the display cap can never hide one;
|
||||||
# the trigger reason keys off the human-facing triggers alone
|
# the trigger reason keys off the human-facing triggers alone
|
||||||
has_triggers = bool(attention)
|
has_triggers = bool(attention)
|
||||||
attention = faults + attention[:5]
|
attention = faults + soft_notes + attention[:5]
|
||||||
|
|
||||||
reasons = []
|
reasons = []
|
||||||
if counts["🔴"]:
|
if counts["🔴"]:
|
||||||
@@ -200,6 +218,11 @@ def main():
|
|||||||
if faults:
|
if faults:
|
||||||
# CONTRACT: review-gauntlet/verdict_gate.py anchors on this wording
|
# CONTRACT: review-gauntlet/verdict_gate.py anchors on this wording
|
||||||
reasons.append("pipeline fault(s) recorded — degraded run")
|
reasons.append("pipeline fault(s) recorded — degraded run")
|
||||||
|
if soft_notes:
|
||||||
|
# Deliberately does NOT match the gauntlet's degraded-run anchors:
|
||||||
|
# unverified findings are a human-attention item, not a re-runnable
|
||||||
|
# lane fault.
|
||||||
|
reasons.append(f"{len(soft_fv)} finding(s) unverified (verifier branch fault)")
|
||||||
if has_triggers:
|
if has_triggers:
|
||||||
reasons.append("always-human trigger(s) fired")
|
reasons.append("always-human trigger(s) fired")
|
||||||
verdict = "NEEDS-HUMAN" if reasons else "MERGE-READY"
|
verdict = "NEEDS-HUMAN" if reasons else "MERGE-READY"
|
||||||
|
|||||||
@@ -0,0 +1,52 @@
|
|||||||
|
#!/usr/bin/env python3
|
||||||
|
"""Local verifier-fault retry gate for the code-reviewer graph.
|
||||||
|
|
||||||
|
finding-verifier converts its own branch faults into DATA (UNVERIFIABLE
|
||||||
|
verdicts with a PIPELINE-FAULT: note, or parse_fault's pipeline-fault
|
||||||
|
sentinel), so the verify agent node completes "successfully" and its
|
||||||
|
max_attempts never fires. Without this gate the fault escalates all the
|
||||||
|
way to the gauntlet, which re-runs the ENTIRE code-review lane (~1 h) to
|
||||||
|
recover a ~4-minute verifier — the wrong trade. This gate re-runs just
|
||||||
|
the verifier ONCE; if faults persist, verdict.py downgrades them to a
|
||||||
|
proportionate NEEDS-HUMAN (findings stand, unverified).
|
||||||
|
|
||||||
|
Anchored detection only: the JSON-encoded note prefix and the sentinel id
|
||||||
|
— a finding merely QUOTING the marker mid-text does not trip it. Always
|
||||||
|
exits 0 with valid JSON; a crash falls forward on the declared edge.
|
||||||
|
"""
|
||||||
|
import json
|
||||||
|
import os
|
||||||
|
import sys
|
||||||
|
|
||||||
|
|
||||||
|
def load_state():
|
||||||
|
if path := os.environ.get("GRAPH_STATE_FILE"):
|
||||||
|
with open(path) as f:
|
||||||
|
return json.load(f)
|
||||||
|
return json.loads(os.environ.get("GRAPH_STATE", "{}"))
|
||||||
|
|
||||||
|
|
||||||
|
try:
|
||||||
|
state = load_state()
|
||||||
|
raw = state.get("verifier_output")
|
||||||
|
text = raw if isinstance(raw, str) else (json.dumps(raw) if raw is not None else "")
|
||||||
|
faulted = (
|
||||||
|
text.lstrip().startswith("Agent node failed:")
|
||||||
|
or '"note": "PIPELINE-FAULT:' in text
|
||||||
|
or "'note': 'PIPELINE-FAULT:" in text
|
||||||
|
or '"id": "pipeline-fault"' in text
|
||||||
|
)
|
||||||
|
try:
|
||||||
|
retries = int(state.get("verify_retries") or 0)
|
||||||
|
except (TypeError, ValueError):
|
||||||
|
retries = 0
|
||||||
|
if faulted and retries < 1:
|
||||||
|
sys.stderr.write(
|
||||||
|
"WARN: verifier output carries pipeline-fault markers — re-running "
|
||||||
|
"finding-verifier locally (attempt 2/2) before computing the verdict.\n"
|
||||||
|
)
|
||||||
|
print(json.dumps({"_next": "verify", "verify_retries": retries + 1}))
|
||||||
|
else:
|
||||||
|
print(json.dumps({"verify_retries": retries}))
|
||||||
|
except Exception as e: # never block the verdict
|
||||||
|
print(json.dumps({"verify_gate_error": f"verify_gate crashed: {e}"}))
|
||||||
@@ -174,7 +174,7 @@ nodes:
|
|||||||
- fs_read
|
- fs_read
|
||||||
- fs_cat
|
- fs_cat
|
||||||
- fs_grep
|
- fs_grep
|
||||||
max_iterations: 8
|
max_iterations: 14
|
||||||
max_attempts: 2
|
max_attempts: 2
|
||||||
state_updates:
|
state_updates:
|
||||||
verdict: '{{output}}'
|
verdict: '{{output}}'
|
||||||
|
|||||||
@@ -51,6 +51,7 @@ settings:
|
|||||||
timeout: 69600
|
timeout: 69600
|
||||||
|
|
||||||
initial_state:
|
initial_state:
|
||||||
|
retry_notes: []
|
||||||
diff_spec: worktree
|
diff_spec: worktree
|
||||||
plan_context: ''
|
plan_context: ''
|
||||||
rigor: production
|
rigor: production
|
||||||
|
|||||||
@@ -30,6 +30,7 @@ hard rules:
|
|||||||
import json
|
import json
|
||||||
import os
|
import os
|
||||||
import re
|
import re
|
||||||
|
import sys
|
||||||
import time
|
import time
|
||||||
|
|
||||||
# 69600 (graph timeout) − 2 × 11400 (largest lane envelope) − 1200 (script stages)
|
# 69600 (graph timeout) − 2 × 11400 (largest lane envelope) − 1200 (script stages)
|
||||||
@@ -184,6 +185,26 @@ def main():
|
|||||||
"review_incomplete": incomplete,
|
"review_incomplete": incomplete,
|
||||||
}
|
}
|
||||||
)
|
)
|
||||||
|
prior_notes = state.get("retry_notes")
|
||||||
|
notes = [n for n in prior_notes if isinstance(n, str)] if isinstance(prior_notes, list) else []
|
||||||
|
for lane in declined:
|
||||||
|
notes.append(
|
||||||
|
f"lane {lane} faulted and was NOT re-run "
|
||||||
|
f"(attempts {attempts.get(lane, '?')}/{MAX_ATTEMPTS}"
|
||||||
|
+ ("" if within_budget else "; wall-clock budget exhausted")
|
||||||
|
+ ") — recorded in review_incomplete"
|
||||||
|
)
|
||||||
|
for lane in retry:
|
||||||
|
notes.append(
|
||||||
|
f"lane {lane} faulted (missing / sentinel-less / degraded report) — "
|
||||||
|
f"re-running: attempt {attempts.get(lane, 0) + 1}/{MAX_ATTEMPTS}"
|
||||||
|
)
|
||||||
|
if retry or declined:
|
||||||
|
# stderr is discarded on success by today's engine but is the
|
||||||
|
# forward-compatible live-visibility channel; retry_notes carries the
|
||||||
|
# same lines into the final report either way.
|
||||||
|
sys.stderr.write("WARN: " + " | ".join(notes[-(len(retry) + len(declined)):]) + "\n")
|
||||||
|
out["retry_notes"] = notes
|
||||||
if retry:
|
if retry:
|
||||||
out["_next"] = "build_items"
|
out["_next"] = "build_items"
|
||||||
print(json.dumps(out))
|
print(json.dumps(out))
|
||||||
|
|||||||
@@ -360,6 +360,11 @@ def main():
|
|||||||
+ [f"| {lane} | {status} | {detail} |" for lane, (status, detail) in rows.items()]
|
+ [f"| {lane} | {status} | {detail} |" for lane, (status, detail) in rows.items()]
|
||||||
)
|
)
|
||||||
md += "\n\n## Lane selection\n" + (state.get("lanes_summary") or "(none recorded)")
|
md += "\n\n## Lane selection\n" + (state.get("lanes_summary") or "(none recorded)")
|
||||||
|
_notes = state.get("retry_notes")
|
||||||
|
if isinstance(_notes, list):
|
||||||
|
_shown = [n for n in _notes if isinstance(n, str)][:8]
|
||||||
|
if _shown:
|
||||||
|
md += "\n\n## Pipeline notes\n" + "\n".join(f"- {n}" for n in _shown)
|
||||||
if blockers:
|
if blockers:
|
||||||
md += "\n\n## Blockers\n" + "\n".join(f"- {b}" for b in blockers)
|
md += "\n\n## Blockers\n" + "\n".join(f"- {b}" for b in blockers)
|
||||||
if attention:
|
if attention:
|
||||||
|
|||||||
Reference in new issue
Block a user