2 Commits

Include renames

Author	SHA1	Message	Date
3 people	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
3 people	c09cf80b40	Feature/observability gateway health (#247) ... * feat(observability): revision_count + audit_log query index (migration 045) Adds tasks.revision_count (the O(1) rework counter — forward-only, existing rows default 0) and the composite index audit_log(target_id, event_type, timestamp) that powers the cycle-time and rework reconstruction queries. Verified the real upgrade/downgrade/upgrade chain on a throwaway pgvector PG. First task of the 0.10.0 observability dashboards. * feat(observability): count reworks + attribute qa_fail/pr_fail to the rejector Every transition into needs_revision increments tasks.revision_count at the single audit chokepoint (exactly once per bounce, across all paths incl. pr_fail and ceo_reject), so the rework rate is an O(1) read. A QA or PR-review bounce also emits a named task.qa_fail / task.pr_fail audit event carrying the rejector's agent_id, so the per-agent rework scorecard charges the rejection to the reviewer who made it, not the developer who owns the task. * feat(observability): cycle-time, bottleneck, rework, and scorecard metrics MetricsService gains four read methods on the audit_log + tasks data: per-stage cycle time reconstructed from the transition journey (excluding the named qa_fail/pr_fail events), bottleneck distribution (cumulative dwell + live parked counts), rework rate (overall/by-team/by-agent with rejector attribution + cost via spawn-session task_id), and a fused per-agent/per-cell scorecard. Dataclass models with to_dict(). Verified against a real Postgres journey. * feat(observability): cycle-time/bottleneck/rework/scorecard read endpoints Thin read-only routes on the dashboard router delegating to MetricsService: /metrics/cycle-time, /metrics/bottlenecks, /metrics/rework, and /metrics/scorecard/{agent,team}. 404 when an agent scorecard target is absent. 5 route tests (200 + shape + the agent-404 case). * feat(panel): Delivery observability tab (cycle-time, bottlenecks, rework, scorecards) A third Metrics tab built on the observability endpoints: a per-stage cycle-time bar chart, a bottleneck panel (worst stage + cumulative dwell + live parked counts), a rework panel (rate + by-team + by-agent attribution + cost), and per-cell scorecards. Reuses Recharts + Card/Badge/Skeleton and the React-Query hook pattern; observabilityApi mirrors usageApi with mock-mode fallbacks. tsc + eslint clean; 113 panel tests pass. * docs(observability): changelog + CLAUDE.md for the delivery dashboards * feat(gateway-health): recover a broken-but-alive agent instead of protecting it The verb-heartbeat cannot tell a quiet-healthy agent from one whose MCP gateway is broken (a corrupted /app/.venv firing no verb) yet whose container is up — the reaper's live-skip would shield it forever. The reaper now probes the gateway out-of-band (docker exec: does the gateway venv import its deps?) and, once it has been broken past gateway_health_grace_seconds (tolerating a transient probe miss), kills + evicts the container so it falls through to release + respawn. Probe-inconclusive or healthy spares the container. Gated by gateway_health_enabled (default-on reliability fix; in the panel Feature Flags). Defers the optional agent-side self-check + full registry re-adoption — the reaper's docker-liveness fallback already recovers a broken-after-restart agent. * docs(gateway-health): changelog + CLAUDE.md for broken-but-alive recovery * docs(observability): user-facing docs for the Delivery dashboards + gateway-health Documents the new Metrics -> Delivery tab (cycle-time, bottlenecks, rework with rejector attribution, cell scorecards) in the panel guide and the operations health-and-metrics guide, and adds the gateway-health env vars + an agent-gateway recovery note. Published MkDocs site only; settings.md's default-off flag table intentionally omits the default-on gateway-health flag (same as overload-break). * chore(release): cut 0.10.0 (changelog section + version refs) * fix(gateway): exempt PM coordinators from single-task claim guards A Main/Cell PM plans and delegates many root tasks in parallel; the work then runs in the delegated cells, not in the PM's own hands. But the claim-time concurrency guards meant for developers — already_active and paused (the latter firing after i_am_idle auto-pauses the PM's own umbrella) — were applied to the PM too, so once it held one root it could never plan a second: it thrashed between its claimed roots and respawned forever, burning tokens for zero progress. _run_claim_guards now skips already_active/paused for the coordinator PM roles (_COORDINATOR_ROLES = {main_pm, cell_pm}); only unmet_dependency — a real upstream sequence constraint, which parks the root back to pending — still gates a PM. paused_tasks_guard also excludes the target task itself, so a PM re-entering its own paused umbrella never self-blocks. Tests: a coordinator plans a second root with one in_progress + one paused sibling (full path + claimed-recovery path), the paused target exclusion, and the developer guards still fire. Repurposed the pre-fix test that asserted the now-removed PM block. * fix(metrics): coerce SQL avg/extract hours aggregates to float (panel toFixed crash) EXTRACT(epoch ...) returns numeric on PostgreSQL 14+, which asyncpg surfaces as a Decimal; a Decimal serializes to a quoted JSON string, so the panel's avg_cycle_hours.toFixed(1) (and the other hours fields) threw 'toFixed is not a function' and blanked the Delivery tab. A single _as_hours helper now rounds every SQL-averaged hours field to a real float — avg_cycle_hours on the new scorecards plus the pre-existing avg_completion_hours / avg_blocked_hours / longest_blocked_hours. Token and cost fields were already float()-cast and are unaffected. Regression test asserts _as_hours coerces Decimal -> float and preserves the None/zero behavior. * feat(panel): edit a task's sequence from the details page A task's sequence (order within siblings, lower runs first) was display-only with no way to change it from the UI, and TaskUpdate didn't carry the field so PATCH couldn't set it either. The details page's Dependencies tab now has an inline sequence editor mirroring the parent / dependency editors, and PATCH /tasks/{id} accepts a sequence field (owner or privileged role) through the existing generic update path. * fix(mypy): green the full make-quality type gate make quality runs 'mypy roboco/ tests/', which the per-module checks on the 0.10.0 branch never exercised. Two issues surfaced: - The coordinator-exemption change added role_str to Choreographer._run_claim_guards but not to the ChoreographerHelpers protocol base, so the composed Choreographer had incompatible base-class signatures. Sync the protocol signature. - The gateway-health / stale-reaper tests stubbed methods by direct assignment (orch._m = AsyncMock()) and typed their duck-typed task doubles as object, tripping method-assign / assignment / attr-defined. Switch to monkeypatch.setattr (keeping a local mock ref for the assertions) and type the doubles as Any — no type: ignore. Full mypy roboco/ tests/ clean (785 files); the 21 runtime tests pass. * fix(metrics): static cycle-time SQL — clear bandit B608 (CI gate) The cycle-time query interpolated an optional team clause into the text() SQL via an f-string, which bandit flags as B608 (hardcoded SQL) and turned the merge gate red. The team value was always a bound parameter, so it was a false positive — but the f-string is the trigger. Rebuilt as one static query with (CAST(:team AS text) IS NULL OR a.details->>'team' = :team) and an always-bound team param (CAST, not ::text — SQLAlchemy's :param parser collides with PostgreSQL's :: cast operator, which broke the query as a stray param). Full make quality green vs a real pgvector PG (all 21 gate steps). --------- Co-authored-by: Renn F <rennf93@users.noreply.github.com>	2026-06-23 07:26:41 +02:00