Shared review tier and findings seams
Epic 12251, child C2. Builds the seams shared with the outcome-ledger epic 12243 (operator Q1: routing builds them; the ledger reuses them and must not re-create them). Depends on C3a because `review_findings.routing_decision_id` is a foreign key to C3a's `routing_decisions` table (a real data dependency, not file overlap).
## Original requirement (verbatim)
Ask 5 (part): "Their findings write the ledger's security-findings facts."
Ask 3 (part): "exploration capped at a per-project budget share, default 10%."
## Settled decisions
- D5: the coder runner declares the review tier; CP stores it. A8: declared when it reports its PR (enqueue/settle call); latest declaration wins.
- R8: each finding records severity, file:line, category (security/correctness/test), detecting persona and configuration, authoring configuration; findings show on the coder task page. D2: advisory only.
- R6/A1: per-project exploration share knob (0–100%, default 10%), dollars over rolling 30 days. One knob serves routing exploration and the ledger benchmark budget.
## Approach (Architect §3.2, §6 — pinned)
```
tasks.review_tier text null check in ('trivial','minor','standard','major','security') -- null = undeclared
accepted on pull_request_enqueue (PrQueueIn.review_tier, routes/agent_merge_lane.py:64; ToolArg apps/jaahd/agent_vocabulary.py:392; merge_lane.report_pr kw)
AND task_complete/task_fail (SettleIn.review_tier, task_terminal.py:256; _TOOL_SETTLE :246)
review_findings
id uuid pk, org_id uuid (OrgScoped), task_id bigint fk tasks (AUTHORING task), detecting_task_id bigint null fk tasks
detecting_lane text check in ('blind_reviewer','blind_security_reviewer','blind_tester','in_session_reviewer','in_session_security_reviewer','in_session_verifier','ci_scan')
detecting_config jsonb, authoring_config jsonb, routing_decision_id uuid null fk routing_decisions
severity text check in ('blocker','major','minor','info'), category text check in ('security','correctness','test')
file text null, line int null, title text, detail text, head_sha text null, created_at timestamptz
index (task_id), index (org_id, created_at)
projects.experiment_budget_share numeric(4,3) not null default 0.100 check 0..1
```
```python
# apps/api/app/review_findings.py
def declare_tier(session, task, tier: str|None) -> None # the ONE setter; fires hooks
def on_tier_declared(fn: Callable[[Session, Task, str], None]) -> None # C6a registers
def record(session, task, findings: list[FindingIn], *, lane: str, detecting_task=None, detecting_config: dict, routing_decision_id=None) -> list[UUID]
class FindingIn(StrictModel): severity: Literal['blocker','major','minor','info']; category: Literal['security','correctness','test']; file: str|None; line: int|None; title: str; detail: str
```
- Runner tool args: `pull_request_enqueue` +`review_tier?`; `task_complete`/`task_fail` +`review_tier?` and +`findings?: FindingIn[]` (optional for coders; C6a makes it required for terminal `review`). Update the jaahd vocabulary and its parity tests; add one line to `portfolio/fleet/runner-charter.md` telling coder runners to declare `review_tier` per `docs/policies/code-review-policy.md` (do NOT change that policy's tier definitions).
- Single-writer test: no module other than `review_findings` inserts into `review_findings`.
- UI: the "Experiment budget" field in the project Routing group (C1 built the group); a "Review findings" tab on task detail listing findings with severity, file:line, category, detecting and authoring configuration (display names).
Credentials & reach:
- Dev UI flow (findings tab): `me/jaah/debug/main/operator-login` and `org/console/dev-login` — verified.
- API tests: the worktree only — verified.
Out of scope: changing account quota semantics or spillover; training a model from scratch; routing analysis personas before coder tasks have data; public marketplace billing; any ledger metric definition (owned by the outcome ledger epic). MUST NOT create `task_outcomes` or any ledger writer. MUST NOT make findings block, delay or gate a PR, the merge lane or a settle. Do NOT spawn children (C6a).
Done means:
- AC12 Findings recorded (pre-ledger part). → Verify: on dev, the coder task page lists the child's findings with severity, file:line, category, the detecting configuration and the authoring configuration. (Full dev observation completes with C6a; here an API test records findings via `task_complete(findings=...)` and the tab renders them.)
- API tests: `review_tier` accepted on enqueue and settle, latest wins, hooks fire once per declaration; invalid tier → 422; findings written only by `review_findings.record` (single-writer test); `experiment_budget_share` bounds 0..1 validated.
- Quality floor: the tab shows display names (no raw config JSON or enum slugs), wraps at 390px without page-level horizontal scroll, follows ui-style-guide/ui-copy-standards.
- Gates (AC16): `./dc check quick` green; changed-area suites (apps/api, apps/jaahd, apps/web) green; merge lane `./dc check full` green.
Activity
- Draft
- Ready