Files
roboco/docs/map/review-findings.md
80ccf415cb feat(cockpit): expose first_pass_yield and a real escaped-defects metric (#709)
The Company Scorecard renders three charter objectives but the cockpit
summary only ever carried one of the metrics, so two cards read "No data
yet" permanently.

first_pass_yield is a pass-through — MetricsService.get_org_scorecard()
already computes it on the same 30d/org scope the rest of the delivery block
uses, and CockpitService.summary simply never forwarded it.

escaped_defects is new. The obvious definition — a blocker finding opened on
a task that already reached a terminal state — is unimplementable: every
producer of a task_review_findings row fires as part of a bounce whose
transition requires a non-terminal task, so it would read zero forever, and a
permanently-green card is the same fabrication the panel change removes.

What it counts instead: a blocker still at 'addressed', never 'verified', on
a task that has since completed. That is reachable because
stamp_addressed_verified only bulk-verifies rows matching its OWN origin, so
a blocker raised by one origin and never re-confirmed by that origin survives
to completion on the developer's word alone.

docs/map/metrics-observability.md documents what a zero actually means: the
one reachable trigger is a PM-origin blocker on a task escalated to the CEO
rather than completed by the PM, since escalate_to_ceo carries no
findings-resolved precondition and ceo_approve verifies only ceo-origin rows.
It also records that the count is per-finding over a rolling 30-day window,
which is not the same unit as the charter's "per release".

Co-authored-by: Renn F <rennf93@users.noreply.github.com>
2026-07-26 19:11:27 +02:00

182 lines
24 KiB
Markdown

# RoboCo Map — `review-findings` slice
## Purpose
The revision-findings ledger: the structured replacement for prose-only QA/PR-gate/PM/CEO bounce feedback. Before this slice, a rejection was an `issues: list[str]` flattened into free text (or, for `request_changes`/`ceo_reject`, not even that — a raw `dev_notes` append the next developer handoff note silently overwrote). Now all four failure producers (`fail_review`, `pr_fail`, `request_changes`, `ceo_reject`) validate structured `Finding`s, persist one append-only row per finding to `task_review_findings` (migration 071), render a deterministic per-finding summary into the existing note-mirror columns (`qa_notes`/`pr_reviewer_notes`/the new `pm_notes`), and deliver the open ledger to every downstream consumer — the bounced dev's `evidence()`, a round-2+ reviewer's `claim_review`/`claim_gate_review`, the orchestrator's respawn prompts, the panel's Findings tab, metrics, and vault task notes. Always-on — no feature flag, this is core delivery lifecycle.
## Files
| Path | Role | LOC |
|---|---|---|
| `alembic/versions/071_review_findings.py` | Migration: `task_review_findings` table + `tasks.pm_notes` column | 81 |
| `roboco/db/tables.py` | `TaskReviewFindingTable` ORM class + `TaskTable.pm_notes` column | (slice ~60 lines within a shared file) |
| `roboco/services/repositories/review_findings.py` | `ReviewFindingsRepository` — insert/list/aggregate/mark_addressed/mark_verified/mark_waived | 164 |
| `roboco/foundation/policy/content/enums.py` | `Severity` StrEnum (`blocker`/`major`/`minor`/`nit`) | 35 |
| `roboco/foundation/policy/content/models.py` | `Finding` (extended: `fix`/`evidence`, caps, repo-relative `file`), `QaNote.findings`, new `PmReviewContent` ("pm_review"), `validate_findings` | 614 (whole content-model file) |
| `roboco/services/content_notes.py` | `_MIRROR_COLUMN["pm_review"] = "pm_notes"` | (1-line addition within a shared file) |
| `roboco/services/gateway/choreographer/findings.py` | Shared producer helpers: shim, count guard, criterion check, ledger insert/render, verify-stamp | 256 |
| `roboco/services/gateway/choreographer/qa.py` | `fail_review` — QA-origin producer | (slice within a shared file) |
| `roboco/services/gateway/choreographer/pr_gate.py` | `pr_fail` — pr_gate-origin producer; `pr_pass` — verify-stamp | (slice within a shared file) |
| `roboco/services/gateway/choreographer/_impl.py` | `request_changes` — pm-origin producer; `i_am_done`/`submit_up`/`submit_root``resolved_findings` resolution; `complete` — verify-stamp | (slice within a shared file) |
| `roboco/services/task.py` | `ceo_reject` — ceo-origin producer + reason validation; `ceo_approve` — best-effort verify-stamp; `_audit_events_for``task.request_changes`/`task.ceo_reject`; `qa_fail`/`request_changes` dev_notes-append removal | (slice within an 8.7k-line file) |
| `roboco/foundation/policy/tracing.py` | `Requirement.FINDINGS_ADDRESSED`, `GateContext.open_finding_ids`, `_check_findings_addressed` | (slice within a shared file) |
| `roboco/services/gateway/evidence_builder.py` | `EvidencePayload.revision_findings`/`.prior_findings`, `build_task_handoff` findings, `render_findings`, `_extract_qa_review`/`_extract_pm_review` | 393 |
| `roboco/services/gateway/remediation.py` | `hint_for_open_findings` | 111 |
| `roboco/runtime/orchestrator.py` | `REVISION_REQUIRED` prompt findings block, PM triage "bounced" block, `_open_findings_prompt_block`/`_revision_bounced_block` | (slice within a shared file) |
| `roboco/mcp/flow_server.py` | MCP param wiring: `findings`/`resolved_findings` on the 6 affected tools | (slice within a shared file) |
| `roboco/api/schemas/v1/flow.py` | `ResolvedFindingInput`; `findings`/`resolved_findings` fields on the flow request schemas | (slice within a shared file) |
| `roboco/api/routes/tasks.py` | `GET /{task_id}/findings` route | (slice within a shared file) |
| `roboco/api/schemas/tasks.py` | `TaskFindingResponse`/`TaskFindingsSummaryRow`/`TaskFindingsResponse`; `TaskResponse.revision_count`/`.pm_notes` | 1035 (whole schema file) |
| `roboco/services/metrics.py` | `_REWORK_EVENT_TYPES`/`_REWORK_EVENT_TO_FIELD`, `pm_rejects`/`ceo_rejects` on `AgentReworkRate`, `findings_open`/`findings_total` on `get_task_metrics` | (slice within a shared file) |
| `roboco/models/metrics.py` | `AgentReworkRate`/`TaskMetrics` gain `pm_rejects`/`ceo_rejects`(/`findings_open`/`findings_total`) | (slice within a shared file) |
| `roboco/services/vault_assembly.py` | `_resolve_findings` — fail-open findings fetch for `TaskNoteData` | (slice within a shared file) |
| `roboco/services/vault_writer.py` | `FindingRow`, `_FINDINGS_CAP=20`, `_findings_section` — task note `## Findings` | (slice within a shared file) |
| `panel/src/components/tasks/task-detail/tab-findings.tsx` | `TabFindings` — per-round findings view | 169 |
| `panel/src/components/tasks/task-detail/task-header.tsx` | `bounced xN` chip (`revision_count`) | (slice within a shared file) |
| `panel/src/components/tasks/task-detail/task-tabs.tsx` | "Findings" tab wiring | (slice within a shared file) |
| `panel/src/hooks/use-tasks.ts` / `panel/src/lib/api/tasks.ts` | `useTaskFindings` / `tasksApi.getFindings` | (slice within shared files) |
| `panel/src/components/metrics/delivery-tab.tsx` | "PM rejects"/"CEO rejects" rework columns | (slice within a shared file) |
## Key Symbols
| Name | Kind | File:Line | Responsibility |
|---|---|---|---|
| `TaskReviewFindingTable` | ORM class | `roboco/db/tables.py` | The append-only ledger row: `task_id`, `origin`, `round`, `author_slug`, `file`/`line`/`severity`/`criterion`/`expected`/`actual`/`fix`/`evidence`, `status`, `addressed_by_commit`, `resolution_note`. `origin`/`severity`/`status` are plain `String` columns, not a native Postgres enum. |
| `Finding` | Pydantic model | `roboco/foundation/policy/content/models.py:92` | One structured finding, shared by `post_pr_review` (external PRs) and the four internal producers. Caps: `file` ≤300 (repo-relative, no `..`), `line` ≥1, `expected`/`actual` ≤300, `fix` ≤500, `evidence` ≤2000. `criterion` has **no Pydantic `max_length`** despite the DB column being `String(500)` — see Regression Risks. `file` is additionally shape-gated (`_PATH_SHAPE_RE`, #687): a value that doesn't look like a repo-relative path (prose like a PR reference, which used to validate and then doomed the panel's code-snippet fetch) is rejected with a remediate naming the file-less option for cross-cutting findings; the class admits `+`/`@` (SvelteKit route files, `@types` dirs, `@2x` assets) but excludes spaces, the prose signal. `file` remains OPTIONAL for the `issues` shim's file-less findings. |
| `PmReviewContent` | Pydantic model | `roboco/foundation/policy/content/models.py` | New content type `"pm_review"` (`summary` + `findings`, no separate `verdict` — the transition to `needs_revision` IS the verdict); mirrors to the new `tasks.pm_notes` column via `_MIRROR_COLUMN`. |
| `ReviewFindingsRepository` | class | `roboco/services/repositories/review_findings.py:32` | `insert_many` (append rows, one flush, no independent commit), `list_for_task` (default cap 500, newest round first), `status_counts_for_task` (SQL `GROUP BY (origin, status)`, whole ledger — independent of the 500 cap), `mark_addressed` (8-char-prefix match against OPEN rows, no-op on 0 or >1 matches, never raises), `mark_verified` (bulk, by full id), `mark_waived` (exists, unwired — no verb calls it), `escaped_defects_since` (`(task_id, origin)` for blocker findings still `addressed`, never `verified`, on a task that has since gone `COMPLETED` within a window — the Company Scorecard's `escaped_defects` metric; see `docs/map/metrics-observability.md`). |
| `findings_count_guard` / `findings_count_hint` | functions | `roboco/services/gateway/choreographer/findings.py:98,115` | Hard-reject `Envelope` above `FINDINGS_HARD_CAP=10`; non-blocking hint above `FINDINGS_NUDGE_COUNT=5`. |
| `issues_to_findings` / `merge_findings_and_issues` | functions | `roboco/services/gateway/choreographer/findings.py:57,81` | Legacy `issues: list[str]` shim → file-less `severity=major` findings (deprecation-logged); merges with any `findings` sent in the same call rather than one silently dropping the other. |
| `next_round` | function | `roboco/services/gateway/choreographer/findings.py:43` | `(task.revision_count or 0) + 1`, read BEFORE the transition — the round a finding written during this call belongs to. |
| `unknown_finding_criteria` / `criterion_mismatch_rejection` | functions | `roboco/services/gateway/choreographer/findings.py:127,147` | A supplied `criterion` must match an `acceptance_criteria_ids` entry or exact AC text, or the call is rejected. |
| `insert_and_render` / `render_findings_summary` / `render_finding_line` | functions | `roboco/services/gateway/choreographer/findings.py:186,176,163` | Ledger insert chokepoint (all 3 choreographer producers share it) + the deterministic `[F-id8] file:line (severity) — expected → actual → fix` rendering used for both the structured note's `summary` and the A2A body. |
| `stamp_addressed_verified` | function | `roboco/services/gateway/choreographer/findings.py:243` | Bulk-promotes `addressed→verified` for one `origin` on a task; not best-effort except at the `ceo_approve` call site. |
| `fail_review` | async verb | `roboco/services/gateway/choreographer/qa.py` | QA-origin producer: `(qa_agent_id, task_id, issues=None, findings=None)`. |
| `pr_fail` | async verb | `roboco/services/gateway/choreographer/pr_gate.py` | pr_gate-origin producer, same shape; findings insert happens before `_record_gate_verdict_for` so the verdict note reads real ledger ids. |
| `request_changes` | async verb | `roboco/services/gateway/choreographer/_impl.py` | pm-origin producer, same shape; writes `PmReviewContent``pm_notes`. |
| `ceo_reject` | method | `roboco/services/task.py` | ceo-origin producer — NOT a choreographer verb (no gateway wrapper; the CEO-approval route calls `TaskService.ceo_reject` directly). Validates `reason` via `reject_trivial` (previously an uncaught Pydantic `ValidationError` → 500 on empty/trivial input) and converts it into one `severity=blocker` Finding. |
| `_bump_coordination_ceo_reject` | method | `roboco/services/task.py` | Manually bumps `revision_count` + inserts a `task.ceo_reject` audit row for a branchless-coordination reject, since that path routes to `PENDING` via `admin_set_status` and never reaches the normal `_emit_status_transition_audit` chokepoint. |
| `_audit_events_for` | staticmethod | `roboco/services/task.py:997` | Gains `task.request_changes` (agent_role `cell_pm`/`main_pm`) and `task.ceo_reject` (agent_role `ceo`) branches alongside the existing `task.qa_fail`/`task.pr_fail`. |
| `Requirement.FINDINGS_ADDRESSED` / `_check_findings_addressed` | enum + checker | `roboco/foundation/policy/tracing.py` | `VERB_REQUIREMENTS["i_am_done"/"submit_up"/"submit_root"]` gain this — every still-OPEN finding id (computed by the choreographer AFTER applying this call's `resolved_findings`) must be resolved or the call rejects. |
| `EvidencePayload.revision_findings` / `.prior_findings` | fields | `roboco/services/gateway/evidence_builder.py` | Open findings (any consumer) vs the FULL ledger (`claim_review`/`claim_gate_review` only, for round-2+ review). Both omitted from `as_dict()` when empty. |
| `build_task_handoff` / `_extract_qa_review` / `_extract_pm_review` | functions | `roboco/services/gateway/evidence_builder.py` | `open_findings` param renders into `handoff["revision_findings"]`; `_extract_qa_review`/`_extract_pm_review` are new siblings of `_extract_pr_review`, each surfacing `{findings_count, verdict?, summary?}` from `notes_structured`. |
| `hint_for_open_findings` | function | `roboco/services/gateway/remediation.py:36` | Builds the `i_am_done(resolved_findings=[...])` remediation string naming every still-open `[F-id8]`. |
| `_open_findings_prompt_block` / `_revision_bounced_block` | functions | `roboco/runtime/orchestrator.py` | Capped-10 open-findings render for the `REVISION_REQUIRED` dev prompt and the PM triage "bounced" block; fails open to `""` on any DB error. |
| `GET /{task_id}/findings` | route | `roboco/api/routes/tasks.py` | `list_for_task` (cap 500) + `status_counts_for_task` (SQL aggregate, whole ledger) → `TaskFindingsResponse{findings, summary, total, truncated}`. No role restriction beyond an authenticated agent context. |
| `TabFindings` | React component | `panel/src/components/tasks/task-detail/tab-findings.tsx` | Per-round **collapsible** sections (newest expanded, #687), origin summary badges, severity/status badges, file:line, collapsible evidence, truncation footer; only attempts a code-snippet fetch for a path-shaped `file` — a historical prose ref (pre-#687 data) renders as plain metadata instead of a doomed loader. |
## Data Flow
**Producing a finding.** A reviewer/PM/CEO calls their fail verb with `findings: list[dict]` (or the deprecated `issues: list[str]` shim). The verb validates the merged list against `Finding` (`validate_findings`), runs the count guard (hard-reject >10, hint >5) and the criterion-match check, computes `round = revision_count + 1`, inserts one row per finding via `ReviewFindingsRepository.insert_many` (same transaction, no independent commit), renders the deterministic summary, writes the origin's structured note (`QaNote`/`PrReviewContent`/`PmReviewContent`) which mirrors into `qa_notes`/`pr_reviewer_notes`/`pm_notes`, then runs the normal transition to `needs_revision` — which is where `_emit_status_transition_audit` bumps `revision_count` for real and emits the rejector-attributed audit event (`task.qa_fail`/`task.pr_fail`/`task.request_changes`/`task.ceo_reject`). `ceo_reject` on a branchless coordination root is the one path that bypasses this chokepoint (routes via `admin_set_status`), so it manually mirrors the bump + audit emit.
**Resolving a finding.** The bounced dev/PM calls `i_am_done`/`submit_up`/`submit_root` with `resolved_findings: list[{finding_id, commit?, note?}]`. Each entry is applied via `ReviewFindingsRepository.mark_addressed` (8-char-prefix match against currently-OPEN rows, no-op on zero/ambiguous match) BEFORE the tracing gate re-reads still-open ids (write-then-gate) — any id left open rejects the call, naming it. A subsequent same-origin pass (`pass_review`/`pr_pass`/`complete`) bulk-verifies (`addressed→verified`) every `addressed` row of its own origin in the same transaction (not best-effort — a stamp failure fails the verb); `ceo_approve` does the equivalent for `origin=ceo`, but best-effort.
**Delivery.** Every read path that already existed grew a findings slot: the generic `evidence()` do-tool and `i_am_done`'s pre-transition evidence both carry `revision_findings` (open only); `claim_review`/`claim_gate_review` additionally carry `prior_findings` (the full ledger, so a round-2+ reviewer checks prior findings instead of re-deriving them). The orchestrator's respawn path renders open findings inline into the `REVISION_REQUIRED` dev prompt and a "bounced" block prepended to PM triage prompts. The panel's `GET /{task_id}/findings` route backs a dedicated Findings tab plus a `bounced xN` header chip. `MetricsService` extends its rework-by-agent aggregation to the two new audit events and adds a second, separate query for per-task findings counts. Vault task notes render a capped `## Findings` section, fail-open on any fetch error.
## Mermaid
```mermaid
stateDiagram-v2
direction LR
[*] --> Produce: fail_review / pr_fail / request_changes / ceo_reject
Produce --> Ledger: insert_many (origin, round=revision_count+1, status=open)
Produce --> Note: structured note (summary = rendering) -> qa_notes/pr_reviewer_notes/pm_notes
Produce --> Transition: needs_revision (audit: task.qa_fail/pr_fail/request_changes/ceo_reject)
Transition --> Deliver: evidence()/build_task_handoff (revision_findings=open)
Deliver --> Resolve: i_am_done/submit_up/submit_root(resolved_findings=[...])
Resolve --> Ledger2: mark_addressed (open->addressed)
Resolve --> Gate: FINDINGS_ADDRESSED (reject if any still open)
Gate --> Reviewed: pass_review/pr_pass/complete
Reviewed --> Ledger3: stamp_addressed_verified (addressed->verified, own origin)
Reviewed --> RoundN1: claim_review/claim_gate_review (prior_findings=full ledger)
```
## Logical Tree
```
review-findings slice
├── Data model
│ ├── task_review_findings (migration 071) + tasks.pm_notes
│ ├── TaskReviewFindingTable (db/tables.py)
│ └── Finding (extended: fix/evidence/caps) + QaNote.findings + PmReviewContent ("pm_review") + validate_findings
├── Repository
│ └── ReviewFindingsRepository (insert_many/list_for_task/status_counts_for_task/mark_addressed/mark_verified/mark_waived[unwired])
├── Shared producer helpers (choreographer/findings.py)
│ ├── issues_to_findings / merge_findings_and_issues (shim)
│ ├── findings_count_guard (hard cap 10) / findings_count_hint (nudge 5)
│ ├── unknown_finding_criteria / criterion_mismatch_rejection
│ ├── next_round / insert_and_render / render_findings_summary / render_finding_line
│ └── open_findings_for_task / full_ledger_for_task / stamp_addressed_verified
├── Producers
│ ├── fail_review (qa.py, origin=qa)
│ ├── pr_fail (pr_gate.py, origin=pr_gate)
│ ├── request_changes (_impl.py, origin=pm) -> pm_notes
│ └── ceo_reject (task.py, origin=ceo) -> reason validation + coordination-root bump
├── Resolution
│ ├── i_am_done / submit_up / submit_root (resolved_findings)
│ ├── Requirement.FINDINGS_ADDRESSED (tracing.py)
│ └── pass_review / pr_pass / complete (verify-stamp own origin) + ceo_approve (best-effort)
├── Delivery
│ ├── evidence() / build_task_handoff (revision_findings)
│ ├── claim_review / claim_gate_review (prior_findings)
│ ├── orchestrator REVISION_REQUIRED + PM triage bounced block
│ ├── A2A fail bodies (shared rendering)
│ ├── GET /api/tasks/{id}/findings -> panel Findings tab + bounced xN chip
│ ├── MetricsService (pm_rejects/ceo_rejects + findings_open/findings_total)
│ └── vault task note ## Findings section (capped 20, fail-open)
```
## Dependencies
- `roboco.foundation.policy.content``Finding`, `Severity`, `QaNote`, `PrReviewContent`, `PmReviewContent`, `validate_findings`, `CONTENT_MODELS`
- `roboco.services.content_notes``apply_structured_note` + `_MIRROR_COLUMN`
- `roboco.services.repositories.base``BaseRepository`
- `roboco.foundation.policy.tracing``Requirement`, `GateContext`, `VERB_REQUIREMENTS`
- `roboco.services.gateway.envelope``Envelope`
- `roboco.services.gateway.evidence_builder``BRIEFING_LIST_CAP`
- `roboco.db.tables``TaskReviewFindingTable`, `TaskTable.pm_notes`
- Consumed by: `roboco.services.metrics` (`MetricsService`), `roboco.services.cockpit` (`CockpitService.summary`'s `escaped_defects` field), `roboco.services.vault_assembly`/`vault_writer`, `roboco.runtime.orchestrator`, `roboco.mcp.flow_server`, `roboco.api.routes.tasks`/`v1.flow_*`, the panel's `task-detail`/`metrics` components
## Entry Points
| Name | File | Trigger |
|---|---|---|
| `fail_review` | `choreographer/qa.py` | QA calls `fail(task_id, findings=[...])` via `roboco-flow` MCP / HTTP |
| `pr_fail` | `choreographer/pr_gate.py` | PR reviewer calls `pr_fail(task_id, findings=[...])` on a claimed gate review |
| `request_changes` | `choreographer/_impl.py` | Cell/Main PM calls `request_changes(task_id, findings=[...])` on an `awaiting_pm_review` task |
| `ceo_reject` (`TaskService` method, no verb) | `roboco/services/task.py` | CEO-approval-chain route on `awaiting_ceo_approval` |
| `i_am_done` / `submit_up` / `submit_root` | `choreographer/_impl.py` | Dev/PM resolves findings via `resolved_findings=[...]` on resubmit |
| `GET /api/tasks/{id}/findings` | `roboco/api/routes/tasks.py` | Panel's `useTaskFindings` hook, any authenticated agent context |
## Config Flags
None — always-on, core lifecycle (no `ROBOCO_*` gate). The only tunables are compile-time constants: `FINDINGS_NUDGE_COUNT=5` / `FINDINGS_HARD_CAP=10` (`choreographer/findings.py`), the per-field char caps on `Finding` (`content/models.py`), and `_FINDINGS_CAP=20` (vault section, `vault_writer.py`).
## Gotchas
- `Finding.criterion` has **no Pydantic `max_length`** despite the DB column being `String(500)` — a criterion string over 500 chars passes model validation and only fails (or silently truncates, depending on the driver) at the DB layer, not as a clean `ContentValidationError`. Every other Finding field's cap is enforced at both layers; this one isn't.
- `mark_waived` is a fully-implemented repository method with no caller anywhere in the tree — the `waived` status is unreachable through any verb or route today. A deliberate release-scoped gap (per the design spec), not an oversight, but worth remembering before assuming a finding can ever leave `open`/`addressed`/`verified` in production.
- The `issues=[...]` shim and `findings=[...]` are **merged**, not mutually exclusive — a caller sending both gets every concern from both lists (subject to the same combined count guard), not one silently dropping the other (an explicit fix over the naive "prefer findings" approach).
- `round` is computed as `revision_count + 1` and read BEFORE the transition, because `_emit_status_transition_audit` only bumps `revision_count` on ENTRY into `needs_revision` — computing it after the transition would read the wrong (already-bumped) value on a second bounce. `ceo_reject`'s branchless-coordination path computes this manually since it never crosses that chokepoint.
- `ceo_reject` is not a choreographer verb — it has no `roboco-flow` MCP wrapper; it's called directly by the CEO-approval-chain route against `TaskService`. Searching `choreographer/_impl.py` for it will find nothing.
- `stamp_addressed_verified` is NOT best-effort at three of its four call sites (`pass_review`, `pr_pass`, `complete`) — a repository error there fails the whole verb before the transition runs. Only the `ceo_approve` call site wraps it in try/except (logged, swallowed) since CEO approval shouldn't hinge on a cosmetic ledger-stamp failure.
- `_open_finding_ids` (the choreographer helper feeding `GateContext.open_finding_ids`) fails open — a DB lookup error returns `()` (nothing open), never blocking the resubmit on an infrastructure hiccup, but also indistinguishable from "no findings were ever filed."
## Drift from CLAUDE.md
None as of this doc's authoring — CLAUDE.md was updated in the same pass to add the "Revision findings ledger" section and correct the `request_changes` role-transition-table row that previously said "concrete issues."
## Related
- `docs/map/task-service.md``ceo_reject`, `_audit_events_for`
- `docs/map/pr-gate-review.md``pr_fail` findings wiring, gate evidence, verify-stamp
- `docs/map/metrics-observability.md` — rework-by-agent event widening, per-task findings counts, `escaped_defects` Company Scorecard metric
- `docs/map/vault.md` — task note `## Findings` section
- `docs/map/panel.md` — Findings tab, `bounced xN` chip, findings route
- `docs/internal/specs/2026-07-11-revision-findings-ledger.md` — the design spec this slice implements
## Health
The slice is a clean, well-centralized refactor: one shared helper module (`choreographer/findings.py`) is the single chokepoint for validation/capping/rendering/insertion across all three choreographer-level producers, and `ceo_reject` (the one producer outside the choreographer) mirrors the same primitives by hand rather than drifting to its own shape. The append-only ledger design (contrast the overwrite-in-place `notes_structured` it supplements) genuinely fixes the "rounds erase each other" defect the design spec named, and the write-then-gate resolution flow (`resolved_findings` applied before `FINDINGS_ADDRESSED` re-reads) is a sound pattern already proven by the AC-coverage gate it mirrors. The two real gaps — `criterion`'s missing char cap and `mark_waived`'s unwired status — are both small, named, and low-severity (the first is defense-in-depth the DB column still bounds in practice; the second is a deliberate release-scoped deferral, not a broken feature). Best-effort posture is applied precisely where it should be (vault fetch, `ceo_approve` stamp) and withheld where a silent failure would be worse (the three same-transaction verify-stamps).