Files
roboco/tests/unit/runtime/test_revision_findings_prompt.py
T
cea3e56628 feat(lifecycle): revision findings ledger — structured failure feedback, persisted and delivered down the chain (#486)
* feat(lifecycle): revision findings ledger — structured QA/PR/PM/CEO failure feedback, persisted and delivered down the chain

Every bounce used to survive only as flattened prose: rounds overwrote each
other in notes_structured, request_changes persisted nothing, two raw
dev_notes appends were silently destroyed by the next handoff note, and the
dev prompt pointed at fields (qa_notes via evidence(), pm_notes) the API
never delivered. Agents re-interpreted and re-discovered every failure
before they could start fixing it.

- task_review_findings (migration 071, append-only): file/line/severity/
  criterion(AC-id-validated)/expected/actual/fix/evidence per finding, with
  origin (qa|pr_gate|pm|ceo), round, and an open->addressed->verified
  lifecycle (waived reserved); new tasks.pm_notes + PmReviewContent give
  request_changes a structured home
- producers: fail_review/pr_fail/request_changes take findings=[...] (prose
  issues shimmed+merged for one release, deprecation-logged); ceo_reject
  validates its reason (no 500), lands an origin=ceo finding, and bumps
  round+audit on branchless coordination roots; guardrails at the verb
  chokepoint (nudge >5, hard reject >10, field caps, traversal-safe file);
  the dev_notes data-loss appends are removed; new task.request_changes +
  task.ceo_reject audit events close rework attribution
- delivery: qa_notes/pr_reviewer_notes/pm_notes carry the deterministic
  [F-id8] rendering; claim briefings, evidence(), the REVISION_REQUIRED
  spawn prompt, PM triage bounced-blocks, and A2A bodies deliver open
  findings; round-N+1 QA and gate reviewers get the full prior ledger;
  panel Findings tab + bounced-xN chip; metrics pm_rejects/ceo_rejects +
  findings counts; vault task notes render a Findings section (fail-open)
- resolution closes for every origin: i_am_done and submit_up/submit_root
  take resolved_findings gated by FINDINGS_ADDRESSED (owner-gated so a
  stale non-owner PM can never mutate the ledger); pass_review/pr_pass/
  complete verify-stamp same-transaction; ceo_approve stamps best-effort
- 24 real-DB integration tests drive the full loop through the real
  choreographer; full suite 12856 green

* docs: revision findings ledger sweep — CLAUDE.md, map, RAG corpus

- CLAUDE.md: new ledger section + corrected request_changes row
- docs/map/review-findings.md (new subsystem map) + surgical updates to
  task-service/pr-gate-review/metrics-observability/vault/panel maps
- docs/rag: producers' findings contract across qa/pr-reviewer/developer/
  cell-pm/main-pm/ceo role docs (the PM docs were missing request_changes
  entirely), verb references, and a new architecture/review-findings.md
  disambiguating ledger findings from convention findings

* test(e2e): resubmit resolves the pr_fail finding per the ledger contract

The scripted pr_fail revision loop resubmitted submit_up without
resolved_findings — correctly rejected now that FINDINGS_ADDRESSED gates
the PM resubmit verbs (green locally, red only in CI since the e2e suite
skips without ROBOCO_E2E_SMOKE=1). The scripted PM now reads the open
ledger row pr_fail persisted (new open_finding_ids arc helper) and
resolves it on resubmit, asserting the open set drains — exercising the
coordinator half of the new contract end to end.

---------

Co-authored-by: Renn F <rennf93@users.noreply.github.com>
2026-07-11 22:54:42 +02:00

246 lines
7.8 KiB
Python

"""Dispatch prompts render the revision-findings ledger inline.
The REVISION_REQUIRED block (developer respawn) and the PM triage prompts'
bounced-block (cell_pm / main_pm respawn onto a needs_revision root) both
render the task's open ledger findings — id, file:line, expected -> actual
-> fix — instead of pointing at ``qa_notes`` / ``pm_notes`` fields that
``evidence()`` never populated.
"""
from __future__ import annotations
from contextlib import asynccontextmanager
from typing import TYPE_CHECKING, Any
from unittest.mock import AsyncMock, patch
from uuid import uuid4
import pytest
from roboco.runtime.orchestrator import (
_PROMPT_FINDINGS_CAP,
AgentOrchestrator,
)
if TYPE_CHECKING:
from collections.abc import AsyncIterator
def _orch() -> AgentOrchestrator:
orch = object.__new__(AgentOrchestrator)
orch._instances = {}
return orch
class _Row:
"""A bare stand-in for a ``TaskReviewFindingTable`` row."""
def __init__(
self,
*,
file: str | None = "roboco/services/task.py",
line: int | None = 42,
expected: str = "raises ValueError",
actual: str = "swallows the error",
fix: str | None = "add the raise",
) -> None:
self.id = uuid4()
self.file = file
self.line = line
self.expected = expected
self.actual = actual
self.fix = fix
def _patch_findings_repo(rows: list[_Row]) -> tuple[Any, Any]:
@asynccontextmanager
async def _fake_ctx() -> AsyncIterator[AsyncMock]:
yield AsyncMock()
repo = AsyncMock()
repo.list_for_task = AsyncMock(return_value=rows)
return (
patch("roboco.db.base.get_db_context", _fake_ctx),
patch(
"roboco.services.repositories.review_findings.ReviewFindingsRepository",
return_value=repo,
),
)
# ---------------------------------------------------------------------------
# _open_findings_prompt_block
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_renders_file_line_expected_actual_fix() -> None:
orch = _orch()
row = _Row()
db_ctx, repo_ctx = _patch_findings_repo([row])
with db_ctx, repo_ctx:
block = await orch._open_findings_prompt_block(str(uuid4()))
assert "roboco/services/task.py:42" in block
assert "raises ValueError" in block
assert "swallows the error" in block
assert "add the raise" in block
assert f"F-{str(row.id)[:8]}" in block
@pytest.mark.asyncio
async def test_empty_ledger_returns_empty_string() -> None:
orch = _orch()
db_ctx, repo_ctx = _patch_findings_repo([])
with db_ctx, repo_ctx:
assert await orch._open_findings_prompt_block(str(uuid4())) == ""
@pytest.mark.asyncio
async def test_no_task_id_returns_empty_string_without_db() -> None:
orch = _orch()
with patch("roboco.db.base.get_db_context") as ctx:
assert await orch._open_findings_prompt_block("") == ""
ctx.assert_not_called()
@pytest.mark.asyncio
async def test_caps_at_ten_with_overflow_line() -> None:
orch = _orch()
rows = [_Row(actual=f"issue {i}") for i in range(_PROMPT_FINDINGS_CAP + 3)]
db_ctx, repo_ctx = _patch_findings_repo(rows)
with db_ctx, repo_ctx:
block = await orch._open_findings_prompt_block(str(uuid4()))
lines = block.splitlines()
assert len(lines) == _PROMPT_FINDINGS_CAP + 1 # 10 findings + overflow line
assert "+3 more via evidence()" in lines[-1]
@pytest.mark.asyncio
async def test_db_error_fails_open_to_empty_string() -> None:
orch = _orch()
with patch("roboco.db.base.get_db_context", side_effect=RuntimeError("db down")):
assert await orch._open_findings_prompt_block(str(uuid4())) == ""
# ---------------------------------------------------------------------------
# _build_dev_prompt — REVISION_REQUIRED renders the block inline
# ---------------------------------------------------------------------------
def _task(**over: Any) -> dict[str, Any]:
base: dict[str, Any] = {
"id": str(uuid4()),
"title": "Fix the parser",
"status": "needs_revision",
"plan": "did the thing",
}
base.update(over)
return base
@pytest.mark.asyncio
async def test_revision_required_prompt_embeds_seeded_finding() -> None:
orch = _orch()
task = _task()
row = _Row(file="api/routes/foo.py", line=17, actual="missing null guard")
db_ctx, repo_ctx = _patch_findings_repo([row])
with db_ctx, repo_ctx:
prompt = await orch._build_dev_prompt(task)
assert "api/routes/foo.py:17" in prompt
assert "missing null guard" in prompt
assert "qa_notes" not in prompt
assert "pm_notes" not in prompt
@pytest.mark.asyncio
async def test_non_revision_prompt_never_touches_db() -> None:
"""EXECUTING (in_progress) must not pay for a findings-ledger fetch."""
orch = _orch()
task = _task(status="in_progress")
with patch("roboco.db.base.get_db_context") as ctx:
prompt = await orch._build_dev_prompt(task)
ctx.assert_not_called()
assert "IN PROGRESS" in prompt
@pytest.mark.asyncio
async def test_revision_required_with_no_findings_still_renders() -> None:
orch = _orch()
task = _task()
db_ctx, repo_ctx = _patch_findings_repo([])
with db_ctx, repo_ctx:
prompt = await orch._build_dev_prompt(task)
assert "REVISION REQUESTED" in prompt
assert "no findings on the ledger" in prompt
# ---------------------------------------------------------------------------
# PM triage prompts — the bounced-block
# ---------------------------------------------------------------------------
def test_pm_triage_prompt_prepends_bounced_block_when_given() -> None:
orch = _orch()
prompt = orch._build_pm_triage_prompt(
_task(team="backend"), bounced_block="[F-abcd1234] api.py:9 — x -> y"
)
assert prompt.startswith("## THIS TASK BOUNCED")
assert "[F-abcd1234] api.py:9" in prompt
assert "You are the PM for backend team" in prompt
def test_pm_triage_prompt_omits_block_when_empty() -> None:
orch = _orch()
prompt = orch._build_pm_triage_prompt(_task(team="backend"), bounced_block="")
assert "THIS TASK BOUNCED" not in prompt
assert prompt.startswith("You are the PM for backend team")
def test_main_pm_triage_prompt_prepends_bounced_block_when_given() -> None:
orch = _orch()
prompt = orch._build_main_pm_triage_prompt(
_task(), bounced_block="[F-abcd1234] api.py:9 — x -> y"
)
assert prompt.startswith("## THIS ROOT BOUNCED")
assert "[F-abcd1234] api.py:9" in prompt
assert "You are the MAIN PM at RoboCo" in prompt
def test_main_pm_triage_prompt_omits_block_when_empty() -> None:
orch = _orch()
prompt = orch._build_main_pm_triage_prompt(_task(), bounced_block="")
assert "THIS ROOT BOUNCED" not in prompt
assert prompt.startswith("You are the MAIN PM at RoboCo")
# ---------------------------------------------------------------------------
# _get_prompt_for_agent — threads the bounced-block into cell_pm/main_pm
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_get_prompt_for_agent_fetches_bounced_block_for_needs_revision_pm(
monkeypatch: pytest.MonkeyPatch,
) -> None:
orch = _orch()
monkeypatch.setattr(
orch, "_revision_bounced_block", AsyncMock(return_value="[F-11112222] x")
)
prompt = await orch._get_prompt_for_agent("main-pm", _task(status="needs_revision"))
assert "[F-11112222] x" in prompt
@pytest.mark.asyncio
async def test_revision_bounced_block_skips_fetch_when_not_needs_revision() -> None:
orch = _orch()
with patch("roboco.db.base.get_db_context") as ctx:
block = await orch._revision_bounced_block(_task(status="in_progress"))
assert block == ""
ctx.assert_not_called()
if __name__ == "__main__":
pytest.main([__file__, "-q"])