Add required human review gate to the Agent pipeline.
Agent web jobs now stop after Brain consolidation and enter needs_review with a persisted review queue (blocking: high-severity, low-confidence, sensitive-category findings; audit sample of clean clusters). Humans decide confirm/reject/unsure/needs_clarification via new review API and frontend queue; a finalizer applies decisions (rejections suppressed with reason codes), performs bounded targeted reruns for clarifications, drafts RFIs only for kept issues, and only then marks the job done and sends the final email. Two-phase email (review-required, then final report), per-decision feedback labels with redacted aggregate metrics, restart recovery from job artifacts, and CLI --no-review bypass. Classic pipeline unchanged. 65 non-LLM tests.
This commit is contained in:
John Wilganowski
@@ -0,0 +1,65 @@
|
||||
from backend.agents.base import AgentResult
|
||||
from backend.agents.runner import run_agent_pipeline
|
||||
|
||||
|
||||
def _patch_brain(monkeypatch):
|
||||
monkeypatch.setattr("backend.agents.runner.convert_pdf_to_images", lambda path: [{"page_number": 1, "base64": "x"}])
|
||||
monkeypatch.setattr("backend.agents.runner.BrainAgent", lambda usage: type("B", (), {"run": lambda self, findings, sheet_index, jurisdiction: ([{"issue_id": "AGENT-0001", "severity": "high", "confidence": "high", "category": "note_or_spec_contradiction", "source_stage": "conflict"}], [])})())
|
||||
|
||||
|
||||
def test_agent_runner_can_enter_review_mode(monkeypatch, tmp_path):
|
||||
_patch_brain(monkeypatch)
|
||||
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=True)
|
||||
assert report["summary"]["agent_status"] == "needs_review"
|
||||
assert report["summary"]["review"]["required"] == 1
|
||||
|
||||
|
||||
def test_review_mode_writes_memory_snapshot(monkeypatch, tmp_path):
|
||||
"""The finalizer needs agent/memory.json for targeted clarification reruns."""
|
||||
_patch_brain(monkeypatch)
|
||||
pdf = tmp_path / "dummy.pdf"
|
||||
pdf.write_bytes(b"%PDF-1.4\n")
|
||||
run_agent_pipeline(str(pdf), out_dir=str(tmp_path), require_review=True)
|
||||
assert (tmp_path / "agent" / "memory.json").is_file()
|
||||
|
||||
|
||||
def test_review_mode_summary_includes_agent_observability(monkeypatch, tmp_path):
|
||||
"""Review-mode candidate reports must carry the same usage/stats block as
|
||||
the wave-7 path so finalizer fix-ups and feedback labels have real data."""
|
||||
_patch_brain(monkeypatch)
|
||||
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=True)
|
||||
summary = report["summary"]
|
||||
assert "agent_stats" in summary
|
||||
assert summary["by_stage"]["rfis"] == 0
|
||||
assert summary["by_stage"]["validated"] == 1
|
||||
assert "conflicts" in summary["by_stage"]
|
||||
assert "cost_usd" in summary
|
||||
assert "llm_calls" in summary
|
||||
assert "cached_calls" in summary
|
||||
assert "cost_by_stage" in summary
|
||||
assert "models_used" in summary
|
||||
|
||||
|
||||
def test_agent_runner_without_review_still_writes_rfis(monkeypatch, tmp_path):
|
||||
_patch_brain(monkeypatch)
|
||||
monkeypatch.setattr(
|
||||
"backend.agents.runner.RFIWriterAgent",
|
||||
lambda usage: type("R", (), {
|
||||
"name": "rfi_writer",
|
||||
"run": lambda self, scope: AgentResult(
|
||||
scope_id=scope.scope_id,
|
||||
artifacts=[{"issue_id": "AGENT-0001", "question": "Confirm intent?"}],
|
||||
),
|
||||
})(),
|
||||
)
|
||||
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["summary"]["agent_status"] == "complete"
|
||||
assert "review" not in report["summary"]
|
||||
assert len(report["rfis"]) == 1
|
||||
assert report["rfis"][0]["issue_id"] == "AGENT-0001"
|
||||
@@ -0,0 +1,439 @@
|
||||
"""API tests for the human-review endpoints and review-aware job states."""
|
||||
|
||||
import json
|
||||
import os
|
||||
import threading
|
||||
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
import backend.jobs
|
||||
from backend.main import app
|
||||
from backend.review.store import ReviewStore
|
||||
|
||||
|
||||
class _SyncThread:
|
||||
"""Drop-in threading.Thread replacement that runs the target inline."""
|
||||
|
||||
def __init__(self, target=None, args=(), **kwargs):
|
||||
self._target = target
|
||||
self._args = args
|
||||
|
||||
def start(self):
|
||||
self._target(*self._args)
|
||||
|
||||
|
||||
def _queue_item(item_id: str) -> dict:
|
||||
return {"review_item_id": item_id, "kind": "finding",
|
||||
"blocking": True, "reasons": ["high_severity"], "payload": {}}
|
||||
|
||||
|
||||
def test_review_queue_and_decision_save(monkeypatch, tmp_path):
|
||||
store = ReviewStore(str(tmp_path))
|
||||
store.write_queue([_queue_item("finding:AGENT-0001")])
|
||||
client = TestClient(app)
|
||||
monkeypatch.setattr("backend.main.get_job", lambda job_id: {"job_id": job_id, "status": "needs_review", "report": {"summary": {}}, "out_dir": str(tmp_path)})
|
||||
queue_response = client.get("/jobs/job1/review")
|
||||
assert queue_response.status_code == 200
|
||||
decision_response = client.post("/jobs/job1/review-decisions", json={"decisions": [{"review_item_id": "finding:AGENT-0001", "decision": "confirm"}]})
|
||||
assert decision_response.status_code == 200
|
||||
|
||||
|
||||
def test_review_decision_emits_feedback_label(monkeypatch, tmp_path):
|
||||
"""Every saved decision appends one feedback label under review/."""
|
||||
store = ReviewStore(str(tmp_path))
|
||||
store.write_queue([_queue_item("finding:AGENT-0001")])
|
||||
client = TestClient(app)
|
||||
monkeypatch.setattr("backend.main.get_job", lambda job_id: {"job_id": job_id, "status": "needs_review", "report": {"summary": {}}, "out_dir": str(tmp_path)})
|
||||
response = client.post("/jobs/job1/review-decisions", json={"decisions": [{"review_item_id": "finding:AGENT-0001", "decision": "confirm"}]})
|
||||
assert response.status_code == 200
|
||||
path = os.path.join(str(tmp_path), "review", "feedback_labels.jsonl")
|
||||
with open(path, encoding="utf-8") as f:
|
||||
labels = [json.loads(line) for line in f if line.strip()]
|
||||
assert len(labels) == 1
|
||||
assert labels[0]["review_item_id"] == "finding:AGENT-0001"
|
||||
assert labels[0]["decision"] == "confirm"
|
||||
|
||||
|
||||
def test_review_endpoints_404_for_unknown_job(monkeypatch, tmp_path):
|
||||
monkeypatch.setattr("backend.config.OUTPUT_DIR", str(tmp_path))
|
||||
client = TestClient(app)
|
||||
assert client.get("/jobs/nope/review").status_code == 404
|
||||
assert client.post("/jobs/nope/review-decisions", json={"decisions": []}).status_code == 404
|
||||
|
||||
|
||||
def test_review_decision_invalid_returns_422(monkeypatch, tmp_path):
|
||||
client = TestClient(app)
|
||||
monkeypatch.setattr("backend.main.get_job", lambda job_id: {"job_id": job_id, "status": "needs_review", "report": {"summary": {}}, "out_dir": str(tmp_path)})
|
||||
response = client.post("/jobs/job1/review-decisions", json={"decisions": [{"review_item_id": "finding:AGENT-0001", "decision": "bogus"}]})
|
||||
assert response.status_code == 422
|
||||
|
||||
|
||||
def test_partial_review_moves_job_to_reviewing(monkeypatch, tmp_path):
|
||||
"""Saving some but not all required decisions flips needs_review -> reviewing."""
|
||||
store = ReviewStore(str(tmp_path))
|
||||
store.write_queue([_queue_item("finding:AGENT-0001"), _queue_item("finding:AGENT-0002")])
|
||||
job_id = "jobreviewing"
|
||||
backend.jobs._jobs[job_id] = {
|
||||
"job_id": job_id,
|
||||
"status": "needs_review",
|
||||
"out_dir": str(tmp_path),
|
||||
"report": {"summary": {}},
|
||||
}
|
||||
try:
|
||||
client = TestClient(app)
|
||||
response = client.post(f"/jobs/{job_id}/review-decisions", json={
|
||||
"decisions": [{"review_item_id": "finding:AGENT-0001", "decision": "confirm"}],
|
||||
})
|
||||
assert response.status_code == 200
|
||||
assert response.json()["progress"]["remaining"] == 1
|
||||
assert backend.jobs.get_job(job_id)["status"] == "reviewing"
|
||||
finally:
|
||||
backend.jobs._jobs.pop(job_id, None)
|
||||
|
||||
|
||||
def test_agent_job_needs_review_skips_notify(monkeypatch, tmp_path):
|
||||
"""Carried finding from Task 4: an agent report needing review must not be emailed."""
|
||||
sent = []
|
||||
monkeypatch.setattr("backend.config.OUTPUT_DIR", str(tmp_path))
|
||||
monkeypatch.setattr("backend.jobs.run_agent_pipeline", lambda pdf_path, **kw: {
|
||||
"summary": {"agent_status": "needs_review"}, "conflicts": [],
|
||||
})
|
||||
monkeypatch.setattr("backend.jobs.send_conflict_report", lambda *a, **kw: sent.append((a, kw)))
|
||||
monkeypatch.setattr(threading, "Thread", _SyncThread)
|
||||
|
||||
pdf = tmp_path / "upload.pdf"
|
||||
pdf.write_bytes(b"%PDF-1.4 dummy")
|
||||
job_id = backend.jobs.create_job(str(pdf), source_filename="set.pdf",
|
||||
email="arch@example.com", pipeline_mode="agent")
|
||||
try:
|
||||
job = backend.jobs.get_job(job_id)
|
||||
assert job["status"] == "needs_review"
|
||||
assert job["report"]["summary"]["agent_status"] == "needs_review"
|
||||
assert sent == []
|
||||
finally:
|
||||
backend.jobs._jobs.pop(job_id, None)
|
||||
|
||||
|
||||
def test_classic_job_still_completes_and_notifies(monkeypatch, tmp_path):
|
||||
"""Classic pipeline behavior is unchanged: done status + completion email."""
|
||||
sent = []
|
||||
monkeypatch.setattr("backend.config.OUTPUT_DIR", str(tmp_path))
|
||||
monkeypatch.setattr("backend.jobs.run_pipeline", lambda pdf_path, **kw: {
|
||||
"summary": {}, "conflicts": [],
|
||||
})
|
||||
monkeypatch.setattr("backend.jobs.send_conflict_report", lambda *a, **kw: sent.append((a, kw)))
|
||||
monkeypatch.setattr(threading, "Thread", _SyncThread)
|
||||
|
||||
pdf = tmp_path / "upload.pdf"
|
||||
pdf.write_bytes(b"%PDF-1.4 dummy")
|
||||
job_id = backend.jobs.create_job(str(pdf), source_filename="set.pdf",
|
||||
email="arch@example.com", pipeline_mode="classic")
|
||||
try:
|
||||
assert backend.jobs.get_job(job_id)["status"] == "done"
|
||||
assert len(sent) == 1
|
||||
finally:
|
||||
backend.jobs._jobs.pop(job_id, None)
|
||||
|
||||
|
||||
def test_job_status_includes_review_progress(monkeypatch, tmp_path):
|
||||
"""GET /jobs/{id} surfaces report.summary.review for a needs_review job."""
|
||||
review = {"required": 1, "completed": 0, "remaining": 1, "total": 1}
|
||||
client = TestClient(app)
|
||||
monkeypatch.setattr("backend.main.get_job", lambda job_id: {
|
||||
"job_id": job_id, "status": "needs_review",
|
||||
"report": {"summary": {"agent_status": "needs_review", "review": review}},
|
||||
"out_dir": str(tmp_path),
|
||||
})
|
||||
response = client.get("/jobs/job1")
|
||||
assert response.status_code == 200
|
||||
body = response.json()
|
||||
assert body["status"] == "needs_review"
|
||||
assert body["report"]["summary"]["review"] == review
|
||||
|
||||
|
||||
def test_review_response_includes_saved_decisions(monkeypatch, tmp_path):
|
||||
"""GET /jobs/{id}/review also returns the decisions map for UI pre-population."""
|
||||
store = ReviewStore(str(tmp_path))
|
||||
store.write_queue([_queue_item("finding:AGENT-0001")])
|
||||
client = TestClient(app)
|
||||
monkeypatch.setattr("backend.main.get_job", lambda job_id: {"job_id": job_id, "status": "needs_review", "report": {"summary": {}}, "out_dir": str(tmp_path)})
|
||||
post = client.post("/jobs/job1/review-decisions", json={"decisions": [
|
||||
{"review_item_id": "finding:AGENT-0001", "decision": "confirm", "comment": "looks right"},
|
||||
]})
|
||||
assert post.status_code == 200
|
||||
get = client.get("/jobs/job1/review")
|
||||
assert get.status_code == 200
|
||||
decisions = get.json()["decisions"]
|
||||
assert decisions["finding:AGENT-0001"]["decision"] == "confirm"
|
||||
assert decisions["finding:AGENT-0001"]["comment"] == "looks right"
|
||||
|
||||
|
||||
def test_review_flow_via_disk_fallback(monkeypatch, tmp_path):
|
||||
"""Smoke: a synthetic on-disk needs_review job served by the REAL get_job
|
||||
(disk fallback), with decisions persisting across review GETs."""
|
||||
job_id = "jobdisk"
|
||||
out_dir = os.path.join(str(tmp_path), job_id)
|
||||
os.makedirs(out_dir)
|
||||
report = {
|
||||
"source": "set.pdf",
|
||||
"summary": {
|
||||
"agent_status": "needs_review",
|
||||
"review": {"required": 1, "completed": 0, "remaining": 1, "total": 1},
|
||||
},
|
||||
"conflicts": [],
|
||||
}
|
||||
with open(os.path.join(out_dir, "conflicts.json"), "w", encoding="utf-8") as f:
|
||||
json.dump(report, f)
|
||||
store = ReviewStore(out_dir)
|
||||
store.write_queue([_queue_item("finding:AGENT-0001")])
|
||||
monkeypatch.setattr("backend.config.OUTPUT_DIR", str(tmp_path))
|
||||
client = TestClient(app)
|
||||
|
||||
job_response = client.get(f"/jobs/{job_id}")
|
||||
assert job_response.status_code == 200
|
||||
assert job_response.json()["report"]["summary"]["review"]["required"] == 1
|
||||
|
||||
review_response = client.get(f"/jobs/{job_id}/review")
|
||||
assert review_response.status_code == 200
|
||||
body = review_response.json()
|
||||
assert [i["review_item_id"] for i in body["queue"]] == ["finding:AGENT-0001"]
|
||||
assert body["progress"]["remaining"] == 1
|
||||
assert body["decisions"] == {}
|
||||
|
||||
post = client.post(f"/jobs/{job_id}/review-decisions", json={"decisions": [
|
||||
{"review_item_id": "finding:AGENT-0001", "decision": "reject",
|
||||
"reason_code": "not_a_contradiction"},
|
||||
]})
|
||||
assert post.status_code == 200
|
||||
assert post.json()["progress"]["remaining"] == 0
|
||||
|
||||
again = client.get(f"/jobs/{job_id}/review")
|
||||
assert again.status_code == 200
|
||||
saved = again.json()["decisions"]["finding:AGENT-0001"]
|
||||
assert saved["decision"] == "reject"
|
||||
assert saved["reason_code"] == "not_a_contradiction"
|
||||
|
||||
|
||||
def _write_restart_job(tmp_path, job_id, email=None):
|
||||
"""On-disk needs_review job artifacts, as a pre-restart run would leave
|
||||
them: candidate report + memory snapshot + review queue (+ job.json)."""
|
||||
out_dir = os.path.join(str(tmp_path), job_id)
|
||||
os.makedirs(os.path.join(out_dir, "agent"))
|
||||
report = {
|
||||
"source": "set.pdf",
|
||||
"generated_at": "2026-07-28T00:00:00+00:00",
|
||||
"summary": {
|
||||
"sheets_analyzed": 0, "disciplines": [], "assertions_extracted": 0,
|
||||
"clusters_checked": 0, "conflicts_found": 0,
|
||||
"by_severity": {"high": 0, "medium": 0, "low": 0}, "by_category": {},
|
||||
"pipeline_mode": "agent", "agent_status": "needs_review",
|
||||
"review": {"required": 1, "completed": 0, "remaining": 1, "total": 1},
|
||||
},
|
||||
"conflicts": [], "sheets": [],
|
||||
"validated_issues": [{"issue_id": "AGENT-0001", "severity": "high"}],
|
||||
"suppressed_issues": [], "rfis": [],
|
||||
}
|
||||
with open(os.path.join(out_dir, "conflicts.json"), "w", encoding="utf-8") as f:
|
||||
json.dump(report, f)
|
||||
with open(os.path.join(out_dir, "agent", "memory.json"), "w", encoding="utf-8") as f:
|
||||
json.dump({}, f)
|
||||
if email is not None:
|
||||
with open(os.path.join(out_dir, "job.json"), "w", encoding="utf-8") as f:
|
||||
json.dump({"job_id": job_id, "email": email,
|
||||
"pipeline_mode": "agent", "source": "set.pdf"}, f)
|
||||
store = ReviewStore(out_dir)
|
||||
store.write_queue([_queue_item("finding:AGENT-0001")])
|
||||
return out_dir
|
||||
|
||||
|
||||
def test_restart_recovered_needs_review_job_finalizes(monkeypatch, tmp_path):
|
||||
"""CRITICAL: after a restart, a needs_review job recovered from disk keeps
|
||||
its status (not "done"), hydrates the in-memory registry, and the whole
|
||||
decide -> finalize flow completes to done via the real get_job."""
|
||||
job_id = "jobrestart"
|
||||
_write_restart_job(tmp_path, job_id)
|
||||
monkeypatch.setattr("backend.config.OUTPUT_DIR", str(tmp_path))
|
||||
monkeypatch.setattr("backend.review.finalizer._draft_rfis", lambda kept: [])
|
||||
monkeypatch.setattr("backend.jobs._notify", lambda *a, **kw: None)
|
||||
monkeypatch.setattr(threading, "Thread", _SyncThread)
|
||||
try:
|
||||
# Real disk fallback: simulates a fresh post-restart process.
|
||||
job = backend.jobs.get_job(job_id)
|
||||
assert job["status"] == "needs_review"
|
||||
assert job_id in backend.jobs._jobs # hydrated for _set() transitions
|
||||
|
||||
client = TestClient(app)
|
||||
post = client.post(f"/jobs/{job_id}/review-decisions", json={"decisions": [
|
||||
{"review_item_id": "finding:AGENT-0001", "decision": "confirm"},
|
||||
]})
|
||||
assert post.status_code == 200
|
||||
|
||||
fin = client.post(f"/jobs/{job_id}/finalize-review")
|
||||
assert fin.status_code == 200
|
||||
job = backend.jobs.get_job(job_id)
|
||||
assert job["status"] == "done"
|
||||
assert job["report"]["summary"]["agent_status"] == "complete"
|
||||
finally:
|
||||
backend.jobs._jobs.pop(job_id, None)
|
||||
|
||||
|
||||
def test_restart_recovered_job_final_email_uses_job_json(monkeypatch, tmp_path):
|
||||
"""CRITICAL: job.json (written at job start) restores the recipient email
|
||||
after a restart, so finalization still fires the final report email."""
|
||||
job_id = "jobemail"
|
||||
_write_restart_job(tmp_path, job_id, email="arch@example.com")
|
||||
monkeypatch.setattr("backend.config.OUTPUT_DIR", str(tmp_path))
|
||||
monkeypatch.setattr("backend.review.finalizer._draft_rfis", lambda kept: [])
|
||||
sent = []
|
||||
monkeypatch.setattr("backend.jobs.send_conflict_report",
|
||||
lambda email, report, **kw: sent.append(email))
|
||||
monkeypatch.setattr(threading, "Thread", _SyncThread)
|
||||
try:
|
||||
job = backend.jobs.get_job(job_id)
|
||||
assert job["status"] == "needs_review"
|
||||
assert job["email"] == "arch@example.com"
|
||||
|
||||
client = TestClient(app)
|
||||
post = client.post(f"/jobs/{job_id}/review-decisions", json={"decisions": [
|
||||
{"review_item_id": "finding:AGENT-0001", "decision": "confirm"},
|
||||
]})
|
||||
assert post.status_code == 200
|
||||
fin = client.post(f"/jobs/{job_id}/finalize-review")
|
||||
assert fin.status_code == 200
|
||||
assert backend.jobs.get_job(job_id)["status"] == "done"
|
||||
assert sent == ["arch@example.com"]
|
||||
finally:
|
||||
backend.jobs._jobs.pop(job_id, None)
|
||||
|
||||
|
||||
def test_review_decisions_409_for_non_review_job(monkeypatch, tmp_path):
|
||||
"""Positive state guard: only needs_review/reviewing jobs accept decisions."""
|
||||
client = TestClient(app)
|
||||
monkeypatch.setattr("backend.main.get_job", lambda job_id: {
|
||||
"job_id": job_id, "status": "done", "out_dir": str(tmp_path),
|
||||
})
|
||||
response = client.post("/jobs/job1/review-decisions", json={"decisions": [
|
||||
{"review_item_id": "finding:AGENT-0001", "decision": "confirm"}]})
|
||||
assert response.status_code == 409
|
||||
assert "done" in response.json()["detail"]["detail"]
|
||||
|
||||
|
||||
def test_review_queue_get_does_not_create_review_dir(monkeypatch, tmp_path):
|
||||
"""The read-only GET endpoint must not create review/ dirs on read."""
|
||||
client = TestClient(app)
|
||||
monkeypatch.setattr("backend.main.get_job", lambda job_id: {
|
||||
"job_id": job_id, "status": "needs_review",
|
||||
"report": {"summary": {}}, "out_dir": str(tmp_path),
|
||||
})
|
||||
response = client.get("/jobs/job1/review")
|
||||
assert response.status_code == 200
|
||||
assert response.json()["queue"] == []
|
||||
assert response.json()["decisions"] == {}
|
||||
assert not os.path.exists(os.path.join(str(tmp_path), "review"))
|
||||
|
||||
|
||||
def _write_finalizable_job(tmp_path, decisions):
|
||||
"""Minimal review-mode artifacts: candidate report + queue + decisions."""
|
||||
out_dir = str(tmp_path)
|
||||
os.makedirs(os.path.join(out_dir, "agent"), exist_ok=True)
|
||||
report = {
|
||||
"source": "set.pdf",
|
||||
"generated_at": "2026-07-28T00:00:00+00:00",
|
||||
"summary": {
|
||||
"sheets_analyzed": 0, "disciplines": [], "assertions_extracted": 0,
|
||||
"clusters_checked": 0, "conflicts_found": 0,
|
||||
"by_severity": {"high": 0, "medium": 0, "low": 0}, "by_category": {},
|
||||
"pipeline_mode": "agent", "agent_status": "needs_review",
|
||||
"review": {"required": 1, "completed": 0, "remaining": 1, "total": 1},
|
||||
},
|
||||
"conflicts": [], "sheets": [],
|
||||
"validated_issues": [{"issue_id": "AGENT-0001", "severity": "high"}],
|
||||
"suppressed_issues": [], "rfis": [],
|
||||
}
|
||||
with open(os.path.join(out_dir, "conflicts.json"), "w", encoding="utf-8") as f:
|
||||
json.dump(report, f)
|
||||
with open(os.path.join(out_dir, "agent", "memory.json"), "w", encoding="utf-8") as f:
|
||||
json.dump({}, f)
|
||||
store = ReviewStore(out_dir)
|
||||
store.write_queue([_queue_item("finding:AGENT-0001")])
|
||||
for decision in decisions:
|
||||
store.append_decision(decision)
|
||||
return out_dir
|
||||
|
||||
|
||||
def test_finalize_review_409_while_undecided(monkeypatch, tmp_path):
|
||||
out_dir = _write_finalizable_job(tmp_path, decisions=[])
|
||||
client = TestClient(app)
|
||||
monkeypatch.setattr("backend.main.get_job", lambda job_id: {
|
||||
"job_id": job_id, "status": "needs_review", "out_dir": out_dir,
|
||||
})
|
||||
response = client.post("/jobs/job1/finalize-review")
|
||||
assert response.status_code == 409
|
||||
body = response.json()
|
||||
assert body["detail"]["detail"] == "incomplete review"
|
||||
assert body["detail"]["progress"]["remaining"] == 1
|
||||
|
||||
|
||||
def test_finalize_review_404_for_unknown_job(monkeypatch, tmp_path):
|
||||
monkeypatch.setattr("backend.config.OUTPUT_DIR", str(tmp_path))
|
||||
client = TestClient(app)
|
||||
assert client.post("/jobs/nope/finalize-review").status_code == 404
|
||||
|
||||
|
||||
def test_finalize_review_409_when_already_done(monkeypatch, tmp_path):
|
||||
client = TestClient(app)
|
||||
monkeypatch.setattr("backend.main.get_job", lambda job_id: {
|
||||
"job_id": job_id, "status": "done", "out_dir": str(tmp_path),
|
||||
})
|
||||
assert client.post("/jobs/job1/finalize-review").status_code == 409
|
||||
|
||||
|
||||
def test_finalize_review_409_for_job_not_in_review(monkeypatch, tmp_path):
|
||||
"""A running (or otherwise non-review) job must not be finalizable: no
|
||||
finalization thread, no artifact clobbering, no final email."""
|
||||
threads = []
|
||||
notified = []
|
||||
monkeypatch.setattr(threading, "Thread",
|
||||
lambda *a, **kw: threads.append((a, kw)) or _SyncThread(*a, **kw))
|
||||
monkeypatch.setattr("backend.jobs._notify",
|
||||
lambda *a, **kw: notified.append(a))
|
||||
monkeypatch.setattr("backend.main.get_job", lambda job_id: {
|
||||
"job_id": job_id, "status": "running", "out_dir": str(tmp_path),
|
||||
})
|
||||
client = TestClient(app)
|
||||
response = client.post("/jobs/job1/finalize-review")
|
||||
assert response.status_code == 409
|
||||
assert "running" in response.json()["detail"]["detail"]
|
||||
assert threads == []
|
||||
assert notified == []
|
||||
assert not os.path.exists(os.path.join(str(tmp_path), "conflicts.json"))
|
||||
|
||||
|
||||
def test_finalize_review_happy_path_notifies_once(monkeypatch, tmp_path):
|
||||
out_dir = _write_finalizable_job(tmp_path, decisions=[{
|
||||
"review_item_id": "finding:AGENT-0001", "decision": "confirm",
|
||||
}])
|
||||
notified = []
|
||||
monkeypatch.setattr("backend.review.finalizer._draft_rfis", lambda kept: [])
|
||||
monkeypatch.setattr("backend.jobs._notify",
|
||||
lambda job_id, report, out_dir: notified.append(job_id))
|
||||
monkeypatch.setattr(threading, "Thread", _SyncThread)
|
||||
job_id = "jobfinalize"
|
||||
backend.jobs._jobs[job_id] = {
|
||||
"job_id": job_id, "status": "reviewing", "out_dir": out_dir,
|
||||
"report": None, "email": "arch@example.com",
|
||||
}
|
||||
try:
|
||||
client = TestClient(app)
|
||||
response = client.post(f"/jobs/{job_id}/finalize-review")
|
||||
assert response.status_code == 200
|
||||
assert response.json() == {"status": "finalizing"}
|
||||
job = backend.jobs.get_job(job_id)
|
||||
assert job["status"] == "done"
|
||||
assert job["report"]["summary"]["agent_status"] == "complete"
|
||||
assert notified == [job_id]
|
||||
for name in ("conflicts.json", "validated_issues.json",
|
||||
"suppressed_issues.json", "rfis.json", "report.md"):
|
||||
assert os.path.isfile(os.path.join(out_dir, name)), name
|
||||
finally:
|
||||
backend.jobs._jobs.pop(job_id, None)
|
||||
@@ -0,0 +1,106 @@
|
||||
"""Tests for the two-phase email flow: review-required notice, then final report."""
|
||||
|
||||
import threading
|
||||
|
||||
import backend.jobs
|
||||
from backend.email_sender import send_review_required
|
||||
|
||||
|
||||
class _SyncThread:
|
||||
"""Drop-in threading.Thread replacement that runs the target inline."""
|
||||
|
||||
def __init__(self, target=None, args=(), **kwargs):
|
||||
self._target = target
|
||||
self._args = args
|
||||
|
||||
def start(self):
|
||||
self._target(*self._args)
|
||||
|
||||
|
||||
def test_review_required_email_skips_without_smtp(monkeypatch):
|
||||
monkeypatch.setattr("backend.email_sender._smtp_ready", lambda: False)
|
||||
assert send_review_required("user@example.com", {"source": "set.pdf", "summary": {}}, "http://localhost:8099/?job=abc") is False
|
||||
|
||||
|
||||
def test_review_required_email_sends_with_smtp(monkeypatch):
|
||||
"""With SMTP ready, the message goes out with recipient, review URL, and
|
||||
the required-item count (0 when the report has no review summary)."""
|
||||
sent = []
|
||||
monkeypatch.setattr("backend.email_sender._smtp_ready", lambda: True)
|
||||
monkeypatch.setattr("backend.email_sender._send",
|
||||
lambda msg: sent.append(msg) or True)
|
||||
|
||||
review_url = "http://localhost:8099/?job=abc"
|
||||
report = {"source": "set.pdf", "summary": {"review": {"required": 3}}}
|
||||
assert send_review_required("user@example.com", report, review_url) is True
|
||||
|
||||
assert len(sent) == 1
|
||||
msg = sent[0]
|
||||
assert msg["To"] == "user@example.com"
|
||||
assert "review" in msg["Subject"].lower()
|
||||
body = msg.get_content()
|
||||
assert "review" in body.lower()
|
||||
assert review_url in body
|
||||
assert "3" in body
|
||||
|
||||
# Missing review summary -> required count defaults to 0.
|
||||
sent.clear()
|
||||
assert send_review_required("user@example.com", {"source": "set.pdf", "summary": {}}, review_url) is True
|
||||
assert "0" in sent[0].get_content()
|
||||
|
||||
|
||||
def test_agent_needs_review_sends_review_email_not_report(monkeypatch, tmp_path):
|
||||
"""An agent job entering needs_review emails the review-required notice
|
||||
exactly once and never sends the final conflict report."""
|
||||
review_emails = []
|
||||
report_emails = []
|
||||
monkeypatch.setattr("backend.config.OUTPUT_DIR", str(tmp_path))
|
||||
monkeypatch.setattr("backend.jobs.run_agent_pipeline", lambda pdf_path, **kw: {
|
||||
"summary": {"agent_status": "needs_review"}, "conflicts": [],
|
||||
})
|
||||
monkeypatch.setattr("backend.jobs.send_review_required",
|
||||
lambda *a, **kw: review_emails.append((a, kw)))
|
||||
monkeypatch.setattr("backend.jobs.send_conflict_report",
|
||||
lambda *a, **kw: report_emails.append((a, kw)))
|
||||
monkeypatch.setattr(threading, "Thread", _SyncThread)
|
||||
|
||||
pdf = tmp_path / "upload.pdf"
|
||||
pdf.write_bytes(b"%PDF-1.4 dummy")
|
||||
job_id = backend.jobs.create_job(str(pdf), source_filename="set.pdf",
|
||||
email="arch@example.com", pipeline_mode="agent")
|
||||
try:
|
||||
job = backend.jobs.get_job(job_id)
|
||||
assert job["status"] == "needs_review"
|
||||
assert len(review_emails) == 1
|
||||
args, _ = review_emails[0]
|
||||
assert args[0] == "arch@example.com"
|
||||
assert f"/?job={job_id}" in args[2]
|
||||
assert report_emails == []
|
||||
finally:
|
||||
backend.jobs._jobs.pop(job_id, None)
|
||||
|
||||
|
||||
def test_classic_job_sends_only_conflict_report(monkeypatch, tmp_path):
|
||||
"""Classic pipeline is untouched: only the final report email fires."""
|
||||
review_emails = []
|
||||
report_emails = []
|
||||
monkeypatch.setattr("backend.config.OUTPUT_DIR", str(tmp_path))
|
||||
monkeypatch.setattr("backend.jobs.run_pipeline", lambda pdf_path, **kw: {
|
||||
"summary": {}, "conflicts": [],
|
||||
})
|
||||
monkeypatch.setattr("backend.jobs.send_review_required",
|
||||
lambda *a, **kw: review_emails.append((a, kw)))
|
||||
monkeypatch.setattr("backend.jobs.send_conflict_report",
|
||||
lambda *a, **kw: report_emails.append((a, kw)))
|
||||
monkeypatch.setattr(threading, "Thread", _SyncThread)
|
||||
|
||||
pdf = tmp_path / "upload.pdf"
|
||||
pdf.write_bytes(b"%PDF-1.4 dummy")
|
||||
job_id = backend.jobs.create_job(str(pdf), source_filename="set.pdf",
|
||||
email="arch@example.com", pipeline_mode="classic")
|
||||
try:
|
||||
assert backend.jobs.get_job(job_id)["status"] == "done"
|
||||
assert len(report_emails) == 1
|
||||
assert review_emails == []
|
||||
finally:
|
||||
backend.jobs._jobs.pop(job_id, None)
|
||||
@@ -0,0 +1,87 @@
|
||||
"""Feedback labels and aggregate metrics for human-review decisions."""
|
||||
|
||||
import json
|
||||
import os
|
||||
from datetime import datetime
|
||||
|
||||
from backend.review.feedback import decision_to_label, write_label
|
||||
from backend.review.metrics import aggregate_labels
|
||||
|
||||
|
||||
def test_aggregate_redacts_text_by_default():
|
||||
labels = [{"decision": "reject", "reason_code": "missing_evidence", "comment": "secret", "payload": {"evidence": [{"source_text": "secret"}]}}]
|
||||
summary = aggregate_labels(labels)
|
||||
assert summary["reject"] == 1
|
||||
assert "secret" not in str(summary)
|
||||
|
||||
|
||||
def test_aggregate_include_text_embeds_labels():
|
||||
labels = [{"decision": "reject", "reason_code": "missing_evidence", "comment": "secret"}]
|
||||
summary = aggregate_labels(labels, include_text=True)
|
||||
assert summary["labels"] == labels
|
||||
|
||||
|
||||
def _queue_item() -> dict:
|
||||
return {
|
||||
"review_item_id": "finding:AGENT-0007",
|
||||
"kind": "finding",
|
||||
"blocking": True,
|
||||
"reasons": ["high_severity"],
|
||||
"payload": {
|
||||
"issue_id": "AGENT-0007",
|
||||
"source_stage": "conflict",
|
||||
"category": "elevation_disagreement",
|
||||
"severity": "high",
|
||||
"confidence": "medium",
|
||||
"location": "Room 204 / Level 2",
|
||||
"disciplines": ["Architectural", "Mechanical"],
|
||||
"sheets": ["A2.1", "M2.1"],
|
||||
"drawing_type": "floor_plan",
|
||||
},
|
||||
}
|
||||
|
||||
|
||||
def test_decision_to_label_builds_spec_shape():
|
||||
decision = {"review_item_id": "finding:AGENT-0007", "decision": "reject",
|
||||
"reason_code": "same_value_different_representation"}
|
||||
job = {"job_id": "abc123", "pipeline_mode": "agent",
|
||||
"report": {"summary": {"models_used": ["google/gemini-2.5-pro"]}}}
|
||||
label = decision_to_label(_queue_item(), decision, job)
|
||||
assert label["review_item_id"] == "finding:AGENT-0007"
|
||||
assert label["job_id"] == "abc123"
|
||||
assert label["pipeline_mode"] == "agent"
|
||||
assert label["source_stage"] == "conflict"
|
||||
assert label["category"] == "elevation_disagreement"
|
||||
assert label["severity"] == "high"
|
||||
assert label["confidence"] == "medium"
|
||||
assert label["decision"] == "reject"
|
||||
assert label["reason_code"] == "same_value_different_representation"
|
||||
assert label["location"] == "Room 204 / Level 2"
|
||||
assert label["disciplines"] == ["Architectural", "Mechanical"]
|
||||
assert label["sheets"] == ["A2.1", "M2.1"]
|
||||
assert label["drawing_type"] == "floor_plan"
|
||||
assert label["models_used"] == ["google/gemini-2.5-pro"]
|
||||
datetime.fromisoformat(label["created_at"])
|
||||
|
||||
|
||||
def test_decision_to_label_degrades_on_missing_fields():
|
||||
label = decision_to_label({"review_item_id": "finding:AGENT-0001"}, {}, {})
|
||||
assert label["review_item_id"] == "finding:AGENT-0001"
|
||||
assert label["job_id"] is None
|
||||
assert label["decision"] is None
|
||||
assert label["reason_code"] is None
|
||||
assert label["category"] is None
|
||||
assert label["source_stage"] is None
|
||||
assert label["models_used"] == []
|
||||
datetime.fromisoformat(label["created_at"])
|
||||
|
||||
|
||||
def test_write_label_appends_json_lines(tmp_path):
|
||||
label1 = {"review_item_id": "finding:AGENT-0001", "decision": "confirm"}
|
||||
label2 = {"review_item_id": "finding:AGENT-0002", "decision": "reject"}
|
||||
write_label(str(tmp_path), label1)
|
||||
write_label(str(tmp_path), label2)
|
||||
path = os.path.join(str(tmp_path), "review", "feedback_labels.jsonl")
|
||||
with open(path, encoding="utf-8") as f:
|
||||
lines = [json.loads(line) for line in f if line.strip()]
|
||||
assert lines == [label1, label2]
|
||||
@@ -0,0 +1,291 @@
|
||||
"""Non-LLM tests for the review finalizer: decisions, reruns, final artifacts."""
|
||||
|
||||
import json
|
||||
import os
|
||||
|
||||
import pytest
|
||||
|
||||
from backend.agents.base import AgentResult
|
||||
from backend.review.finalizer import (
|
||||
apply_decisions,
|
||||
finalize_review,
|
||||
rerun_clarified_scopes,
|
||||
)
|
||||
from backend.review.store import ReviewStore
|
||||
|
||||
|
||||
def test_reject_suppresses_with_reason():
|
||||
prioritized = [{"issue_id": "AGENT-0001", "severity": "high"}]
|
||||
decisions = {"finding:AGENT-0001": {"decision": "reject", "reason_code": "duplicate"}}
|
||||
kept, suppressed = apply_decisions(prioritized, decisions)
|
||||
assert kept == []
|
||||
assert suppressed[0]["review_state"] == "rejected"
|
||||
assert suppressed[0]["reason_code"] == "duplicate"
|
||||
|
||||
|
||||
def test_unsure_is_kept_but_flagged():
|
||||
prioritized = [{"issue_id": "AGENT-0002", "severity": "medium"}]
|
||||
decisions = {"finding:AGENT-0002": {"decision": "unsure"}}
|
||||
kept, suppressed = apply_decisions(prioritized, decisions)
|
||||
assert kept[0]["review_state"] == "unsure"
|
||||
assert suppressed == []
|
||||
|
||||
|
||||
def _write_job(out_dir, prioritized, queue, decisions=None, memory=None):
|
||||
"""Hand-written review-mode artifacts (conflicts.json + agent/memory.json)."""
|
||||
os.makedirs(os.path.join(out_dir, "agent"), exist_ok=True)
|
||||
report = {
|
||||
"source": "set.pdf",
|
||||
"generated_at": "2026-07-28T00:00:00+00:00",
|
||||
"summary": {
|
||||
"sheets_analyzed": 0,
|
||||
"disciplines": [],
|
||||
"assertions_extracted": 0,
|
||||
"clusters_checked": 0,
|
||||
"conflicts_found": 0,
|
||||
"by_severity": {"high": 0, "medium": 0, "low": 0},
|
||||
"by_category": {},
|
||||
"pipeline_mode": "agent",
|
||||
"agent_status": "needs_review",
|
||||
"review": {"required": 1, "completed": 0, "remaining": 1, "total": 1},
|
||||
"by_stage": {"validated": len(prioritized), "rfis": 0},
|
||||
},
|
||||
"conflicts": [],
|
||||
"sheets": [],
|
||||
"validated_issues": prioritized,
|
||||
"suppressed_issues": [],
|
||||
"rfis": [],
|
||||
}
|
||||
with open(os.path.join(out_dir, "conflicts.json"), "w", encoding="utf-8") as f:
|
||||
json.dump(report, f)
|
||||
with open(os.path.join(out_dir, "agent", "memory.json"), "w", encoding="utf-8") as f:
|
||||
json.dump(memory or {}, f)
|
||||
store = ReviewStore(out_dir)
|
||||
store.write_queue(queue)
|
||||
for decision in decisions or []:
|
||||
store.append_decision(decision)
|
||||
|
||||
|
||||
def _blocking_item(issue_id):
|
||||
return {"review_item_id": f"finding:{issue_id}", "kind": "finding",
|
||||
"blocking": True, "reasons": ["high_severity"], "payload": {}}
|
||||
|
||||
|
||||
def test_finalize_confirm_keeps_confirmed(monkeypatch, tmp_path):
|
||||
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": "confirm"}],
|
||||
)
|
||||
report = finalize_review("job1", str(tmp_path))
|
||||
assert report["validated_issues"][0]["review_state"] == "confirmed"
|
||||
assert report["suppressed_issues"] == []
|
||||
assert report["summary"]["agent_status"] == "complete"
|
||||
|
||||
|
||||
def test_finalize_no_decision_keeps_unreviewed(monkeypatch, tmp_path):
|
||||
"""Non-blocking (audit) items don't need a decision; issue stays unreviewed."""
|
||||
monkeypatch.setattr("backend.review.finalizer._draft_rfis", lambda kept: [])
|
||||
item = {**_blocking_item("AGENT-0001"), "blocking": False, "kind": "audit_finding"}
|
||||
_write_job(
|
||||
str(tmp_path),
|
||||
prioritized=[{"issue_id": "AGENT-0001", "severity": "medium"}],
|
||||
queue=[item],
|
||||
)
|
||||
report = finalize_review("job1", str(tmp_path))
|
||||
assert report["validated_issues"][0]["review_state"] == "unreviewed"
|
||||
|
||||
|
||||
def test_finalize_clarification_replacement_marked_clarified(monkeypatch, tmp_path):
|
||||
monkeypatch.setattr("backend.review.finalizer._draft_rfis", lambda kept: [])
|
||||
replacement = {"issue_id": "AGENT-0001-R1", "severity": "medium",
|
||||
"clarification_of": "AGENT-0001"}
|
||||
monkeypatch.setattr(
|
||||
"backend.review.finalizer.rerun_clarified_scopes",
|
||||
lambda snapshot, decisions, prioritized=None: [replacement],
|
||||
)
|
||||
_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": "needs_clarification",
|
||||
"clarification_answer": "Ceiling is 9'-0\" AFF."}],
|
||||
)
|
||||
report = finalize_review("job1", str(tmp_path))
|
||||
kept = report["validated_issues"]
|
||||
assert [issue["issue_id"] for issue in kept] == ["AGENT-0001-R1"]
|
||||
assert kept[0]["review_state"] == "clarified"
|
||||
|
||||
|
||||
def test_finalize_failed_clarification_flagged(monkeypatch, tmp_path):
|
||||
monkeypatch.setattr("backend.review.finalizer._draft_rfis", lambda kept: [])
|
||||
monkeypatch.setattr(
|
||||
"backend.review.finalizer.rerun_clarified_scopes",
|
||||
lambda snapshot, decisions, prioritized=None: [],
|
||||
)
|
||||
_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": "needs_clarification",
|
||||
"clarification_answer": "Ceiling is 9'-0\" AFF."}],
|
||||
)
|
||||
report = finalize_review("job1", str(tmp_path))
|
||||
assert report["validated_issues"][0]["review_state"] == "clarification_failed"
|
||||
|
||||
|
||||
def test_finalize_incomplete_review_raises(monkeypatch, tmp_path):
|
||||
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")],
|
||||
)
|
||||
with pytest.raises(ValueError, match="incomplete review"):
|
||||
finalize_review("job1", str(tmp_path))
|
||||
|
||||
|
||||
def test_finalize_writes_final_artifacts(monkeypatch, tmp_path):
|
||||
monkeypatch.setattr("backend.review.finalizer._draft_rfis",
|
||||
lambda kept: [{"issue_id": kept[0]["issue_id"], "question": "?"}])
|
||||
_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": "confirm"}],
|
||||
)
|
||||
report = finalize_review("job1", str(tmp_path))
|
||||
assert report["summary"]["by_stage"]["validated"] == 1
|
||||
assert report["summary"]["by_stage"]["rfis"] == 1
|
||||
for name in ("conflicts.json", "validated_issues.json",
|
||||
"suppressed_issues.json", "rfis.json", "report.md"):
|
||||
assert os.path.isfile(os.path.join(str(tmp_path), name)), name
|
||||
with open(os.path.join(str(tmp_path), "validated_issues.json"), encoding="utf-8") as f:
|
||||
assert json.load(f)[0]["review_state"] == "confirmed"
|
||||
|
||||
|
||||
def test_finalize_reject_rebuilds_conflicts_and_counts(monkeypatch, tmp_path):
|
||||
"""Rejected conflict-stage findings must not survive into the final
|
||||
report's conflicts / headline counts; suppressed_issues keeps them."""
|
||||
monkeypatch.setattr("backend.review.finalizer._draft_rfis", lambda kept: [])
|
||||
kept_finding = {
|
||||
"issue_id": "AGENT-0001", "source_stage": "conflict",
|
||||
"category": "note_or_spec_contradiction", "severity": "high",
|
||||
"location": "Grid A", "disciplines": ["A", "S"], "sheets": ["A-1"],
|
||||
"description": "kept finding", "evidence": [],
|
||||
"recommended_resolution": "fix", "confidence": "high",
|
||||
}
|
||||
rejected_finding = {
|
||||
**kept_finding, "issue_id": "AGENT-0002", "severity": "medium",
|
||||
"description": "rejected finding",
|
||||
}
|
||||
_write_job(
|
||||
str(tmp_path),
|
||||
prioritized=[kept_finding, rejected_finding],
|
||||
queue=[_blocking_item("AGENT-0001"), _blocking_item("AGENT-0002")],
|
||||
decisions=[
|
||||
{"review_item_id": "finding:AGENT-0001", "decision": "confirm"},
|
||||
{"review_item_id": "finding:AGENT-0002", "decision": "reject",
|
||||
"reason_code": "not_a_contradiction"},
|
||||
],
|
||||
)
|
||||
# Simulate the pre-review candidate values the finalizer must overwrite.
|
||||
candidate_path = os.path.join(str(tmp_path), "conflicts.json")
|
||||
with open(candidate_path, encoding="utf-8") as f:
|
||||
candidate = json.load(f)
|
||||
candidate["conflicts"] = [{"description": "kept finding", "severity": "high",
|
||||
"category": "note_or_spec_contradiction"},
|
||||
{"description": "rejected finding", "severity": "medium",
|
||||
"category": "note_or_spec_contradiction"}]
|
||||
candidate["summary"]["conflicts_found"] = 2
|
||||
candidate["summary"]["by_severity"] = {"high": 1, "medium": 1, "low": 0}
|
||||
candidate["summary"]["by_category"] = {"note_or_spec_contradiction": 2}
|
||||
with open(candidate_path, "w", encoding="utf-8") as f:
|
||||
json.dump(candidate, f)
|
||||
|
||||
report = finalize_review("job1", str(tmp_path))
|
||||
assert [c["description"] for c in report["conflicts"]] == ["kept finding"]
|
||||
assert report["summary"]["conflicts_found"] == 1
|
||||
assert report["summary"]["by_severity"] == {"high": 1, "medium": 0, "low": 0}
|
||||
assert report["summary"]["by_category"] == {"note_or_spec_contradiction": 1}
|
||||
suppressed = report["suppressed_issues"]
|
||||
assert [s["issue_id"] for s in suppressed] == ["AGENT-0002"]
|
||||
assert suppressed[0]["review_state"] == "rejected"
|
||||
assert suppressed[0]["reason_code"] == "not_a_contradiction"
|
||||
with open(os.path.join(str(tmp_path), "report.md"), encoding="utf-8") as f:
|
||||
assert "rejected finding" not in f.read()
|
||||
|
||||
|
||||
def test_rerun_missing_cluster_degrades_to_analysis_gap():
|
||||
snapshot = {"findings": [{"issue_id": "AGENT-0001", "scope_id": "conflict:link:1"}],
|
||||
"clusters": []}
|
||||
decisions = {"finding:AGENT-0001": {
|
||||
"decision": "needs_clarification", "clarification_answer": "9'-0\" AFF"}}
|
||||
findings = rerun_clarified_scopes(snapshot, decisions)
|
||||
assert len(findings) == 1
|
||||
assert findings[0]["category"] == "analysis_gap"
|
||||
assert findings[0]["source_stage"] == "qaqc"
|
||||
assert findings[0]["severity"] == "low"
|
||||
assert findings[0]["confidence"] == "high"
|
||||
|
||||
|
||||
def test_rerun_non_conflict_scope_noted_as_analysis_gap():
|
||||
"""v1 only reruns conflict scopes; other scopes get a visible gap, no raise."""
|
||||
snapshot = {"findings": [{"issue_id": "AGENT-0002", "scope_id": "code: egress"}],
|
||||
"clusters": []}
|
||||
decisions = {"finding:AGENT-0002": {
|
||||
"decision": "needs_clarification", "clarification_answer": "Corridor is 44 in."}}
|
||||
findings = rerun_clarified_scopes(snapshot, decisions)
|
||||
assert len(findings) == 1
|
||||
assert findings[0]["category"] == "analysis_gap"
|
||||
|
||||
|
||||
def test_rerun_successful_scope_prepends_clarification_and_tags(monkeypatch):
|
||||
"""Real rerun path (non-LLM): cluster lookup, pseudo-assertion injection,
|
||||
and clarification_of tagging through the real rerun_clarified_scopes."""
|
||||
captured = {}
|
||||
|
||||
class FakeCritic:
|
||||
name = "conflict_critic"
|
||||
|
||||
def __init__(self, usage):
|
||||
pass
|
||||
|
||||
def run(self, scope):
|
||||
captured["scope"] = scope
|
||||
return AgentResult(
|
||||
scope_id=scope.scope_id,
|
||||
artifacts=[{"issue_id": "AGENT-0001-R1", "severity": "medium"}],
|
||||
)
|
||||
|
||||
monkeypatch.setattr("backend.review.finalizer.ConflictCriticAgent", FakeCritic)
|
||||
snapshot = {
|
||||
"findings": [{"issue_id": "AGENT-0001", "scope_id": "conflict:link:1"}],
|
||||
"clusters": [{"key": "link:1",
|
||||
"assertions": [{"attribute": "height", "value": "10'-0\""}]}],
|
||||
}
|
||||
decisions = {"finding:AGENT-0001": {
|
||||
"decision": "needs_clarification", "clarification_answer": "9'-0\" AFF"}}
|
||||
findings = rerun_clarified_scopes(snapshot, decisions)
|
||||
|
||||
assert len(findings) == 1
|
||||
assert findings[0]["issue_id"] == "AGENT-0001-R1"
|
||||
assert findings[0]["clarification_of"] == "AGENT-0001"
|
||||
|
||||
payload = captured["scope"].payload
|
||||
assert payload["page_to_b64"] == {}
|
||||
assertions = payload["cluster"]["assertions"]
|
||||
# Prepended at index 0 so front-truncation can't drop the clarification.
|
||||
assert assertions[0]["discipline"] == "Reviewer"
|
||||
assert assertions[0]["attribute"] == "clarification"
|
||||
assert assertions[0]["value"] == "9'-0\" AFF"
|
||||
assert assertions[1]["attribute"] == "height"
|
||||
|
||||
|
||||
def test_rerun_ignores_other_decisions():
|
||||
decisions = {"finding:AGENT-0001": {"decision": "confirm"}}
|
||||
assert rerun_clarified_scopes({}, decisions) == []
|
||||
@@ -0,0 +1,28 @@
|
||||
from backend.review.gate import build_review_queue
|
||||
|
||||
|
||||
def test_gate_marks_blocking_and_audit_items():
|
||||
memory = {"clusters": [{"key": "room:101", "location": "Room 101", "assertions": [{"id": "a1"}, {"id": "a2"}]}], "findings": []}
|
||||
prioritized = [
|
||||
{"issue_id": "AGENT-0001", "severity": "high", "confidence": "high", "category": "note_or_spec_contradiction", "source_stage": "conflict"},
|
||||
{"issue_id": "AGENT-0002", "severity": "low", "confidence": "high", "category": "note_or_spec_contradiction", "source_stage": "conflict"},
|
||||
]
|
||||
queue = build_review_queue(memory, prioritized, [])
|
||||
by_id = {item["review_item_id"]: item for item in queue}
|
||||
assert by_id["finding:AGENT-0001"]["blocking"] is True
|
||||
assert by_id["finding:AGENT-0002"]["blocking"] is False
|
||||
assert any(item["kind"] == "clean_cluster" for item in queue)
|
||||
|
||||
|
||||
def test_gate_limit_caps_clean_cluster_items():
|
||||
memory = {
|
||||
"clusters": [
|
||||
{"key": f"room:{index}", "assertions": [{"id": "a"}, {"id": "b"}]}
|
||||
for index in range(3)
|
||||
],
|
||||
"findings": [],
|
||||
}
|
||||
queue = build_review_queue(memory, [], [], limit=1)
|
||||
clean_items = [item for item in queue if item["kind"] == "clean_cluster"]
|
||||
assert len(clean_items) == 1
|
||||
assert clean_items[0]["review_item_id"] == "clean_cluster:room:0"
|
||||
@@ -0,0 +1,130 @@
|
||||
from backend import config
|
||||
from backend.review.policy import build_audit_sample, requires_review
|
||||
from backend.review.schemas import validate_decision
|
||||
|
||||
|
||||
def test_high_severity_requires_review():
|
||||
issue = {"severity": "high", "confidence": "high", "category": "note_or_spec_contradiction", "source_stage": "conflict"}
|
||||
assert "severity_high" in requires_review(issue)
|
||||
|
||||
|
||||
def test_low_confidence_requires_review():
|
||||
issue = {"severity": "low", "confidence": "low", "category": "note_or_spec_contradiction", "source_stage": "conflict"}
|
||||
assert "confidence_low" in requires_review(issue)
|
||||
|
||||
|
||||
def test_sensitive_code_category_requires_review():
|
||||
issue = {"severity": "medium", "confidence": "high", "category": "egress", "source_stage": "code"}
|
||||
assert "sensitive_category" in requires_review(issue)
|
||||
|
||||
|
||||
def test_medium_high_confidence_note_does_not_require_review():
|
||||
issue = {"severity": "medium", "confidence": "high", "category": "note_or_spec_contradiction", "source_stage": "conflict"}
|
||||
assert requires_review(issue) == []
|
||||
|
||||
|
||||
def test_build_audit_sample_returns_clean_cluster_spot_check():
|
||||
memory = {
|
||||
"clusters": [
|
||||
{
|
||||
"key": "room:101",
|
||||
"location": "Room 101",
|
||||
"assertions": [{"id": "a1"}, {"id": "a2"}],
|
||||
}
|
||||
],
|
||||
"findings": [],
|
||||
}
|
||||
prioritized = []
|
||||
items = build_audit_sample(memory, prioritized)
|
||||
assert len(items) == 1
|
||||
item = items[0]
|
||||
assert item["kind"] == "clean_cluster"
|
||||
assert item["blocking"] is False
|
||||
assert item["review_item_id"] == "clean_cluster:room:101"
|
||||
|
||||
|
||||
def test_build_audit_sample_strips_base64_from_assertions():
|
||||
memory = {
|
||||
"clusters": [
|
||||
{
|
||||
"key": "room:101",
|
||||
"assertions": [
|
||||
{"id": "a1", "base64": "AAAA"},
|
||||
{"id": "a2", "base64": "BBBB"},
|
||||
],
|
||||
}
|
||||
],
|
||||
"findings": [],
|
||||
}
|
||||
items = build_audit_sample(memory, [])
|
||||
assert len(items) == 1
|
||||
assertions = items[0]["payload"]["assertions"]
|
||||
assert assertions == [{"id": "a1"}, {"id": "a2"}]
|
||||
assert all("base64" not in assertion for assertion in assertions)
|
||||
|
||||
|
||||
def test_build_audit_sample_respects_limit():
|
||||
memory = {
|
||||
"clusters": [
|
||||
{"key": f"room:{index}", "assertions": [{"id": "a"}, {"id": "b"}]}
|
||||
for index in range(4)
|
||||
],
|
||||
"findings": [],
|
||||
}
|
||||
items = build_audit_sample(memory, [], limit=2)
|
||||
assert len(items) == 2
|
||||
assert [item["review_item_id"] for item in items] == [
|
||||
"clean_cluster:room:0",
|
||||
"clean_cluster:room:1",
|
||||
]
|
||||
|
||||
|
||||
def test_build_audit_sample_excludes_implicated_clusters():
|
||||
memory = {
|
||||
"clusters": [
|
||||
{"key": "room:101", "assertions": [{"id": "a1"}, {"id": "a2"}]},
|
||||
{"key": "room:102", "assertions": [{"id": "b1"}, {"id": "b2"}]},
|
||||
],
|
||||
"findings": [{"scope_id": "conflict:room:101"}],
|
||||
}
|
||||
items = build_audit_sample(memory, [])
|
||||
assert [item["review_item_id"] for item in items] == ["clean_cluster:room:102"]
|
||||
|
||||
|
||||
def test_validate_decision_confirm_without_reason_code():
|
||||
result = validate_decision({"review_item_id": "x", "decision": "confirm"})
|
||||
assert result is not None
|
||||
assert result["decision"] == "confirm"
|
||||
assert result["reason_code"] is None
|
||||
|
||||
|
||||
def test_validate_decision_reject_with_valid_reason_code():
|
||||
result = validate_decision({"decision": "reject", "reason_code": "duplicate"})
|
||||
assert result is not None
|
||||
assert result["reason_code"] == "duplicate"
|
||||
|
||||
|
||||
def test_validate_decision_reject_with_missing_reason_code_returns_none():
|
||||
assert validate_decision({"decision": "reject"}) is None
|
||||
|
||||
|
||||
def test_validate_decision_reject_with_invalid_reason_code_returns_none():
|
||||
assert validate_decision({"decision": "reject", "reason_code": "bogus"}) is None
|
||||
|
||||
|
||||
def test_validate_decision_unknown_decision_returns_none():
|
||||
assert validate_decision({"decision": "approve"}) is None
|
||||
|
||||
|
||||
def test_validate_decision_non_dict_returns_none():
|
||||
assert validate_decision("confirm") is None
|
||||
|
||||
|
||||
def test_validate_decision_invalid_reason_code_on_non_reject_returns_none():
|
||||
assert validate_decision({"decision": "confirm", "reason_code": "bogus"}) is None
|
||||
|
||||
|
||||
def test_review_defaults():
|
||||
assert config.AGENT_REQUIRE_REVIEW is True
|
||||
assert config.AGENT_REVIEW_AUDIT_SAMPLE == 5
|
||||
assert config.REVIEW_AGGREGATE_INCLUDE_TEXT is False
|
||||
@@ -0,0 +1,24 @@
|
||||
import json
|
||||
from backend.review.store import ReviewStore
|
||||
|
||||
|
||||
def test_queue_and_decisions_round_trip(tmp_path):
|
||||
store = ReviewStore(str(tmp_path))
|
||||
queue = [{"review_item_id": "finding:1", "blocking": True}]
|
||||
store.write_queue(queue)
|
||||
assert store.read_queue() == queue
|
||||
store.append_decision({"review_item_id": "finding:1", "decision": "confirm"})
|
||||
assert store.read_decisions()["finding:1"]["decision"] == "confirm"
|
||||
|
||||
|
||||
def test_progress_counts_required_items(tmp_path):
|
||||
store = ReviewStore(str(tmp_path))
|
||||
queue = [
|
||||
{"review_item_id": "a", "blocking": True},
|
||||
{"review_item_id": "b", "blocking": False},
|
||||
]
|
||||
store.write_queue(queue)
|
||||
store.append_decision({"review_item_id": "a", "decision": "confirm"})
|
||||
progress = store.progress(queue)
|
||||
assert progress["required"] == 1
|
||||
assert progress["completed"] == 1
|
||||
Reference in New Issue
Block a user