fix: wave 5b review blockers - suppressed memory key, finalizer merge, zero-image guard
This commit is contained in:
@@ -7,7 +7,8 @@ import threading
|
|||||||
from typing import Any, Dict, Iterable, Optional
|
from typing import Any, Dict, Iterable, Optional
|
||||||
|
|
||||||
|
|
||||||
_COLLECTION_KEYS = {"sheets", "clusters", "findings", "decisions", "rfis"}
|
_COLLECTION_KEYS = {"sheets", "clusters", "findings", "decisions", "rfis",
|
||||||
|
"suppressed"}
|
||||||
_MAPPING_KEYS = {"sheet_index", "jurisdiction", "object_graph"}
|
_MAPPING_KEYS = {"sheet_index", "jurisdiction", "object_graph"}
|
||||||
_MEMORY_KEYS = _COLLECTION_KEYS | _MAPPING_KEYS
|
_MEMORY_KEYS = _COLLECTION_KEYS | _MAPPING_KEYS
|
||||||
|
|
||||||
|
|||||||
+14
-11
@@ -173,28 +173,31 @@ def run_agent_pipeline(
|
|||||||
]
|
]
|
||||||
|
|
||||||
orchestrator.stage("Agent wave 5b: evidence verification")
|
orchestrator.stage("Agent wave 5b: evidence verification")
|
||||||
sheet_to_page = {s.get("sheet_number"): s.get("page_number") for s in sheets}
|
sheet_to_page = {str(s.get("sheet_number")): s.get("page_number")
|
||||||
|
for s in sheets}
|
||||||
verify_targets = select_findings(
|
verify_targets = select_findings(
|
||||||
specialist_findings, clusters,
|
specialist_findings, clusters,
|
||||||
max_checks=config.AGENT_VERIFY_MAX_CHECKS,
|
max_checks=config.AGENT_VERIFY_MAX_CHECKS,
|
||||||
severities=config.AGENT_VERIFY_SEVERITIES,
|
severities=config.AGENT_VERIFY_SEVERITIES,
|
||||||
)
|
)
|
||||||
target_indexes = {id(f): i for i, f in enumerate(specialist_findings)}
|
target_indexes = {id(f): i for i, f in enumerate(specialist_findings)}
|
||||||
verify_scopes = [
|
verify_scopes = []
|
||||||
AgentScope(
|
for finding in verify_targets:
|
||||||
|
images = [
|
||||||
|
page_to_b64[sheet_to_page[str(name)]]
|
||||||
|
for name in (finding.get("sheets") or [])[:config.AGENT_CONFLICT_MAX_IMAGES]
|
||||||
|
if sheet_to_page.get(str(name)) in page_to_b64
|
||||||
|
]
|
||||||
|
if not images:
|
||||||
|
continue # never judge evidence against images we could not load
|
||||||
|
verify_scopes.append(AgentScope(
|
||||||
scope_id=f"verify:{target_indexes[id(finding)]}",
|
scope_id=f"verify:{target_indexes[id(finding)]}",
|
||||||
payload={
|
payload={
|
||||||
"finding_index": target_indexes[id(finding)],
|
"finding_index": target_indexes[id(finding)],
|
||||||
"finding": finding,
|
"finding": finding,
|
||||||
"images_b64": [
|
"images_b64": images,
|
||||||
page_to_b64[sheet_to_page[name]]
|
|
||||||
for name in (finding.get("sheets") or [])[:config.AGENT_CONFLICT_MAX_IMAGES]
|
|
||||||
if sheet_to_page.get(name) in page_to_b64
|
|
||||||
],
|
|
||||||
},
|
},
|
||||||
)
|
))
|
||||||
for finding in verify_targets
|
|
||||||
]
|
|
||||||
verify_results = orchestrator.run_scopes(
|
verify_results = orchestrator.run_scopes(
|
||||||
EvidenceVerifierAgent(usage), verify_scopes, config.AGENT_VERIFY_CONCURRENCY)
|
EvidenceVerifierAgent(usage), verify_scopes, config.AGENT_VERIFY_CONCURRENCY)
|
||||||
suppressed = apply_verdicts(specialist_findings, verify_results)
|
suppressed = apply_verdicts(specialist_findings, verify_results)
|
||||||
|
|||||||
@@ -216,7 +216,7 @@ def finalize_review(job_id: str, out_dir: str) -> dict:
|
|||||||
rfis = _draft_rfis(kept)
|
rfis = _draft_rfis(kept)
|
||||||
|
|
||||||
report["validated_issues"] = kept
|
report["validated_issues"] = kept
|
||||||
report["suppressed_issues"] = suppressed
|
report["suppressed_issues"] = (report.get("suppressed_issues") or []) + suppressed
|
||||||
report["rfis"] = rfis
|
report["rfis"] = rfis
|
||||||
summary = report.setdefault("summary", {})
|
summary = report.setdefault("summary", {})
|
||||||
summary["agent_status"] = "complete"
|
summary["agent_status"] = "complete"
|
||||||
|
|||||||
@@ -0,0 +1,86 @@
|
|||||||
|
"""Runner-level wave-5b tests: suppression path and zero-image guard."""
|
||||||
|
|
||||||
|
import backend.agents.runner as runner_mod
|
||||||
|
from backend.agents.base import AgentResult
|
||||||
|
from backend.agents.runner import run_agent_pipeline
|
||||||
|
|
||||||
|
|
||||||
|
def _finding(sheets):
|
||||||
|
return {
|
||||||
|
"issue_id": "C1", "severity": "critical", "confidence": "high",
|
||||||
|
"source_stage": "constructability", "sheets": sheets,
|
||||||
|
"description": "HSS16x4 on (2) 2x6 STUD PACK is unbuildable",
|
||||||
|
"evidence": [{"sheet": sheets[0], "source_text": "(2) 2x6 STUD PACK"}],
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
def _stub_agent(artifacts):
|
||||||
|
return lambda usage: type("S", (), {
|
||||||
|
"name": "stub",
|
||||||
|
"run": lambda self, scope: AgentResult(
|
||||||
|
scope_id=scope.scope_id, artifacts=list(artifacts)),
|
||||||
|
})()
|
||||||
|
|
||||||
|
|
||||||
|
def _patch_pipeline(monkeypatch, finding):
|
||||||
|
monkeypatch.setattr(
|
||||||
|
runner_mod, "convert_pdf_to_images",
|
||||||
|
lambda path: [{"page_number": 1, "base64": "QUJD"}])
|
||||||
|
monkeypatch.setattr(runner_mod, "SheetExtractorAgent", _stub_agent([
|
||||||
|
{"sheet_number": "S401", "page_number": 1, "level": "roof",
|
||||||
|
"discipline": "S", "assertions": [
|
||||||
|
{"text": "(2) 2x6 STUD PACK", "object_type": "framing"},
|
||||||
|
{"text": "HSS16X4 beam", "object_type": "framing"},
|
||||||
|
]},
|
||||||
|
]))
|
||||||
|
monkeypatch.setattr(runner_mod, "SheetIndexAgent", _stub_agent([{}]))
|
||||||
|
monkeypatch.setattr(runner_mod, "JurisdictionAgent", _stub_agent([{}]))
|
||||||
|
monkeypatch.setattr(runner_mod, "LinkerAgent", _stub_agent([
|
||||||
|
{"key": "c1", "location": "roof beam pocket", "assertions": []},
|
||||||
|
]))
|
||||||
|
monkeypatch.setattr(runner_mod, "ConflictCriticAgent", _stub_agent([]))
|
||||||
|
monkeypatch.setattr(runner_mod, "CodeAgent", _stub_agent([]))
|
||||||
|
monkeypatch.setattr(runner_mod, "ConstructabilityAgent", _stub_agent([finding]))
|
||||||
|
monkeypatch.setattr(runner_mod, "CompletenessAgent", _stub_agent([]))
|
||||||
|
monkeypatch.setattr(
|
||||||
|
runner_mod, "BrainAgent",
|
||||||
|
lambda usage: type("B", (), {
|
||||||
|
"run": lambda self, findings, sheet_index, jurisdiction:
|
||||||
|
(list(findings), [])})())
|
||||||
|
|
||||||
|
|
||||||
|
def test_refuted_finding_is_suppressed_not_crash(monkeypatch, tmp_path):
|
||||||
|
"""Regression: memory.replace("suppressed", ...) must not KeyError."""
|
||||||
|
_patch_pipeline(monkeypatch, _finding(["S401"]))
|
||||||
|
monkeypatch.setattr(
|
||||||
|
"backend.agents.verifier.call_json",
|
||||||
|
lambda **kwargs: {"verdicts": [
|
||||||
|
{"sheet": "S401", "source_text": "(2) 2x6 STUD PACK",
|
||||||
|
"verdict": "corrected", "actual_text": "(5) 2x6 STUD PACK",
|
||||||
|
"notes": "callout reads (5)"},
|
||||||
|
]})
|
||||||
|
pdf = tmp_path / "dummy.pdf"
|
||||||
|
pdf.write_bytes(b"%PDF-1.4\n")
|
||||||
|
report = run_agent_pipeline(str(pdf), out_dir=str(tmp_path),
|
||||||
|
require_review=False)
|
||||||
|
assert [f["issue_id"] for f in report["suppressed_issues"]] == ["C1"]
|
||||||
|
assert report["suppressed_issues"][0]["verification"]["status"] == "refuted"
|
||||||
|
|
||||||
|
|
||||||
|
def test_zero_image_finding_is_not_suppressed(monkeypatch, tmp_path):
|
||||||
|
"""A finding whose sheets resolve to no page images must not be judged
|
||||||
|
(and must never be refuted) without pixels."""
|
||||||
|
_patch_pipeline(monkeypatch, _finding(["S999"])) # no such sheet
|
||||||
|
monkeypatch.setattr(
|
||||||
|
"backend.agents.verifier.call_json",
|
||||||
|
lambda **kwargs: {"verdicts": [
|
||||||
|
{"sheet": "S999", "source_text": "(2) 2x6 STUD PACK",
|
||||||
|
"verdict": "not_found", "actual_text": None, "notes": None},
|
||||||
|
]})
|
||||||
|
pdf = tmp_path / "dummy.pdf"
|
||||||
|
pdf.write_bytes(b"%PDF-1.4\n")
|
||||||
|
report = run_agent_pipeline(str(pdf), out_dir=str(tmp_path),
|
||||||
|
require_review=False)
|
||||||
|
assert report["suppressed_issues"] == []
|
||||||
|
validated = report.get("validated_issues") or []
|
||||||
|
assert any(f.get("issue_id") == "C1" for f in validated)
|
||||||
@@ -85,6 +85,29 @@ def test_finalize_confirm_keeps_confirmed(monkeypatch, tmp_path):
|
|||||||
assert report["summary"]["agent_status"] == "complete"
|
assert report["summary"]["agent_status"] == "complete"
|
||||||
|
|
||||||
|
|
||||||
|
def test_finalize_preserves_verifier_suppressed(monkeypatch, tmp_path):
|
||||||
|
"""Wave-5b (verifier) suppressions must survive review finalization and
|
||||||
|
merge with review-rejected suppressions."""
|
||||||
|
monkeypatch.setattr("backend.review.finalizer._draft_rfis", lambda kept: [])
|
||||||
|
_write_job(
|
||||||
|
str(tmp_path),
|
||||||
|
prioritized=[{"issue_id": "AGENT-0001", "severity": "high"}],
|
||||||
|
queue=[_blocking_item("AGENT-0001")],
|
||||||
|
decisions=[{"review_item_id": "finding:AGENT-0001",
|
||||||
|
"decision": "reject", "reason_code": "not_a_contradiction"}],
|
||||||
|
)
|
||||||
|
path = os.path.join(str(tmp_path), "conflicts.json")
|
||||||
|
with open(path, encoding="utf-8") as f:
|
||||||
|
report = json.load(f)
|
||||||
|
report["suppressed_issues"] = [
|
||||||
|
{"issue_id": "C1", "verification": {"status": "refuted"}}]
|
||||||
|
with open(path, "w", encoding="utf-8") as f:
|
||||||
|
json.dump(report, f)
|
||||||
|
final = finalize_review("job1", str(tmp_path))
|
||||||
|
ids = [f["issue_id"] for f in final["suppressed_issues"]]
|
||||||
|
assert ids == ["C1", "AGENT-0001"]
|
||||||
|
|
||||||
|
|
||||||
def test_finalize_no_decision_keeps_unreviewed(monkeypatch, tmp_path):
|
def test_finalize_no_decision_keeps_unreviewed(monkeypatch, tmp_path):
|
||||||
"""Non-blocking (audit) items don't need a decision; issue stays unreviewed."""
|
"""Non-blocking (audit) items don't need a decision; issue stays unreviewed."""
|
||||||
monkeypatch.setattr("backend.review.finalizer._draft_rfis", lambda kept: [])
|
monkeypatch.setattr("backend.review.finalizer._draft_rfis", lambda kept: [])
|
||||||
|
|||||||
Reference in New Issue
Block a user