fix(review-suite): detect nested finding-verifier faults; harden render/fault markers
code-reviewer verdict.py: finding-verifier now completes "successfully" with fault markers INSIDE its FINDING_VERIFIER_RESULTS payload (parse_fault's `pipeline-fault` sentinel id, verify_fault / verdict_gate crash-guard notes prefixed PIPELINE-FAULT:), which the old "Agent node failed:" prefix check never saw — a dead parse degraded silently to "unverified" with no fault recorded. Detect them, anchored on the sentinel id / note prefix only (never a substring over the report), counted over the raw entry list so duplicate crash-guard ids are not collapsed. Faults lead the attention list uncapped; the display cap of 5 applies to non-fault entries; the "always-human trigger(s) fired" reason fires only when non-fault triggers exist. code-reviewer render.py: CONTRACT comments at the verdict and summary lines review-gauntlet's gate parses; every LLM-influenced value rendered after the summary line (dropped_titles, rigor, bar_provenance, resolved_surfaces, reason) is newline-sanitised so none can forge a second summary line. finding-verifier verdict_gate.py: the outer crash-guard note carries the PIPELINE-FAULT: prefix (the after-retry UNVERIFIABLE path stays unprefixed — model misbehaviour, not infrastructure); README names all three emitters. deep-research: vet_fault's UNVETTED note tells the critic not to request revision solely because the assessment is unavailable (it matched critique's PREPRINT/UNVERIFIED REVISE trigger verbatim, burning the revision budget on an infra fault); vet_fault and critique_fault dedupe repeated faults and preserve accumulated faults on crash via safe_faults(); question_fault keeps its plan-pinned prefix and appends the lane's question identity so distinct dead lanes no longer collapse in combine_findings' dedupe.
This commit is contained in:
8 files changed
+570
-50
No files matched your search
@@ -16,13 +16,19 @@ def load_state():
|
||||
return json.loads(os.environ.get("GRAPH_STATE", "{}"))
|
||||
|
||||
|
||||
# LLM-influenced values rendered after the summary line: a newline could
|
||||
# forge a second summary line that review-gauntlet/verdict_gate.py parses.
|
||||
def one_line(x):
|
||||
return str(x).replace("\n", " ")
|
||||
|
||||
|
||||
def main():
|
||||
state = load_state()
|
||||
v = state.get("verdict_out") or {}
|
||||
findings = v.get("findings_final") or []
|
||||
counts = v.get("counts") or {}
|
||||
changed = state.get("changed_files") or []
|
||||
rigor = state.get("resolved_rigor") or "production"
|
||||
rigor = one_line(state.get("resolved_rigor") or "production")
|
||||
|
||||
# A fault can leave changed_files empty (a parse fault dies before the
|
||||
# diff is resolved; a verifier fault fires even on an empty diff) — a
|
||||
@@ -39,7 +45,8 @@ def main():
|
||||
lines = [
|
||||
"# Code Review Summary",
|
||||
"",
|
||||
f"**Verdict: {verdict}** — {v.get('reason', '')}",
|
||||
# CONTRACT: review-gauntlet/verdict_gate.py anchors on this wording
|
||||
f"**Verdict: {verdict}** — {one_line(v.get('reason', ''))}",
|
||||
"",
|
||||
"## Walkthrough",
|
||||
state.get("walkthrough") or "(no walkthrough provided)",
|
||||
@@ -96,15 +103,17 @@ def main():
|
||||
m for x in findings for m in
|
||||
__import__("re").findall(r"MISS-\d+", (x.get("title") or "") + (x.get("block") or ""))
|
||||
})
|
||||
dropped = ", ".join(one_line(t) for t in v.get("dropped_titles", [])[:5])
|
||||
lines += [
|
||||
"---",
|
||||
# CONTRACT: review-gauntlet/verdict_gate.py parses this line — keep the format in sync
|
||||
f"*Reviewed {len(changed)} files, found {counts.get('🔴', 0)} critical, "
|
||||
f"{counts.get('🟡', 0)} warnings, {counts.get('🟢', 0)} suggestions, "
|
||||
f"{counts.get('💡', 0)} nitpicks ({v.get('deferred_count', 0)} deferred by quality bar)*",
|
||||
f"*Quality bar: {rigor} — provenance: {state.get('bar_provenance', '?')}; "
|
||||
f"surfaces: {state.get('resolved_surfaces') or 'none'}*",
|
||||
f"*Quality bar: {rigor} — provenance: {one_line(state.get('bar_provenance', '?'))}; "
|
||||
f"surfaces: {one_line(state.get('resolved_surfaces') or 'none')}*",
|
||||
f"*Verdict: {verdict}; verifier: dropped {v.get('dropped_count', 0)} provably-false"
|
||||
+ (f" ({', '.join(v.get('dropped_titles', [])[:5])})" if v.get("dropped_count") else "")
|
||||
+ (f" ({dropped})" if v.get("dropped_count") else "")
|
||||
+ f"; review-miss patterns applied: {', '.join(miss_ids) if miss_ids else 'none relevant'}*",
|
||||
]
|
||||
print(json.dumps({"final_report": "\n".join(lines)}))
|
||||
|
||||
@@ -9,8 +9,11 @@ The model that wrote the findings never grades its own homework:
|
||||
- MERGE-READY/NEEDS-HUMAN is computed from counts + always-human triggers
|
||||
(deterministic signals ∪ the synthesis's ADDITIVE-only flags).
|
||||
Fail-closed: any PIPELINE-FAULT recorded upstream (dead domain lane, dead
|
||||
verifier, dead synthesis, missing domain reports) forces NEEDS-HUMAN — a
|
||||
degraded run can never read MERGE-READY.
|
||||
verifier, verifier verdicts carrying a PIPELINE-FAULT note or the
|
||||
`pipeline-fault` sentinel id, dead synthesis, missing domain reports)
|
||||
forces NEEDS-HUMAN — a degraded run can never read MERGE-READY. Faults
|
||||
always lead the attention list; the display cap applies only to the
|
||||
non-fault entries so a degraded run can never hide a fault.
|
||||
The gate never crashes into a silent verdict: any internal error emits
|
||||
NEEDS-HUMAN naming the error.
|
||||
"""
|
||||
@@ -36,35 +39,43 @@ def sev_icon(severity):
|
||||
return "🟢"
|
||||
|
||||
|
||||
def parse_verifier(text):
|
||||
"""Extract {id: {verdict, evidence, note}} from finding-verifier output."""
|
||||
verdicts = {}
|
||||
def parse_verifier_entries(text):
|
||||
"""Extract the raw list of {id, verdict, evidence, note} dicts from
|
||||
finding-verifier output. Order and duplicates are preserved so fault
|
||||
accounting can count every entry (crash-guard entries all carry the
|
||||
same "unknown" id)."""
|
||||
if not isinstance(text, str) or not text.strip():
|
||||
return verdicts
|
||||
return []
|
||||
start, end = text.find("["), text.rfind("]")
|
||||
if start != -1 and end > start:
|
||||
try:
|
||||
for v in json.loads(text[start : end + 1]):
|
||||
if isinstance(v, dict) and v.get("id"):
|
||||
verdicts[str(v["id"])] = v
|
||||
return verdicts
|
||||
return [
|
||||
v for v in json.loads(text[start : end + 1]) if isinstance(v, dict) and v.get("id")
|
||||
]
|
||||
except (ValueError, TypeError):
|
||||
pass
|
||||
entries = []
|
||||
for m in re.finditer(r'\{[^{}]*"id"[^{}]*\}', text):
|
||||
try:
|
||||
v = json.loads(m.group(0))
|
||||
if v.get("id"):
|
||||
verdicts[str(v["id"])] = v
|
||||
entries.append(v)
|
||||
except (ValueError, TypeError):
|
||||
continue
|
||||
return verdicts
|
||||
return entries
|
||||
|
||||
|
||||
def parse_verifier(entries):
|
||||
"""Index verifier entries by finding id; the last entry per id wins."""
|
||||
return {str(v["id"]): v for v in entries}
|
||||
|
||||
|
||||
def main():
|
||||
state = load_state()
|
||||
findings = [f for f in (state.get("findings") or []) if isinstance(f, dict)]
|
||||
rigor = (state.get("resolved_rigor") or "production").lower()
|
||||
verdicts = parse_verifier(state.get("verifier_output"))
|
||||
verifier_entries = parse_verifier_entries(state.get("verifier_output"))
|
||||
verdicts = parse_verifier(verifier_entries)
|
||||
|
||||
kept, dropped_titles = [], []
|
||||
for f in findings:
|
||||
@@ -142,6 +153,27 @@ def main():
|
||||
faults.append(
|
||||
"PIPELINE-FAULT: finding verification failed — findings render as unverified"
|
||||
)
|
||||
# finding-verifier completes "successfully" with faults INSIDE the payload:
|
||||
# parse_fault's `pipeline-fault` sentinel entry, or a per-finding
|
||||
# PIPELINE-FAULT note from verify_fault or verdict_gate's crash guard.
|
||||
# Anchored on the sentinel id / note prefix only. Counted over the raw
|
||||
# entries, not the id-indexed dict, so N crash-guard entries sharing the
|
||||
# "unknown" id count as N.
|
||||
fv_faults = [
|
||||
v
|
||||
for v in verifier_entries
|
||||
if v.get("id") == "pipeline-fault"
|
||||
or (isinstance(v.get("note"), str) and v["note"].lstrip().startswith("PIPELINE-FAULT:"))
|
||||
]
|
||||
if fv_faults:
|
||||
first = fv_faults[0].get("note")
|
||||
detail = first.strip().replace("\n", " ") if isinstance(first, str) else ""
|
||||
if len(detail) > MAX_DETAIL_CHARS:
|
||||
detail = detail[: MAX_DETAIL_CHARS - 1] + "…"
|
||||
faults.append(
|
||||
f"PIPELINE-FAULT: finding verification degraded — {len(fv_faults)} verifier "
|
||||
f"verdict(s) carry a pipeline fault ({detail})"
|
||||
)
|
||||
# Keyed off synth_failure + a non-empty diff, NEVER off findings == [] —
|
||||
# a clean review legitimately has zero findings and stays MERGE-READY.
|
||||
synth_failure = state.get("synth_failure")
|
||||
@@ -155,8 +187,10 @@ def main():
|
||||
"PIPELINE-FAULT: the diff is non-empty but no domain reports were produced — "
|
||||
"the review did not actually run; do not trust this verdict."
|
||||
)
|
||||
# faults go FIRST so the attention cap can never hide one
|
||||
attention = faults + attention
|
||||
# faults go FIRST and uncapped so the display cap can never hide one;
|
||||
# the trigger reason keys off the human-facing triggers alone
|
||||
has_triggers = bool(attention)
|
||||
attention = faults + attention[:5]
|
||||
|
||||
reasons = []
|
||||
if counts["🔴"]:
|
||||
@@ -164,8 +198,9 @@ def main():
|
||||
if yellow_correctness:
|
||||
reasons.append(f"{yellow_correctness} 🟡 [correctness] finding(s) outside the deferred section")
|
||||
if faults:
|
||||
# CONTRACT: review-gauntlet/verdict_gate.py anchors on this wording
|
||||
reasons.append("pipeline fault(s) recorded — degraded run")
|
||||
if attention:
|
||||
if has_triggers:
|
||||
reasons.append("always-human trigger(s) fired")
|
||||
verdict = "NEEDS-HUMAN" if reasons else "MERGE-READY"
|
||||
reason_line = "; ".join(reasons) if reasons else "no blocking findings and no always-human triggers"
|
||||
@@ -180,7 +215,7 @@ def main():
|
||||
"deferred_count": len(deferred),
|
||||
"dropped_count": len(dropped_titles),
|
||||
"dropped_titles": dropped_titles,
|
||||
"attention": attention[:5],
|
||||
"attention": attention,
|
||||
"findings_final": final,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -33,6 +33,14 @@ def load_state():
|
||||
return json.loads(os.environ.get("GRAPH_STATE", "{}"))
|
||||
|
||||
|
||||
def safe_faults():
|
||||
try:
|
||||
state = load_state()
|
||||
return [f for f in (state.get("pipeline_faults") or []) if isinstance(f, str)]
|
||||
except Exception: # noqa: BLE001 — best-effort preservation only
|
||||
return []
|
||||
|
||||
|
||||
def main():
|
||||
state = load_state()
|
||||
faults = [f for f in (state.get("pipeline_faults") or []) if isinstance(f, str)]
|
||||
@@ -42,19 +50,21 @@ def main():
|
||||
detail = "the critique stage died without recording a failure detail"
|
||||
if len(detail) > MAX_DETAIL_CHARS:
|
||||
detail = detail[: MAX_DETAIL_CHARS - 1] + "…"
|
||||
faults.append(f"PIPELINE-FAULT: critique failed — {detail}")
|
||||
fault = f"PIPELINE-FAULT: critique failed — {detail}"
|
||||
if fault not in faults:
|
||||
faults.append(fault)
|
||||
print(json.dumps({"pipeline_faults": faults, "critique": CRITIQUE_NOTE}))
|
||||
|
||||
|
||||
try:
|
||||
main()
|
||||
except Exception as e: # noqa: BLE001 — the fault marker itself must never crash
|
||||
faults = safe_faults()
|
||||
faults.append(f"PIPELINE-FAULT: critique failed — fault-marker script error: {e}")
|
||||
print(
|
||||
json.dumps(
|
||||
{
|
||||
"pipeline_faults": [
|
||||
f"PIPELINE-FAULT: critique failed — fault-marker script error: {e}"
|
||||
],
|
||||
"pipeline_faults": faults,
|
||||
"critique": CRITIQUE_NOTE,
|
||||
}
|
||||
)
|
||||
|
||||
@@ -12,12 +12,15 @@ PIPELINE-FAULT finding and the chain ends here — the map collects the fault
|
||||
as the lane's `finding`, so one dead research lane never sinks the whole
|
||||
map. combine_findings later lifts PIPELINE-FAULT findings into
|
||||
`pipeline_faults` so the fault also surfaces in the report's Pipeline notes.
|
||||
The question text is part of the fault so distinct dead lanes with identical
|
||||
engine failure text do not collapse into one entry there.
|
||||
"""
|
||||
|
||||
import json
|
||||
import os
|
||||
|
||||
MAX_DETAIL_CHARS = 500
|
||||
MAX_QUESTION_CHARS = 200
|
||||
|
||||
|
||||
def load_state():
|
||||
@@ -27,14 +30,25 @@ def load_state():
|
||||
return json.loads(os.environ.get("GRAPH_STATE", "{}"))
|
||||
|
||||
|
||||
def fault_finding(detail):
|
||||
def fault_finding(question, detail):
|
||||
return {
|
||||
"finding": (
|
||||
f"PIPELINE-FAULT: question research failed — finding unavailable — {detail}"
|
||||
"PIPELINE-FAULT: question research failed — finding unavailable — "
|
||||
f'{detail} (question: "{question}")'
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
def question_label(state):
|
||||
raw = state.get("question")
|
||||
question = raw.strip().replace("\n", " ") if isinstance(raw, str) else ""
|
||||
if not question:
|
||||
return "(unknown question)"
|
||||
if len(question) > MAX_QUESTION_CHARS:
|
||||
question = question[: MAX_QUESTION_CHARS - 1] + "…"
|
||||
return question
|
||||
|
||||
|
||||
def main():
|
||||
state = load_state()
|
||||
raw = state.get("finding")
|
||||
@@ -43,10 +57,14 @@ def main():
|
||||
detail = "the research lane died without recording a failure detail"
|
||||
if len(detail) > MAX_DETAIL_CHARS:
|
||||
detail = detail[: MAX_DETAIL_CHARS - 1] + "…"
|
||||
print(json.dumps(fault_finding(detail)))
|
||||
print(json.dumps(fault_finding(question_label(state), detail)))
|
||||
|
||||
|
||||
try:
|
||||
main()
|
||||
except Exception as e: # noqa: BLE001 — the fault marker itself must never crash
|
||||
print(json.dumps(fault_finding(f"fault-marker script error: {e}")))
|
||||
print(
|
||||
json.dumps(
|
||||
fault_finding("(unknown question)", f"fault-marker script error: {e}")
|
||||
)
|
||||
)
|
||||
@@ -20,7 +20,9 @@ MAX_DETAIL_CHARS = 500
|
||||
UNVETTED_NOTE = (
|
||||
"Source credibility assessment unavailable — the vetting stage failed "
|
||||
"(pipeline fault); treat every cited source as unvetted and needing "
|
||||
"corroboration."
|
||||
"corroboration. Do not request revision solely because this assessment "
|
||||
"is unavailable — re-research cannot restore it; judge the findings on "
|
||||
"their own evidence."
|
||||
)
|
||||
|
||||
|
||||
@@ -31,6 +33,14 @@ def load_state():
|
||||
return json.loads(os.environ.get("GRAPH_STATE", "{}"))
|
||||
|
||||
|
||||
def safe_faults():
|
||||
try:
|
||||
state = load_state()
|
||||
return [f for f in (state.get("pipeline_faults") or []) if isinstance(f, str)]
|
||||
except Exception: # noqa: BLE001 — best-effort preservation only
|
||||
return []
|
||||
|
||||
|
||||
def main():
|
||||
state = load_state()
|
||||
faults = [f for f in (state.get("pipeline_faults") or []) if isinstance(f, str)]
|
||||
@@ -40,20 +50,21 @@ def main():
|
||||
detail = "the vetting stage died without recording a failure detail"
|
||||
if len(detail) > MAX_DETAIL_CHARS:
|
||||
detail = detail[: MAX_DETAIL_CHARS - 1] + "…"
|
||||
faults.append(f"PIPELINE-FAULT: source vetting failed — {detail}")
|
||||
fault = f"PIPELINE-FAULT: source vetting failed — {detail}"
|
||||
if fault not in faults:
|
||||
faults.append(fault)
|
||||
print(json.dumps({"pipeline_faults": faults, "source_assessment": UNVETTED_NOTE}))
|
||||
|
||||
|
||||
try:
|
||||
main()
|
||||
except Exception as e: # noqa: BLE001 — the fault marker itself must never crash
|
||||
faults = safe_faults()
|
||||
faults.append(f"PIPELINE-FAULT: source vetting failed — fault-marker script error: {e}")
|
||||
print(
|
||||
json.dumps(
|
||||
{
|
||||
"pipeline_faults": [
|
||||
f"PIPELINE-FAULT: source vetting failed — "
|
||||
f"fault-marker script error: {e}"
|
||||
],
|
||||
"pipeline_faults": faults,
|
||||
"source_assessment": UNVETTED_NOTE,
|
||||
}
|
||||
)
|
||||
|
||||
@@ -46,9 +46,10 @@ flowchart TD
|
||||
contract (VERIFIED = keep + paste evidence; FALSE = drop + tally; UNVERIFIABLE = keep,
|
||||
marked). No final LLM step that could editorialize.
|
||||
|
||||
The dashed edges are fault paths. Both fault markers surface as `PIPELINE-FAULT:` text in
|
||||
a verdict's `note`, and a fault can only ever produce UNVERIFIABLE — degraded output is
|
||||
never a passing verdict. Every path ends at `done`, so the `FINDING_VERIFIER_RESULTS`
|
||||
The dashed edges are fault paths. All three fault emitters (`parse_fault`, `verify_fault`,
|
||||
and `verdict_gate`'s own crash guard) surface as `PIPELINE-FAULT:` text in a verdict's
|
||||
`note`, and a fault can only ever produce UNVERIFIABLE — degraded output is never a
|
||||
passing verdict. Every path ends at `done`, so the `FINDING_VERIFIER_RESULTS`
|
||||
sentinel is always emitted and the caller's verify lane degrades instead of dying.
|
||||
|
||||
## What it is NOT
|
||||
|
||||
@@ -104,6 +104,7 @@ def main():
|
||||
)
|
||||
return
|
||||
|
||||
# Deliberately not PIPELINE-FAULT-prefixed: model misbehavior, not infrastructure.
|
||||
print(
|
||||
json.dumps(
|
||||
{
|
||||
@@ -128,7 +129,7 @@ except Exception as e: # noqa: BLE001 — the gate must never crash the chain
|
||||
"id": "unknown",
|
||||
"verdict": "UNVERIFIABLE",
|
||||
"evidence": "",
|
||||
"note": f"verdict gate error: {e}",
|
||||
"note": f"PIPELINE-FAULT: verdict gate error: {e}",
|
||||
}
|
||||
}
|
||||
)
|
||||
|
||||
Reference in new issue
Block a user