From c09cf80b4071d14206979e1c2dcfab80ad321f4d Mon Sep 17 00:00:00 2001 From: Renzo F <45401804+rennf93@users.noreply.github.com> Date: Tue, 23 Jun 2026 07:26:41 +0200 Subject: [PATCH] Feature/observability gateway health (#247) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 --- CHANGELOG.md | 10 + CLAUDE.md | 3 + README.md | 2 +- alembic/versions/045_observability_rework.py | 48 ++ docs/deploy/deployment.md | 2 +- docs/deploy/env-reference.md | 2 + docs/operations/health-and-metrics.md | 16 + docs/panel/metrics.md | 14 +- docs/rag/workflows/task-claiming.md | 4 +- panel/package.json | 2 +- panel/src/app/(dashboard)/metrics/page.tsx | 10 +- panel/src/components/metrics/delivery-tab.tsx | 263 +++++++++++ .../tasks/task-detail/tab-dependencies.tsx | 120 ++++- panel/src/hooks/use-observability.ts | 66 +++ panel/src/lib/api/observability.ts | 84 ++++ panel/src/types/index.ts | 57 +++ pyproject.toml | 2 +- roboco/__init__.py | 2 +- roboco/api/routes/dashboard.py | 72 +++ roboco/api/schemas/tasks.py | 1 + roboco/config.py | 20 +- roboco/db/tables.py | 8 + roboco/models/metrics.py | 136 ++++++ roboco/runtime/orchestrator.py | 105 ++++- .../services/gateway/choreographer/_impl.py | 32 +- .../gateway/choreographer/_protocol.py | 1 + roboco/services/gateway/claim_guards.py | 18 +- roboco/services/metrics.py | 411 +++++++++++++++++- roboco/services/settings.py | 1 + roboco/services/task.py | 66 ++- tests/integration/test_dashboard_routes.py | 49 +++ .../integration/test_metrics_observability.py | 283 ++++++++++++ .../test_migration_observability.py | 43 ++ .../test_task_service_transitions.py | 26 ++ tests/unit/api/test_schemas_tasks.py | 17 + .../test_choreographer_claim_guards.py | 135 ++++++ .../test_choreographer_impl_branches.py | 27 +- tests/unit/runtime/test_gateway_health.py | 195 +++++++++ tests/unit/runtime/test_stale_claim_reaper.py | 50 ++- .../services/test_metrics_hours_coercion.py | 33 ++ tests/unit/services/test_task_audit_events.py | 44 ++ uv.lock | 2 +- 42 files changed, 2419 insertions(+), 63 deletions(-) create mode 100644 alembic/versions/045_observability_rework.py create mode 100644 panel/src/components/metrics/delivery-tab.tsx create mode 100644 panel/src/hooks/use-observability.ts create mode 100644 panel/src/lib/api/observability.ts create mode 100644 tests/integration/test_metrics_observability.py create mode 100644 tests/integration/test_migration_observability.py create mode 100644 tests/unit/runtime/test_gateway_health.py create mode 100644 tests/unit/services/test_metrics_hours_coercion.py create mode 100644 tests/unit/services/test_task_audit_events.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 150b3c73..f34ca7ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,8 +6,18 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), ## [Unreleased] +## [0.10.0] - 2026-06-23 + +### Added + +- **Delivery observability dashboards — cycle-time, bottlenecks, rework rate, and per-agent/per-cell scorecards.** A new "Delivery" tab on the Metrics page surfaces how work *flows*, built on data RoboCo already captures: per-stage cycle time reconstructed from the `audit_log` transition journey, a bottleneck view (which lifecycle stage holds the most cumulative time + how many tasks are parked there now), a rework view (how often work bounces to `needs_revision`, by team and by agent, with the rejection attributed to the QA / PR-reviewer who made it, plus the rework's token cost), and fused per-agent / per-cell scorecards. Backed by new read-only `MetricsService` methods and `/dashboard/metrics/{cycle-time,bottlenecks,rework,scorecard}` endpoints. To make rework correct and O(1), each task now carries a `revision_count` incremented at the single transition chokepoint (migration 045, with a composite `audit_log(target_id, event_type, timestamp)` index for the reconstruction queries), and QA/PR-review bounces emit rejector-attributed `task.qa_fail` / `task.pr_fail` audit events. No feature flag — it reads the always-on metrics surface. +- **Gateway-health recovery — a broken-but-alive agent is recovered instead of protected forever.** The verb-driven heartbeat cannot distinguish a healthy agent quiet during a long edit/test cycle from one whose MCP gateway is broken (e.g. a corrupted `/app/.venv` so every gateway tool import raises) while its container stays up — and the reaper's live-skip would shield that broken agent indefinitely. The reaper now probes the gateway out-of-band (`docker exec`: does the gateway venv import its deps?) and, once it has been broken longer than `ROBOCO_GATEWAY_HEALTH_GRACE_SECONDS` (so a transient probe miss is tolerated), kills + evicts the container so it falls through to release + respawn; a healthy or inconclusive probe spares it. Gated by `ROBOCO_GATEWAY_HEALTH_ENABLED` (default-on reliability fix, in the panel Feature Flags). Builds on the shipped bash-guard `/app` block and reaper Docker-liveness fallback — together the third leg the live incident exposed. +- **Edit a task's sequence from the task details page.** A task's `sequence` (its order within siblings — lower runs first) was display-only with no way to change it from the UI. The details page's Dependencies tab now carries an inline sequence editor alongside the parent / dependency editors, and `PATCH /tasks/{id}` accepts a `sequence` field (owner or privileged role), so an operator can re-order sibling work directly. + ### Fixed +- **Metrics "hours" fields serialized as JSON strings, crashing the panel.** `EXTRACT(epoch …)` returns `numeric` on PostgreSQL 14+, which asyncpg surfaces as a `Decimal` — and a `Decimal` serializes to a quoted JSON *string*. Every SQL-averaged hours field — `avg_cycle_hours` on the new Delivery scorecards, plus the pre-existing `avg_completion_hours` / `avg_blocked_hours` / `longest_blocked_hours` — was therefore a string, so the panel's `value.toFixed(…)` threw `toFixed is not a function` and blanked the tab. A single `_as_hours` coercion now rounds each to a real `float`, so every hours field is a JSON number. (Token/cost fields were already `float()`-cast and unaffected.) +- **The Main PM could not advance past its first coordination task — the developer single-task concurrency guards were deadlocking the coordinator.** A PM plans and delegates many root tasks in parallel; the real work then runs in the delegated cells, not in the PM's own hands. But the claim-time guards that correctly keep a *developer* to one task at a time — `already_active` (you have another claimed / in-progress task) and `paused` (you have a paused task, resume it first — which fires after `i_am_idle` auto-pauses the PM's own umbrella) — were applied to the PM as well, so once it held one root it could never plan a second: it thrashed between its claimed roots and respawned every few minutes, burning tokens for zero progress. These two guards are now skipped for the coordinator PM roles (`main_pm` / `cell_pm`): a PM may hold any number of roots in parallel, gated only by a genuine upstream **sequence dependency** (`unmet_dependency`), which still parks the task to `pending` until its dependency reaches a terminal state. As defense-in-depth the `paused` guard now also excludes the target task itself, so a PM re-entering its own paused umbrella can never self-block. - **Task notes were invisible in the panel — the API response dropped them.** The `task_to_response` serializer (used by the task list and detail endpoints the panel reads) set `dev_notes` / `qa_notes` / `quick_context` but **omitted `pr_reviewer_notes`, `doc_notes`, and `notes_structured`**, and `TaskResponse` didn't even declare `notes_structured` — so the PR-reviewer's notes, the documenter's notes, and the structured PR-review verdict were always blank in the UI no matter what the agents wrote to the DB (the structured-content write-path and obligation gates work; the data simply wasn't being serialized). The builder now returns all note sections plus the structured source of truth. (`dev_notes`/`qa_notes` on an in-flight task are still legitimately empty until the developer submits / QA reviews.) ## [0.9.0] - 2026-06-23 diff --git a/CLAUDE.md b/CLAUDE.md index cfb314c9..ba32d299 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -480,7 +480,10 @@ Server-side events reach these sockets through `roboco/api/websocket_bridge.py`, - **Provider rate limits** are tracked in Redis (`RateLimitStateTracker`, `roboco/services/gateway/`). On a provider 429 an agent calls `i_am_blocked(reason="rate_limited")`; the spawn gate then **queues** (never drops) further work for that provider, and a background probe-and-resume loop in the orchestrator clears the limit and revives parked agents when it lifts. - **Provider overloads** reuse the same park-and-probe break. A persistent model-API overload (HTTP 529 / 500 / 503 — the SDK already retries transient ones) parks the provider exactly like a 429 instead of crash-retrying the agent straight back into the overload and burning tokens; the overload is detected orchestrator-side from the dead container's log markers, and the background loop revives the parked work when it recovers. Gated by `ROBOCO_OVERLOAD_BREAK_ENABLED` (default-on). +- **Gateway-health recovery** closes a blind spot in the stale-claim reaper: the heartbeat is bumped only by gateway verbs, so a broken-but-alive agent (a corrupted `/app/.venv` so no gateway tool imports) goes heartbeat-stale yet keeps its container up, and the reaper's live-skip would protect it forever. On a stale-heartbeat live container the reaper now probes the gateway out-of-band (`_probe_gateway_health` → `docker exec` the gateway venv imports) and, once broken past `ROBOCO_GATEWAY_HEALTH_GRACE_SECONDS` (a transient probe miss is tolerated), kills + evicts it (`_maybe_recover_broken_gateway`) so it falls through to release + respawn; healthy or inconclusive probes spare it. Gated by `ROBOCO_GATEWAY_HEALTH_ENABLED` (default-on). It is the third leg beside the shipped bash-guard `/app` block (prevents the self-corruption) and the reaper Docker-liveness fallback (stops over-reaping live containers). +- **PM coordinator concurrency.** A Main / Cell PM plans and delegates many root tasks in parallel — the actual work then runs in the delegated children/cells, not in the PM's own hands. The claim-time concurrency guards that keep a *developer* to one task at a time (`already_active` / `paused`, in `roboco/services/gateway/claim_guards.py`) are therefore **skipped for the coordinator PM roles** (`_COORDINATOR_ROLES = {main_pm, cell_pm}`, consulted in `_run_claim_guards`); only a genuine upstream **sequence dependency** (`unmet_dependency`, which parks the task back to `pending`) holds a PM's root back. Without this a single PM that claimed one root could never plan a second — it thrashed between its claimed roots and respawned forever, burning tokens for zero progress (the live `i_am_idle`-auto-paused-umbrella deadlock). The `paused` guard also excludes the target task itself, so a PM re-entering its own paused umbrella never self-blocks. - **Token usage** is captured per agent session from the Claude Code transcript via the SDK server's `/usage/sync` (hook → orchestrator finalize → `agent_spawn_sessions` → `daily_usage_rollups` → dashboard). Cost uses provider-aware pricing in `roboco/billing/pricing.py` (Anthropic priced; local/Ollama intentionally `$0`). The token sweep also publishes `USAGE_SNAPSHOT` to `/ws/system`, so the dashboard's "Token Usage & Cost" panel updates live and falls back to HTTP polling when the stream is down. +- **Delivery observability** (the panel's Metrics → "Delivery" tab) shows how work *flows*, computed by `MetricsService` from data already captured — no new feature flag. Per-stage cycle time and the bottleneck distribution are reconstructed from the `audit_log` transition journey (each generic `task.` event marks entry into a status; the named `task.qa_fail`/`task.pr_fail` events are excluded from the reconstruction). Rework rate reads `tasks.revision_count` — incremented once per transition into `needs_revision` at the single chokepoint `TaskService._emit_status_transition_audit` — and attributes each bounce to the QA / PR-reviewer via those named audit events; rework cost joins `agent_spawn_sessions.task_id`. Read-only endpoints: `/dashboard/metrics/{cycle-time,bottlenecks,rework,scorecard/agent/{id},scorecard/team/{team}}`. ### Startup Sequence diff --git a/README.md b/README.md index ed7aeac4..3f904614 100644 --- a/README.md +++ b/README.md @@ -129,7 +129,7 @@ Choose the registry and version with two env vars (defaults shown): ```bash ROBOCO_REGISTRY=ghcr.io/rennf93 # or docker.io/renzof93 -ROBOCO_VERSION=latest # or a pinned release, e.g. 0.9.0 +ROBOCO_VERSION=latest # or a pinned release, e.g. 0.10.0 ``` The orchestrator spawns the matching pre-built agent images on demand — no build toolchain or source compile on your host. diff --git a/alembic/versions/045_observability_rework.py b/alembic/versions/045_observability_rework.py new file mode 100644 index 00000000..cd820241 --- /dev/null +++ b/alembic/versions/045_observability_rework.py @@ -0,0 +1,48 @@ +"""Observability rework tracking: tasks.revision_count + audit_log query index. + +``tasks.revision_count`` makes the per-task rework rate an O(1) column read +instead of an audit_log scan; the composite index on +``audit_log(target_id, event_type, timestamp)`` keeps the cycle-time and rework +reconstruction queries fast. Pure schema change, no backfill — existing rows +default to 0 (the counter is forward-only, matching the design's forward-only +rework attribution). + +Revision ID: 045_observability_rework +Revises: 044_convention_findings +Create Date: 2026-06-23 + +NOTE: revision id is 24 chars — alembic's ``alembic_version.version_num`` is +``VARCHAR(32)`` and a longer id raises at record time. +""" + +from __future__ import annotations + +import sqlalchemy as sa +from alembic import op + +revision = "045_observability_rework" +down_revision = "044_convention_findings" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.add_column( + "tasks", + sa.Column( + "revision_count", + sa.Integer(), + nullable=False, + server_default="0", + ), + ) + op.create_index( + "ix_audit_log_target_event_ts", + "audit_log", + ["target_id", "event_type", "timestamp"], + ) + + +def downgrade() -> None: + op.drop_index("ix_audit_log_target_event_ts", table_name="audit_log") + op.drop_column("tasks", "revision_count") diff --git a/docs/deploy/deployment.md b/docs/deploy/deployment.md index 2467d3de..a8c93328 100644 --- a/docs/deploy/deployment.md +++ b/docs/deploy/deployment.md @@ -30,7 +30,7 @@ Two variables choose what you pull (defaults shown): ```bash ROBOCO_REGISTRY=ghcr.io/rennf93 # or docker.io/renzof93 -ROBOCO_VERSION=latest # or a pinned release, e.g. 0.9.0 +ROBOCO_VERSION=latest # or a pinned release, e.g. 0.10.0 ``` The orchestrator then spawns the **matching** pre-built agent images on demand (it reads `ROBOCO_AGENT_IMAGE_REGISTRY` / `ROBOCO_AGENT_IMAGE_TAG`, which the registry compose wires to the same registry and version). Pin `ROBOCO_VERSION` to a release tag in production so an upstream `latest` push can't silently change your fleet. diff --git a/docs/deploy/env-reference.md b/docs/deploy/env-reference.md index ba5818f2..f1b9f740 100644 --- a/docs/deploy/env-reference.md +++ b/docs/deploy/env-reference.md @@ -210,6 +210,8 @@ These gate the env-toggled capabilities. Each is inert when off. See [Optional c | Variable | Default | Purpose | |----------|---------|---------| | `ROBOCO_OVERLOAD_BREAK_ENABLED` | `true` | Park a provider on a persistent overload (HTTP 529/500/503) the way a 429 is parked, instead of crash-retrying. | +| `ROBOCO_GATEWAY_HEALTH_ENABLED` | `true` | Probe a stale-heartbeat-but-live agent's gateway and kill + respawn it when the gateway is broken (a corrupted `/app` venv firing no verb), instead of the reaper protecting it forever. Off => spare live containers on verb-heartbeat liveness alone. | +| `ROBOCO_GATEWAY_HEALTH_GRACE_SECONDS` | `180` | How long an agent gateway may probe as broken before recovery — tolerates a transient probe miss. | ### Strategy engine — default **off** diff --git a/docs/operations/health-and-metrics.md b/docs/operations/health-and-metrics.md index 9ebe0447..a88a7091 100644 --- a/docs/operations/health-and-metrics.md +++ b/docs/operations/health-and-metrics.md @@ -22,6 +22,9 @@ curl -s http://localhost:3000/api/ready !!! note "Startup ordering" The app-level root `/health` used during container startup is additionally gated on the in-house RAG engine being operational, which is why the orchestrator can take a minute to report healthy after a cold start while it indexes documents. See [deployment](../deploy/deployment.md) for the full startup sequence. +!!! note "Agent gateway recovery" + Those probes cover the *infrastructure*; individual agents have their own liveness story. An agent whose MCP gateway breaks (a corrupted `/app` venv firing no verb) stays "up" as a container but does no work, and its verb-heartbeat goes stale. With `ROBOCO_GATEWAY_HEALTH_ENABLED` (default on) the reaper probes such a stale-but-live agent's gateway out-of-band and, once it's been broken past `ROBOCO_GATEWAY_HEALTH_GRACE_SECONDS`, kills and respawns it instead of protecting it forever — so a wedged agent self-heals without operator action. You don't monitor this directly; it shows up as a "broken-gateway agent killed and evicted" line in the orchestrator log. + ## The Metrics → Performance view The **Metrics** page has a **Performance** tab driven by tasks, messages, and notifications — your read on whether work is actually flowing. See the panel walkthrough in [Metrics](../panel/metrics.md). @@ -51,6 +54,19 @@ A team with more than 30% of its tasks blocked reads **critical**; over 15% read !!! tip "Health is about flow, not errors" This status is computed from task state, not exceptions or crashes. A green org-health with a `degraded` `/api/ready` means the infrastructure is wobbling even though the backlog looks healthy — watch both signals, they answer different questions. +## The Metrics → Delivery view + +Where Performance answers *is work flowing*, the **Delivery** tab answers *where the time goes and how clean the work is* — reconstructed from the recorded task-transition history, so it costs no extra bookkeeping. Use it when velocity dips and you need the *why*: + +| Panel | Operational read | +|-------|------------------| +| Cycle time by stage | Average dwell per lifecycle stage — a tall stage is where work waits (e.g. review vs coding) | +| Bottlenecks | The single worst stage by total time absorbed + how many tasks are parked in each stage right now | +| Rework | How often work bounces to `needs_revision`, by cell and by agent (the bounce charged to the QA/PR-reviewer who sent it back), plus its token cost | +| Cell scorecards | Per-cell completed / avg cycle / rework / cost over 7 days | + +The full reading guide — especially how rework attribution works — is in the panel walkthrough at [Metrics → Delivery](../panel/metrics.md#delivery). + ## Next - Walk the panel surface in [Metrics](../panel/metrics.md) and [the command center](../panel/command-center.md). diff --git a/docs/panel/metrics.md b/docs/panel/metrics.md index a8a2e9f7..03355e95 100644 --- a/docs/panel/metrics.md +++ b/docs/panel/metrics.md @@ -1,6 +1,6 @@ # Metrics -The Metrics page (`/metrics`) is where you watch the company's throughput and its spend. Two tabs: **Performance** and **Token Usage** (the active tab is in the URL as `?tab=`). +The Metrics page (`/metrics`) is where you watch the company's throughput and its spend. Three tabs: **Performance**, **Token Usage**, and **Delivery** (the active tab is in the URL as `?tab=`). ## Performance @@ -32,6 +32,18 @@ Cost is derived from per-session token counts using provider-aware pricing — a !!! note "Spend against budget lives on the scorecard" Metrics shows raw usage and projection. Your **monthly budget cap** and whether you're over it appear on the Company Scorecard in [Business](./business.md), not here. +## Delivery + +The Delivery tab is the flow dashboard — not *what* the company shipped or what it cost, but *how the work moved*. Every panel is reconstructed from the task lifecycle history RoboCo already records (each status transition is logged), so it needs no extra bookkeeping. Cycle-time, bottlenecks, and rework look back 30 days; the scorecards look back 7. + +- **Cycle Time by Stage** — the average time a task sits in each lifecycle stage (claimed, in progress, awaiting QA, awaiting documentation, awaiting PR review, awaiting PM review, …). This is where you see *where the time actually goes* — a tall "awaiting QA" bar means work waits on review, not on coding. +- **Bottlenecks** — the same data ranked by total time absorbed, with the single **worst stage** called out and a live count of how many tasks are **parked** in each stage right now, plus the current active-blocker count. It answers "what is holding the company up today?" +- **Rework** — how often work bounces back to `needs_revision` (the headline rate = reworked ÷ completed), broken down by cell and by agent, plus the token cost of that rework. Crucially, a bounce is attributed to the **QA or PR-reviewer who sent it back**, not the developer who owns the task — so a high `QA fails` number against a reviewer is a signal about *that reviewer's* gate, and a high rate against a developer is a signal about *their* first-pass quality. +- **Cell scorecards** — one card per cell (Backend / Frontend / UX-UI) with its completed count, average cycle time, rework rate, and cost over the last 7 days — the quick read on which cell is moving cleanly. + +!!! tip "Reading rework attribution" + A bounce charges the reviewer who rejected it via the `task.qa_fail` / `task.pr_fail` events, while the *rate* (`reworked / completed`) is computed against the task's owner. So one agent can show a low rate (good first-pass work) while another shows many `QA fails` (an active, rejecting gate) — both are healthy. Watch for a developer with a high rate **and** a reviewer with near-zero fails: that's a gate letting work through that later needs revision. + ## Next → [Cost & usage](../operations/cost-and-usage.md) for the pricing model and budget cap · [Health & metrics](../operations/health-and-metrics.md) for operational monitoring · [Command Center](./command-center.md) for the at-a-glance view. diff --git a/docs/rag/workflows/task-claiming.md b/docs/rag/workflows/task-claiming.md index b9a5852c..a3645e36 100644 --- a/docs/rag/workflows/task-claiming.md +++ b/docs/rag/workflows/task-claiming.md @@ -29,7 +29,7 @@ The claim verb both claims and starts the task — there is no separate `start` ## Before Claiming -1. Check you have capacity (one task at a time recommended) +1. Check you have capacity (developers / QA / documenters work one task at a time; **PM coordinators are exempt** — a Main / Cell PM may hold many roots at once, gated only by sequence dependencies) 2. Verify dependencies are completed 3. Read task description and acceptance criteria @@ -41,7 +41,7 @@ The claim verb both claims and starts the task — there is no separate `start` ## Claiming Rules -- **One at a time**: Don't claim multiple in-progress tasks +- **One at a time (workers only)**: Developers, QA, and documenters can't hold multiple in-progress tasks at once. **PM coordinators are exempt** — a Main / Cell PM plans and delegates many roots in parallel, so it may hold several at once; only a real upstream **sequence dependency** (an unfinished task it depends on) holds one of its roots back. - **Self-review prevention**: QA cannot `claim_review` tasks they developed - **Self-documentation prevention**: Documenter cannot claim tasks they developed - **Branch requirement**: Branch auto-created on `i_will_work_on` diff --git a/panel/package.json b/panel/package.json index f0a8f2c3..88c4eec7 100644 --- a/panel/package.json +++ b/panel/package.json @@ -1,6 +1,6 @@ { "name": "roboco-panel", - "version": "0.9.0", + "version": "0.10.0", "private": true, "packageManager": "pnpm@10.25.0", "scripts": { diff --git a/panel/src/app/(dashboard)/metrics/page.tsx b/panel/src/app/(dashboard)/metrics/page.tsx index 7f7a7339..c797bfff 100644 --- a/panel/src/app/(dashboard)/metrics/page.tsx +++ b/panel/src/app/(dashboard)/metrics/page.tsx @@ -20,6 +20,7 @@ import { Progress } from "@/components/ui/progress"; import { Skeleton } from "@/components/ui/skeleton"; import { Tabs, TabsContent, TabsList, TabsTrigger } from "@/components/ui/tabs"; import { OfflineState } from "@/components/ui/offline-state"; +import { DeliveryTabContent } from "@/components/metrics/delivery-tab"; import { UsageTimeSeriesChart, ModelUsageDonut, @@ -527,9 +528,9 @@ function CacheEfficiencyCard({ cacheStats, isLoading }: CacheEfficiencyCardProps // ─── Tab types ──────────────────────────────────────────────────────────────── -type MetricsTab = "performance" | "token-usage"; +type MetricsTab = "performance" | "token-usage" | "delivery"; -const VALID_METRICS_TABS: MetricsTab[] = ["performance", "token-usage"]; +const VALID_METRICS_TABS: MetricsTab[] = ["performance", "token-usage", "delivery"]; function isValidMetricsTab(value: string | null): value is MetricsTab { return VALID_METRICS_TABS.includes(value as MetricsTab); @@ -567,6 +568,7 @@ function MetricsPageContent() { Performance Token Usage + Delivery @@ -576,6 +578,10 @@ function MetricsPageContent() { + + + + ); diff --git a/panel/src/components/metrics/delivery-tab.tsx b/panel/src/components/metrics/delivery-tab.tsx new file mode 100644 index 00000000..195f82ec --- /dev/null +++ b/panel/src/components/metrics/delivery-tab.tsx @@ -0,0 +1,263 @@ +"use client"; + +import { + BarChart, + Bar, + XAxis, + YAxis, + CartesianGrid, + Tooltip, + ResponsiveContainer, +} from "recharts"; +import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card"; +import { Skeleton } from "@/components/ui/skeleton"; +import { Badge } from "@/components/ui/badge"; +import { + useCycleTime, + useBottlenecks, + useRework, + useTeamScorecard, +} from "@/hooks/use-observability"; +import type { Scorecard } from "@/types"; + +const CELLS = ["backend", "frontend", "ux_ui"] as const; + +function label(status: string): string { + return status + .split("_") + .map((w) => w.charAt(0).toUpperCase() + w.slice(1)) + .join(" "); +} + +function fmtDuration(seconds: number): string { + if (seconds >= 3600) return (seconds / 3600).toFixed(1) + "h"; + if (seconds >= 60) return (seconds / 60).toFixed(0) + "m"; + return seconds.toFixed(0) + "s"; +} + +function pct(rate: number): string { + return (rate * 100).toFixed(1) + "%"; +} + +// ─── Cycle time ─────────────────────────────────────────────────────────────── + +function CycleTimeCard() { + const { data, isLoading } = useCycleTime(30); + const chartData = (data ?? []).map((s) => ({ + name: label(s.status), + Hours: Number((s.avg_seconds / 3600).toFixed(2)), + })); + return ( + + + Cycle Time by Stage (avg, 30d) + + + {isLoading ? ( + + ) : chartData.length === 0 ? ( +

+ No completed transitions in the window yet. +

+ ) : ( + + + + + v + "h"} + /> + [value + "h", "Avg"]} + contentStyle={{ fontSize: 12 }} + /> + + + + )} +
+
+ ); +} + +// ─── Bottlenecks ────────────────────────────────────────────────────────────── + +function BottlenecksCard() { + const { data, isLoading } = useBottlenecks(30); + return ( + + + Bottlenecks + + + {isLoading ? ( + + ) : ( + <> +
+ Worst stage: + {data?.worst_stage ? ( + {label(data.worst_stage)} + ) : ( + + )} + + {data?.active_blockers ?? 0} active blockers + +
+
+ {(data?.by_stage ?? []).slice(0, 6).map((s) => ( +
+
+ {label(s.status)} + + {fmtDuration(s.cumulative_seconds)} · {s.parked_now} parked + +
+
+
+
+
+ ))} + {(data?.by_stage ?? []).length === 0 && ( +

+ No stage data yet. +

+ )} +
+ + )} + + + ); +} + +// ─── Rework ─────────────────────────────────────────────────────────────────── + +function ReworkCard() { + const { data, isLoading } = useRework(30); + return ( + + + Rework (30d) + + + {isLoading ? ( + + ) : ( + <> +
+ {pct(data?.rate ?? 0)} + + {data?.total_reworked ?? 0}/{data?.total_completed ?? 0} bounced · + ${(data?.rework_cost_usd ?? 0).toFixed(2)} cost + +
+
+ {(data?.by_team ?? []).map((t) => ( + + {label(t.team)} {pct(t.rate)} + + ))} +
+ {(data?.by_agent ?? []).length > 0 && ( + + + + + + + + + + + {(data?.by_agent ?? []).slice(0, 8).map((a) => ( + + + + + + + ))} + +
AgentRateQA failsPR fails
{a.agent_slug}{pct(a.rate)}{a.qa_fails}{a.pr_fails}
+ )} + + )} +
+
+ ); +} + +// ─── Per-cell scorecards ──────────────────────────────────────────────────────── + +function CellScorecard({ team }: { team: string }) { + const { data, isLoading } = useTeamScorecard(team, 7); + return ( + + + {label(team)} (7d) + + + {isLoading ? ( + + ) : ( + + )} + + + ); +} + +function ScorecardBody({ card }: { card: Scorecard | undefined }) { + const stat = (k: string, v: string) => ( +
+ {k} + {v} +
+ ); + return ( +
+ {stat("Completed", String(card?.tasks_completed ?? 0))} + {stat( + "Avg cycle", + card?.avg_cycle_hours != null ? card.avg_cycle_hours.toFixed(1) + "h" : "—", + )} + {stat("Rework", pct(card?.rework_rate ?? 0))} + {stat("Cost", "$" + (card?.cost_usd ?? 0).toFixed(2))} +
+ ); +} + +// ─── Tab ────────────────────────────────────────────────────────────────────── + +export function DeliveryTabContent() { + return ( +
+
+ + +
+ +
+ {CELLS.map((team) => ( + + ))} +
+
+ ); +} diff --git a/panel/src/components/tasks/task-detail/tab-dependencies.tsx b/panel/src/components/tasks/task-detail/tab-dependencies.tsx index a6ac0433..904346ee 100644 --- a/panel/src/components/tasks/task-detail/tab-dependencies.tsx +++ b/panel/src/components/tasks/task-detail/tab-dependencies.tsx @@ -7,7 +7,7 @@ import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card"; import { Button } from "@/components/ui/button"; import { Input } from "@/components/ui/input"; import { Badge } from "@/components/ui/badge"; -import { ArrowUp, Link2, AlertTriangle, Plus, Trash2, X, Check } from "lucide-react"; +import { ArrowUp, Link2, AlertTriangle, Plus, Trash2, X, Check, Hash, Pencil } from "lucide-react"; import Link from "next/link"; import { toast } from "sonner"; @@ -264,6 +264,54 @@ export function TabDependencies({ task }: TabDependenciesProps) { } }; + // Sequence editing (sibling order — lower runs first; mirrors dependency order) + const [editingSequence, setEditingSequence] = useState(false); + const [localSequenceValue, setLocalSequenceValue] = useState(""); + const sequenceInputRef = useRef(null); + + const startEditingSequence = () => { + setLocalSequenceValue(String(task.sequence ?? 0)); + setEditingSequence(true); + }; + + useEffect(() => { + if (editingSequence && sequenceInputRef.current) { + sequenceInputRef.current.focus(); + sequenceInputRef.current.select(); + } + }, [editingSequence]); + + const handleSequenceSave = async () => { + const parsed = parseInt(localSequenceValue, 10); + if (Number.isNaN(parsed) || parsed < 0) { + toast.error("Sequence must be a non-negative integer"); + setLocalSequenceValue(String(task.sequence ?? 0)); + return; + } + if (parsed === task.sequence) { + setEditingSequence(false); + return; + } + try { + await updateTask.mutateAsync({ + taskId: task.id, + updates: { sequence: parsed }, + }); + setEditingSequence(false); + } catch { + toast.error("Failed to update sequence"); + setLocalSequenceValue(String(task.sequence ?? 0)); + } + }; + + const handleSequenceKeyDown = (e: React.KeyboardEvent) => { + if (e.key === "Enter") { + handleSequenceSave(); + } else if (e.key === "Escape") { + setEditingSequence(false); + } + }; + return (
{/* Dependencies (upstream - blocks this task) */} @@ -369,6 +417,76 @@ export function TabDependencies({ task }: TabDependenciesProps) { )} + + {/* Sequence (sibling order) */} + + +
+ + + Sequence + +
+
+ + {editingSequence ? ( +
+ + setLocalSequenceValue(e.target.value)} + onKeyDown={handleSequenceKeyDown} + placeholder="Order within siblings (lower runs first)" + className="h-8 text-sm flex-1" + disabled={updateTask.isPending} + /> + + +
+ ) : ( +
+ + #{task.sequence ?? 0} + + Order within siblings — lower runs first + + +
+ )} +
+
); } diff --git a/panel/src/hooks/use-observability.ts b/panel/src/hooks/use-observability.ts new file mode 100644 index 00000000..c77d6695 --- /dev/null +++ b/panel/src/hooks/use-observability.ts @@ -0,0 +1,66 @@ +"use client"; + +import { useQuery } from "@tanstack/react-query"; +import { observabilityApi } from "@/lib/api/observability"; +import type { + StageTiming, + BottleneckReport, + ReworkReport, + Scorecard, +} from "@/types"; + +// ============================================================================= +// QUERY KEYS +// ============================================================================= + +export const observabilityKeys = { + all: ["observability"] as const, + cycleTime: (days: number, team?: string) => + [...observabilityKeys.all, "cycle-time", days, team ?? "all"] as const, + bottlenecks: (days: number) => + [...observabilityKeys.all, "bottlenecks", days] as const, + rework: (days: number, team?: string) => + [...observabilityKeys.all, "rework", days, team ?? "all"] as const, + teamScorecard: (team: string, days: number) => + [...observabilityKeys.all, "scorecard", "team", team, days] as const, +}; + +// ============================================================================= +// HOOKS +// ============================================================================= + +/** Per-stage cycle time (dwell per lifecycle status). */ +export function useCycleTime(days = 30, team?: string) { + return useQuery({ + queryKey: observabilityKeys.cycleTime(days, team), + queryFn: () => observabilityApi.getCycleTime(days, team), + refetchInterval: 60_000, + }); +} + +/** Bottleneck distribution: cumulative dwell + live parked counts. */ +export function useBottlenecks(days = 30) { + return useQuery({ + queryKey: observabilityKeys.bottlenecks(days), + queryFn: () => observabilityApi.getBottlenecks(days), + refetchInterval: 60_000, + }); +} + +/** Rework rate overall, by team, and by agent + cost. */ +export function useRework(days = 30, team?: string) { + return useQuery({ + queryKey: observabilityKeys.rework(days, team), + queryFn: () => observabilityApi.getRework(days, team), + refetchInterval: 60_000, + }); +} + +/** Per-cell delivery scorecard. */ +export function useTeamScorecard(team: string, days = 7) { + return useQuery({ + queryKey: observabilityKeys.teamScorecard(team, days), + queryFn: () => observabilityApi.getTeamScorecard(team, days), + refetchInterval: 60_000, + }); +} diff --git a/panel/src/lib/api/observability.ts b/panel/src/lib/api/observability.ts new file mode 100644 index 00000000..90dc60c0 --- /dev/null +++ b/panel/src/lib/api/observability.ts @@ -0,0 +1,84 @@ +import api from "./client"; +import { isMockMode } from "@/lib/mock-data"; +import type { + StageTiming, + BottleneckReport, + ReworkReport, + Scorecard, +} from "@/types"; + +// ============================================================================= +// MOCK FALLBACKS (demo / no-backend mode) +// ============================================================================= + +const EMPTY_BOTTLENECK: BottleneckReport = { + by_stage: [], + worst_stage: null, + active_blockers: 0, +}; + +const EMPTY_REWORK: ReworkReport = { + rate: 0, + total_completed: 0, + total_reworked: 0, + by_team: [], + by_agent: [], + rework_cost_usd: 0, +}; + +function emptyScorecard(scope: string, id: string): Scorecard { + return { + scope, + id, + name: id, + tasks_completed: 0, + avg_cycle_hours: null, + rework_rate: 0, + tokens: 0, + cost_usd: 0, + }; +} + +// ============================================================================= +// API OBJECT +// ============================================================================= + +export const observabilityApi = { + /** Per-stage cycle time — GET /dashboard/metrics/cycle-time?days&team */ + getCycleTime: async (days = 30, team?: string): Promise => { + if (isMockMode()) return []; + const { data } = await api.get("/dashboard/metrics/cycle-time", { + params: { days, ...(team ? { team } : {}) }, + }); + return data; + }, + + /** Bottleneck distribution — GET /dashboard/metrics/bottlenecks?days */ + getBottlenecks: async (days = 30): Promise => { + if (isMockMode()) return EMPTY_BOTTLENECK; + const { data } = await api.get( + "/dashboard/metrics/bottlenecks", + { params: { days } }, + ); + return data; + }, + + /** Rework rate + attribution — GET /dashboard/metrics/rework?days&team */ + getRework: async (days = 30, team?: string): Promise => { + if (isMockMode()) return EMPTY_REWORK; + const { data } = await api.get("/dashboard/metrics/rework", { + params: { days, ...(team ? { team } : {}) }, + }); + return data; + }, + + /** Per-cell scorecard — GET /dashboard/metrics/scorecard/team/{team}?days */ + getTeamScorecard: async (team: string, days = 7): Promise => { + if (isMockMode()) return emptyScorecard("cell", team); + const { data } = await api.get( + `/dashboard/metrics/scorecard/team/${team}`, + { params: { days } }, + ); + return data; + }, +}; diff --git a/panel/src/types/index.ts b/panel/src/types/index.ts index 4b985a1c..bd3d171b 100644 --- a/panel/src/types/index.ts +++ b/panel/src/types/index.ts @@ -1361,3 +1361,60 @@ export interface UsageSession { cost: number; model: string; } + +// ============================================================================= +// Observability (0.10.0): cycle-time, bottlenecks, rework, scorecard +// ============================================================================= + +export interface StageTiming { + status: string; + avg_seconds: number; + median_seconds: number; + p90_seconds: number; + sample_size: number; +} + +export interface StageBottleneck { + status: string; + cumulative_seconds: number; + parked_now: number; + pct_of_total: number; +} + +export interface BottleneckReport { + by_stage: StageBottleneck[]; + worst_stage: string | null; + active_blockers: number; +} + +export interface AgentReworkRate { + agent_slug: string; + rate: number; + qa_fails: number; + pr_fails: number; +} + +export interface TeamReworkRate { + team: string; + rate: number; +} + +export interface ReworkReport { + rate: number; + total_completed: number; + total_reworked: number; + by_team: TeamReworkRate[]; + by_agent: AgentReworkRate[]; + rework_cost_usd: number; +} + +export interface Scorecard { + scope: string; + id: string; + name: string; + tasks_completed: number; + avg_cycle_hours: number | null; + rework_rate: number; + tokens: number; + cost_usd: number; +} diff --git a/pyproject.toml b/pyproject.toml index 378f55d5..61dab3ad 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "roboco" -version = "0.9.0" +version = "0.10.0" description = "AI Agents Company - A virtual organization of AI agents functioning as a software development workforce" authors = [ {name = "Renzo Franceschini", email = "rennf93@users.noreply.github.com"} diff --git a/roboco/__init__.py b/roboco/__init__.py index f2f0f543..c3d39a3b 100644 --- a/roboco/__init__.py +++ b/roboco/__init__.py @@ -5,7 +5,7 @@ A virtual organization of 25 AI agents + 1 human CEO, designed to operate as a complete software development workforce. """ -__version__ = "0.9.0" +__version__ = "0.10.0" # Core exports from roboco.config import settings diff --git a/roboco/api/routes/dashboard.py b/roboco/api/routes/dashboard.py index bc642a44..492483f4 100644 --- a/roboco/api/routes/dashboard.py +++ b/roboco/api/routes/dashboard.py @@ -511,3 +511,75 @@ async def get_agent_metrics( status_code=status.HTTP_404_NOT_FOUND, detail="Agent not found" ) return metrics.to_dict() + + +# ============================================================================= +# OBSERVABILITY ENDPOINTS (0.10.0): cycle-time / bottlenecks / rework / scorecard +# ============================================================================= + + +@router.get("/metrics/cycle-time") +async def get_cycle_time( + db: DbSession, + days: int = Query(default=30, ge=1, le=90), + team: Team | None = None, +) -> list[dict[str, Any]]: + """Per-stage cycle time (dwell per lifecycle status) over the window.""" + metrics_service = get_metrics_service(db) + stages = await metrics_service.get_cycle_time_by_stage(team=team, days=days) + return [s.to_dict() for s in stages] + + +@router.get("/metrics/bottlenecks") +async def get_bottlenecks( + db: DbSession, + days: int = Query(default=30, ge=1, le=90), +) -> dict[str, Any]: + """Where work piles up: cumulative dwell per stage + live parked counts.""" + metrics_service = get_metrics_service(db) + report = await metrics_service.get_bottleneck_distribution(days=days) + return report.to_dict() + + +@router.get("/metrics/rework") +async def get_rework( + db: DbSession, + days: int = Query(default=30, ge=1, le=90), + team: Team | None = None, +) -> dict[str, Any]: + """Rework rate (bounced/completed) overall, by team, and by agent + cost.""" + metrics_service = get_metrics_service(db) + report = await metrics_service.get_rework_metrics(team=team, days=days) + return report.to_dict() + + +@router.get("/metrics/scorecard/agent/{agent_id}") +async def get_agent_scorecard( + agent_id: UUID, + db: DbSession, + days: int = Query(default=7, ge=1, le=90), +) -> dict[str, Any]: + """Fused per-agent delivery scorecard.""" + metrics_service = get_metrics_service(db) + card = await metrics_service.get_scorecard(agent_id=agent_id, days=days) + if card is None: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, detail="Agent not found" + ) + return card.to_dict() + + +@router.get("/metrics/scorecard/team/{team}") +async def get_team_scorecard( + team: Team, + db: DbSession, + days: int = Query(default=7, ge=1, le=90), +) -> dict[str, Any]: + """Fused per-cell delivery scorecard.""" + metrics_service = get_metrics_service(db) + card = await metrics_service.get_scorecard(team=team, days=days) + if card is None: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, detail="Team scorecard unavailable" + ) + return card.to_dict() diff --git a/roboco/api/schemas/tasks.py b/roboco/api/schemas/tasks.py index 65ebd35d..d8bf29eb 100644 --- a/roboco/api/schemas/tasks.py +++ b/roboco/api/schemas/tasks.py @@ -204,6 +204,7 @@ class TaskUpdate(BaseModel): description: str | None = Field(default=None, min_length=20) acceptance_criteria: list[str] | None = Field(default=None, min_length=1) priority: int | None = Field(default=None, ge=0, le=3) + sequence: int | None = Field(default=None, ge=0) # Order within siblings target_date: datetime | None = None estimated_complexity: Complexity | None = None diff --git a/roboco/config.py b/roboco/config.py index 8abbe0b2..1513f620 100644 --- a/roboco/config.py +++ b/roboco/config.py @@ -28,7 +28,7 @@ class Settings(BaseSettings): # ========================================================================== # Application # ========================================================================== - app_version: str = "0.9.0" + app_version: str = "0.10.0" debug: bool = False environment: str = Field( default="development", pattern="^(development|staging|production)$" @@ -200,6 +200,22 @@ class Settings(BaseSettings): "of crash-retrying into the overload. Off => crash-retry behavior." ), ) + gateway_health_enabled: bool = Field( + default=True, + description=( + "Detect a broken-but-alive agent gateway (a corrupted /app venv so no " + "gateway verb can fire) and kill + respawn the container, instead of " + "the reaper protecting it forever as a 'live' agent. Off => live " + "containers are spared on verb-heartbeat liveness alone." + ), + ) + gateway_health_grace_seconds: int = Field( + default=180, + description=( + "How long an agent gateway may probe as broken before the reaper " + "recovers it — tolerates a transient probe miss (the gateway mid-call)." + ), + ) # ========================================================================== # Architectural Conventions (per-project placement + house-style standard) @@ -532,7 +548,7 @@ class Settings(BaseSettings): agent_image_tag: str = Field( default="", description=( - "Tag for pre-built agent images (e.g. 'latest' or '0.9.0'). Empty " + "Tag for pre-built agent images (e.g. 'latest' or '0.10.0'). Empty " "leaves the tag implicit (':latest'); only meaningful with " "agent_image_registry set." ), diff --git a/roboco/db/tables.py b/roboco/db/tables.py index caf8e748..29d5485b 100644 --- a/roboco/db/tables.py +++ b/roboco/db/tables.py @@ -330,6 +330,11 @@ class TaskTable(Base): # Review Status self_verified: Mapped[bool] = mapped_column(Boolean, default=False) qa_verified: Mapped[bool | None] = mapped_column(Boolean, nullable=True) + # Rework: incremented on every transition into needs_revision so the + # rework rate is an O(1) column read instead of an audit_log scan. + revision_count: Mapped[int] = mapped_column( + Integer, default=0, server_default="0", nullable=False + ) # Quick Context quick_context: Mapped[str | None] = mapped_column(Text, nullable=True) @@ -1794,6 +1799,9 @@ class AuditLogTable(Base): __table_args__ = ( Index("ix_audit_log_agent_timestamp", "agent_id", "timestamp"), Index("ix_audit_log_target", "target_type", "target_id"), + # Powers the observability cycle-time / rework reconstruction: per-task + # transition journeys are read by (target_id, event_type) ordered by time. + Index("ix_audit_log_target_event_ts", "target_id", "event_type", "timestamp"), ) diff --git a/roboco/models/metrics.py b/roboco/models/metrics.py index 436c20d3..d058f6fc 100644 --- a/roboco/models/metrics.py +++ b/roboco/models/metrics.py @@ -111,3 +111,139 @@ class AgentMetrics: "avg_completion_hours": self.avg_completion_hours, "messages_sent_week": self.messages_sent_week, } + + +# ============================================================================= +# OBSERVABILITY (0.10.0): cycle-time, bottlenecks, rework, scorecard +# ============================================================================= + + +@dataclass +class StageTiming: + """Time tasks spend in one lifecycle status, reconstructed from audit_log.""" + + status: str + avg_seconds: float + median_seconds: float + p90_seconds: float + sample_size: int + + def to_dict(self) -> dict[str, Any]: + return { + "status": self.status, + "avg_seconds": round(self.avg_seconds, 1), + "median_seconds": round(self.median_seconds, 1), + "p90_seconds": round(self.p90_seconds, 1), + "sample_size": self.sample_size, + } + + +@dataclass +class StageBottleneck: + """Cumulative dwell + current parked count for one lifecycle status.""" + + status: str + cumulative_seconds: float + parked_now: int + pct_of_total: float + + def to_dict(self) -> dict[str, Any]: + return { + "status": self.status, + "cumulative_seconds": round(self.cumulative_seconds, 1), + "parked_now": self.parked_now, + "pct_of_total": round(self.pct_of_total, 4), + } + + +@dataclass +class BottleneckReport: + """Where the work piles up: cumulative dwell per stage + live parked counts.""" + + by_stage: list[StageBottleneck] + worst_stage: str | None + active_blockers: int + + def to_dict(self) -> dict[str, Any]: + return { + "by_stage": [s.to_dict() for s in self.by_stage], + "worst_stage": self.worst_stage, + "active_blockers": self.active_blockers, + } + + +@dataclass +class AgentReworkRate: + """Per-agent rework: bounce rate + fails attributed to this agent's reviews.""" + + agent_slug: str + rate: float + qa_fails: int + pr_fails: int + + def to_dict(self) -> dict[str, Any]: + return { + "agent_slug": self.agent_slug, + "rate": round(self.rate, 4), + "qa_fails": self.qa_fails, + "pr_fails": self.pr_fails, + } + + +@dataclass +class TeamReworkRate: + """Per-cell rework rate.""" + + team: str + rate: float + + def to_dict(self) -> dict[str, Any]: + return {"team": self.team, "rate": round(self.rate, 4)} + + +@dataclass +class ReworkReport: + """How often work bounces to needs_revision, by team and by agent.""" + + rate: float + total_completed: int + total_reworked: int + by_team: list[TeamReworkRate] + by_agent: list[AgentReworkRate] + rework_cost_usd: float + + def to_dict(self) -> dict[str, Any]: + return { + "rate": round(self.rate, 4), + "total_completed": self.total_completed, + "total_reworked": self.total_reworked, + "by_team": [t.to_dict() for t in self.by_team], + "by_agent": [a.to_dict() for a in self.by_agent], + "rework_cost_usd": round(self.rework_cost_usd, 4), + } + + +@dataclass +class Scorecard: + """Fused per-agent or per-cell delivery scorecard.""" + + scope: str # "agent" | "cell" + id: str + name: str + tasks_completed: int + avg_cycle_hours: float | None + rework_rate: float + tokens: int + cost_usd: float + + def to_dict(self) -> dict[str, Any]: + return { + "scope": self.scope, + "id": self.id, + "name": self.name, + "tasks_completed": self.tasks_completed, + "avg_cycle_hours": self.avg_cycle_hours, + "rework_rate": round(self.rework_rate, 4), + "tokens": self.tokens, + "cost_usd": round(self.cost_usd, 4), + } diff --git a/roboco/runtime/orchestrator.py b/roboco/runtime/orchestrator.py index 716bbff8..7494a0ae 100644 --- a/roboco/runtime/orchestrator.py +++ b/roboco/runtime/orchestrator.py @@ -668,6 +668,10 @@ class AgentOrchestrator: self.dispatcher_interval = dispatcher_interval self._instances: dict[str, AgentInstance] = {} + # Gateway-health grace tracker: agent slug -> first time its gateway was + # seen broken. Tolerates a transient probe miss before the reaper recovers + # a broken-but-alive agent (see _maybe_recover_broken_gateway). + self._gateway_broken_since: dict[str, datetime] = {} self._waiting_records: dict[str, WaitingRecord] = {} self._health_task: asyncio.Task | None = None self._dispatcher_task: asyncio.Task | None = None @@ -5075,6 +5079,37 @@ Start by: exit_code = None return is_running, exit_code + @staticmethod + async def _probe_gateway_health(slug: str) -> bool | None: + """Probe an agent container's gateway out-of-band: healthy / broken / unknown. + + The heartbeat only proves a verb fired recently; it cannot tell a quiet- + but-healthy agent from one whose MCP gateway is broken (e.g. a corrupted + ``/app/.venv`` so every gateway tool import raises) yet whose container is + still up. This asks the container directly whether the gateway venv imports + its core deps. Returns True (healthy), False (the import failed => broken + gateway), or None when the probe itself could not run (no docker, container + gone) so the caller declines to act on an inconclusive probe. + """ + try: + proc = await asyncio.create_subprocess_exec( + "docker", + "exec", + f"roboco-agent-{slug}", + "/app/.venv/bin/python", + "-c", + "import httpx, mcp", + stdout=asyncio.subprocess.DEVNULL, + stderr=asyncio.subprocess.DEVNULL, + ) + except Exception: + return None + try: + rc = await proc.wait() + except Exception: + return None + return rc == 0 + async def _handle_stopped_container( self, agent_id: str, instance: Any, exit_code: int | None ) -> None: @@ -7112,6 +7147,67 @@ Start now: evidence(task_id="{task_id}") ) return True + async def _maybe_recover_broken_gateway(self, task: Any) -> bool: + """Kill + evict a live agent whose gateway is broken past the grace window. + + The reaper's live-skip protects a running container from a stale-heartbeat + reap — right for a healthy agent quiet during a long edit/test cycle, but + it would shield a broken-but-alive agent (a corrupted gateway firing no + verb) forever. This probes the gateway out-of-band and, once it has been + broken longer than ``gateway_health_grace_seconds`` (so a transient probe + miss is tolerated), kills + evicts the container so the reaper falls + through to release + respawn. Returns True only on a kill; a healthy + gateway, an inconclusive probe, or a still-within-grace breakage returns + False (the live container is spared). Gated by ``gateway_health_enabled``. + """ + if not settings.gateway_health_enabled: + return False + owner = getattr(task, "assigned_to", None) or getattr(task, "claimed_by", None) + if not owner: + return False + slug = self._resolve_agent_slug(str(owner)) + if not await self._gateway_broken_past_grace(slug): + return False + try: + await self._remove_container(f"roboco-agent-{slug}") + except Exception as exc: + logger.error( + "broken-gateway kill failed; will retry next tick", + agent_id=slug, + error=str(exc), + ) + return False + self._instances.pop(slug, None) + self._gateway_broken_since.pop(slug, None) + logger.warning( + "broken-gateway agent killed and evicted", + agent_id=slug, + task_id=str(getattr(task, "id", "")), + ) + return True + + async def _gateway_broken_past_grace(self, slug: str) -> bool: + """True when ``slug``'s gateway has probed broken longer than the grace. + + Probe-inconclusive (None) or healthy clears the grace mark and returns + False; the first broken sighting records the mark and returns False (one + grace tick); a breakage older than ``gateway_health_grace_seconds`` (or a + test-injected ``_gateway_health_grace``) returns True. + """ + healthy = await self._probe_gateway_health(slug) + if healthy is None or healthy: + self._gateway_broken_since.pop(slug, None) + return False + now = datetime.now(UTC) + first_seen = self._gateway_broken_since.get(slug) + if first_seen is None: + self._gateway_broken_since[slug] = now + return False + grace = getattr(self, "_gateway_health_grace", None) + if grace is None: + grace = settings.gateway_health_grace_seconds + return (now - first_seen).total_seconds() >= grace + async def _reap_with_service(self, svc: "TaskService") -> None: """Inner reap loop, parameterized by the TaskService to use. @@ -7140,7 +7236,14 @@ Start now: evidence(task_id="{task_id}") live = self._assignee_has_active_instance( t ) or await self._assignee_container_running(t) - if live and not await self._maybe_kill_wedged_grok(t, ts): + # A live container is spared UNLESS it is wedged (grok) or its + # gateway is broken-but-alive past the grace window — both get + # killed + evicted here so we fall through to release + respawn. + if ( + live + and not await self._maybe_kill_wedged_grok(t, ts) + and not await self._maybe_recover_broken_gateway(t) + ): continue task_id = require_uuid(t.id) try: diff --git a/roboco/services/gateway/choreographer/_impl.py b/roboco/services/gateway/choreographer/_impl.py index 43ba297e..b3d95c72 100644 --- a/roboco/services/gateway/choreographer/_impl.py +++ b/roboco/services/gateway/choreographer/_impl.py @@ -847,11 +847,22 @@ class Choreographer: } return briefing + # PM coordinator roles plan + delegate many roots in parallel; the actual + # work then runs in the delegated children/cells, not in the PM's own hands. + # So the single-active-task concurrency invariants (already_active / paused) + # that keep a *developer* to one task at a time must NOT gate a coordinator + # — only a genuine upstream sequence dependency (unmet_dependency) may hold a + # PM's root back. Without this exemption a PM that claimed one root could + # never plan a second and thrashed between its claimed roots, respawning + # forever and burning tokens for zero progress. + _COORDINATOR_ROLES: ClassVar[frozenset[str]] = frozenset({"main_pm", "cell_pm"}) + async def _run_claim_guards( self, *, agent_id: UUID, task: Any, + role_str: str | None = None, ) -> Envelope | None: """Run concurrency-invariant claim guards. Returns rejection or None. @@ -861,14 +872,21 @@ class Choreographer: in the verb's spec gate; the former role-typed and pm_cannot_execute_code guards have been deleted. + ``role_str`` selects whether the single-active-task guards apply: a PM + coordinator (``_COORDINATOR_ROLES``) is exempt from ``already_active`` / + ``paused`` (it holds many roots in parallel by design) but the sequence + guard ``unmet_dependency`` still applies to everyone. A ``None`` role + keeps the full guards (safe default for non-PM callers). + Pre-gateway location: _helpers.py:124-204. """ - in_progress = await self.task.list_in_progress_for_agent(agent_id) - if guard := already_active_guard(in_progress, task.id): - return guard - paused = await self.task.list_paused_for_agent(agent_id) - if guard := paused_tasks_guard(paused): - return guard + if role_str not in self._COORDINATOR_ROLES: + in_progress = await self.task.list_in_progress_for_agent(agent_id) + if guard := already_active_guard(in_progress, task.id): + return guard + paused = await self.task.list_paused_for_agent(agent_id) + if guard := paused_tasks_guard(paused, task.id): + return guard dep_ids = list(task.dependency_ids) if dep_ids: unmet = await self.task.unmet_dependency_ids(dep_ids) @@ -1005,6 +1023,7 @@ class Choreographer: if guard := await self._run_claim_guards( agent_id=agent_id, task=t, + role_str=role_str, ): return await self._emit_rejection( self._with_briefing(guard, briefing).with_introspection( @@ -1099,6 +1118,7 @@ class Choreographer: if guard := await self._run_claim_guards( agent_id=ctx.agent_id, task=t, + role_str=role_str, ): return await self._emit_rejection( self._with_briefing(guard, briefing).with_introspection( diff --git a/roboco/services/gateway/choreographer/_protocol.py b/roboco/services/gateway/choreographer/_protocol.py index e8379615..1806e95e 100644 --- a/roboco/services/gateway/choreographer/_protocol.py +++ b/roboco/services/gateway/choreographer/_protocol.py @@ -111,6 +111,7 @@ class ChoreographerHelpers: *, agent_id: UUID, task: Any, + role_str: str | None = None, ) -> Envelope | None: raise NotImplementedError diff --git a/roboco/services/gateway/claim_guards.py b/roboco/services/gateway/claim_guards.py index 2b513d3f..f67efbf7 100644 --- a/roboco/services/gateway/claim_guards.py +++ b/roboco/services/gateway/claim_guards.py @@ -57,18 +57,24 @@ def already_active_guard( ) -def paused_tasks_guard(paused_tasks: list[Any]) -> Envelope | None: - """Refuse claim if agent has any paused tasks. +def paused_tasks_guard( + paused_tasks: list[Any], target_task_id: UUID | None = None +) -> Envelope | None: + """Refuse claim if agent has a paused task OTHER than the one being claimed. + + ``target_task_id`` is excluded so a re-entry on the agent's own paused task + (e.g. a PM re-planning an umbrella that ``i_am_idle`` auto-paused) is never + self-blocked — mirroring ``already_active_guard``'s target exclusion. Pre-gateway: _helpers.py:check_paused_tasks 154-165. """ - if not paused_tasks: + blocking = [t for t in paused_tasks if t.id != target_task_id] + if not blocking: return None - paused = paused_tasks[0] + paused = blocking[0] return Envelope.invalid_state( message=( - f"You have {len(paused_tasks)} paused task(s); resume before " - "claiming new work." + f"You have {len(blocking)} paused task(s); resume before claiming new work." ), remediate=( f"resume {paused.id} (call i_will_work_on again) before starting new work" diff --git a/roboco/services/metrics.py b/roboco/services/metrics.py index a9ba1ed6..ad2e5c3d 100644 --- a/roboco/services/metrics.py +++ b/roboco/services/metrics.py @@ -9,15 +9,29 @@ from datetime import UTC, datetime, timedelta from typing import Any, ClassVar from uuid import UUID -from sqlalchemy import and_, func, select +from sqlalchemy import and_, func, select, text from sqlalchemy.ext.asyncio import AsyncSession -from roboco.db.tables import AgentTable, MessageTable, NotificationTable, TaskTable +from roboco.db.tables import ( + AgentSpawnSessionTable, + AgentTable, + AuditLogTable, + MessageTable, + NotificationTable, + TaskTable, +) from roboco.models.base import TaskStatus, Team from roboco.models.metrics import ( AgentMetrics, + AgentReworkRate, BlockerMetrics, + BottleneckReport, + ReworkReport, + Scorecard, + StageBottleneck, + StageTiming, TeamMetrics, + TeamReworkRate, VelocityMetrics, ) from roboco.services.base import BaseService @@ -29,6 +43,18 @@ DEFAULT_COMM_HOURS = 24 HOURS_PER_DAY = 24 SECONDS_PER_HOUR = 3600 + +def _as_hours(value: Any) -> float | None: + """Coerce a SQL avg/extract aggregate to a rounded float, or None. + + ``EXTRACT(epoch ...)`` returns ``numeric`` on PostgreSQL 14+, which asyncpg + surfaces as a ``Decimal`` — and a ``Decimal`` serializes to a JSON *string*, + crashing the panel's numeric formatting (``value.toFixed(...)``). Rounding to + a real ``float`` here keeps every "hours" field a JSON number. + """ + return round(float(value), 2) if value else None + + # Active task statuses for get_team_metrics and related queries. # Note: BLOCKED is intentionally excluded here; get_health_status() uses its # own local list that includes BLOCKED to compute the blocked-task ratio. @@ -138,7 +164,7 @@ class MetricsService(BaseService): period=_format_period(days), tasks_completed=tasks_completed, tasks_created=tasks_created, - avg_completion_hours=round(avg_hours, 2) if avg_hours else None, + avg_completion_hours=_as_hours(avg_hours), completion_rate=round(completion_rate, 2), ) @@ -190,9 +216,9 @@ class MetricsService(BaseService): return BlockerMetrics( active_blockers=active_blockers, - avg_blocked_hours=round(avg_blocked, 2) if avg_blocked else None, + avg_blocked_hours=_as_hours(avg_blocked), longest_blocked_task_id=to_python_uuid(longest_task_id), - longest_blocked_hours=round(longest_hours, 2) if longest_hours else None, + longest_blocked_hours=_as_hours(longest_hours), blockers_by_team=blockers_by_team, ) @@ -289,7 +315,7 @@ class MetricsService(BaseService): active_tasks=active_tasks, completed_tasks_week=completed_tasks_week, blocked_tasks=blocked_tasks, - avg_completion_hours=round(avg_hours, 2) if avg_hours else None, + avg_completion_hours=_as_hours(avg_hours), documentation_coverage=round(doc_coverage, 2), ) @@ -365,7 +391,7 @@ class MetricsService(BaseService): agent_name=agent.name, tasks_completed_week=tasks_completed, current_task_id=to_python_uuid(agent.current_task_id), - avg_completion_hours=round(avg_hours, 2) if avg_hours else None, + avg_completion_hours=_as_hours(avg_hours), messages_sent_week=messages_sent, ) @@ -496,6 +522,377 @@ class MetricsService(BaseService): "completed_this_week": completed_count, } + # ========================================================================= + # OBSERVABILITY (cycle-time / bottleneck / rework / scorecard) + # ========================================================================= + + async def get_cycle_time_by_stage( + self, team: Team | None = None, days: int = 30 + ) -> list[StageTiming]: + """Per-stage dwell time reconstructed from the audit_log journey. + + Each generic ``task.`` event marks entry into a status; the + dwell in that status is the gap to the next event for the same task. + The named ``task.qa_fail`` / ``task.pr_fail`` events are excluded — + ``event_type = 'task.' || to_status`` keeps only generic transitions, so + the same-timestamp named events can't inject a zero-length stage. + """ + since = datetime.now(UTC) - timedelta(days=days) + # A NULL :team disables the team filter — the query is a static string + # (no interpolation), so the team value is only ever a bound parameter. + sql = text( + """ + WITH ordered AS ( + SELECT + (a.details->>'to_status') AS status, + a.timestamp AS entered_at, + LEAD(a.timestamp) OVER ( + PARTITION BY a.target_id ORDER BY a.timestamp + ) AS exited_at + FROM audit_log a + WHERE a.event_type LIKE 'task.%' + AND a.event_type = 'task.' || (a.details->>'to_status') + AND a.timestamp >= :since + AND (CAST(:team AS text) IS NULL OR a.details->>'team' = :team) + ) + SELECT + status, + AVG(EXTRACT(epoch FROM (exited_at - entered_at)))::float AS avg_s, + PERCENTILE_CONT(0.5) WITHIN GROUP ( + ORDER BY EXTRACT(epoch FROM (exited_at - entered_at)) + )::float AS median_s, + PERCENTILE_CONT(0.9) WITHIN GROUP ( + ORDER BY EXTRACT(epoch FROM (exited_at - entered_at)) + )::float AS p90_s, + COUNT(*) AS n + FROM ordered + WHERE exited_at IS NOT NULL + GROUP BY status + ORDER BY avg_s DESC + """ + ) + params: dict[str, Any] = {"since": since, "team": team.value if team else None} + rows = (await self.session.execute(sql, params)).all() + return [ + StageTiming( + status=r.status, + avg_seconds=float(r.avg_s or 0.0), + median_seconds=float(r.median_s or 0.0), + p90_seconds=float(r.p90_s or 0.0), + sample_size=int(r.n), + ) + for r in rows + ] + + async def get_bottleneck_distribution(self, days: int = 30) -> BottleneckReport: + """Where work piles up: cumulative dwell per stage + live parked counts.""" + stages = await self.get_cycle_time_by_stage(days=days) + cumulative = {s.status: s.avg_seconds * s.sample_size for s in stages} + total = sum(cumulative.values()) + + parked_rows = ( + await self.session.execute( + select(TaskTable.status, func.count(TaskTable.id)).group_by( + TaskTable.status + ) + ) + ).all() + parked = { + (st.value if hasattr(st, "value") else str(st)): cnt + for st, cnt in parked_rows + } + + by_stage = [ + StageBottleneck( + status=status, + cumulative_seconds=cum, + parked_now=parked.get(status, 0), + pct_of_total=(cum / total) if total else 0.0, + ) + for status, cum in cumulative.items() + ] + by_stage.sort(key=lambda s: s.cumulative_seconds, reverse=True) + blockers = await self.get_blocker_metrics() + return BottleneckReport( + by_stage=by_stage, + worst_stage=by_stage[0].status if by_stage else None, + active_blockers=blockers.active_blockers, + ) + + async def _completed_reworked_counts( + self, since: datetime, team: Team | None + ) -> tuple[int, int]: + """(#completed, #completed-with-a-rework) in the window, optional team.""" + base: list[Any] = [ + TaskTable.status == TaskStatus.COMPLETED, + TaskTable.completed_at >= since, + ] + if team: + base.append(TaskTable.team == team) + completed = ( + await self.session.execute( + select(func.count(TaskTable.id)).where(and_(*base)) + ) + ).scalar() or 0 + reworked = ( + await self.session.execute( + select(func.count(TaskTable.id)).where( + and_(*base, TaskTable.revision_count > 0) + ) + ) + ).scalar() or 0 + return completed, reworked + + async def _rework_by_agent(self, since: datetime) -> list[AgentReworkRate]: + """Per-agent rework: owner bounce-rate + reviewer-attributed fails.""" + fail_rows = ( + await self.session.execute( + select( + AuditLogTable.agent_id, + AuditLogTable.event_type, + func.count(AuditLogTable.id), + ) + .where( + AuditLogTable.event_type.in_(["task.qa_fail", "task.pr_fail"]), + AuditLogTable.timestamp >= since, + AuditLogTable.agent_id.isnot(None), + ) + .group_by(AuditLogTable.agent_id, AuditLogTable.event_type) + ) + ).all() + completed_by = await self._count_by_assignee(since, reworked_only=False) + reworked_by = await self._count_by_assignee(since, reworked_only=True) + + qa_by: dict[str, int] = {} + pr_by: dict[str, int] = {} + for agent_id, event_type, cnt in fail_rows: + key = str(agent_id) + (qa_by if event_type == "task.qa_fail" else pr_by)[key] = cnt + + agent_ids = set(completed_by) | set(qa_by) | set(pr_by) + if not agent_ids: + return [] + slug_rows = ( + await self.session.execute( + select(AgentTable.id, AgentTable.slug).where( + AgentTable.id.in_([UUID(a) for a in agent_ids]) + ) + ) + ).all() + slug_by = {str(i): s for i, s in slug_rows} + + out = [ + AgentReworkRate( + agent_slug=slug_by.get(aid, aid), + rate=( + reworked_by.get(aid, 0) / completed_by[aid] + if completed_by.get(aid) + else 0.0 + ), + qa_fails=qa_by.get(aid, 0), + pr_fails=pr_by.get(aid, 0), + ) + for aid in agent_ids + ] + out.sort(key=lambda a: (a.qa_fails + a.pr_fails, a.rate), reverse=True) + return out + + async def _count_by_assignee( + self, since: datetime, *, reworked_only: bool + ) -> dict[str, int]: + """#completed tasks per assignee in the window (optionally rework-only).""" + conds: list[Any] = [ + TaskTable.status == TaskStatus.COMPLETED, + TaskTable.completed_at >= since, + TaskTable.assigned_to.isnot(None), + ] + if reworked_only: + conds.append(TaskTable.revision_count > 0) + rows = ( + await self.session.execute( + select(TaskTable.assigned_to, func.count(TaskTable.id)) + .where(and_(*conds)) + .group_by(TaskTable.assigned_to) + ) + ).all() + return {str(a): c for a, c in rows} + + async def _rework_cost(self, since: datetime, team: Team | None) -> float: + """Total spawn-session cost of the reworked tasks in the window.""" + base: list[Any] = [ + TaskTable.status == TaskStatus.COMPLETED, + TaskTable.completed_at >= since, + TaskTable.revision_count > 0, + ] + if team: + base.append(TaskTable.team == team) + ids = ( + (await self.session.execute(select(TaskTable.id).where(and_(*base)))) + .scalars() + .all() + ) + if not ids: + return 0.0 + cost = ( + await self.session.execute( + select( + func.coalesce( + func.sum(AgentSpawnSessionTable.estimated_cost_usd), 0.0 + ) + ).where(AgentSpawnSessionTable.task_id.in_([str(i) for i in ids])) + ) + ).scalar() or 0.0 + return float(cost) + + async def get_rework_metrics( + self, team: Team | None = None, days: int = 30 + ) -> ReworkReport: + """Rework rate (bounced/completed) overall, by team, and by agent + cost.""" + since = datetime.now(UTC) - timedelta(days=days) + completed, reworked = await self._completed_reworked_counts(since, team) + by_team = [] + for t in [Team.BACKEND, Team.FRONTEND, Team.UX_UI]: + c, r = await self._completed_reworked_counts(since, t) + by_team.append(TeamReworkRate(team=t.value, rate=(r / c if c else 0.0))) + by_agent = await self._rework_by_agent(since) + cost = await self._rework_cost(since, team) + return ReworkReport( + rate=(reworked / completed if completed else 0.0), + total_completed=completed, + total_reworked=reworked, + by_team=by_team, + by_agent=by_agent, + rework_cost_usd=cost, + ) + + async def _tokens_cost_for( + self, *, agent_slug: str | None, team: Team | None, since: datetime + ) -> tuple[int, float]: + """Sum (tokens, cost) from spawn sessions for an agent or a team.""" + tok = ( + AgentSpawnSessionTable.tokens_input + + AgentSpawnSessionTable.tokens_output + + AgentSpawnSessionTable.tokens_cache_read + + AgentSpawnSessionTable.tokens_cache_write + ) + conds: list[Any] = [AgentSpawnSessionTable.started_at >= since] + if agent_slug is not None: + conds.append(AgentSpawnSessionTable.agent_slug == agent_slug) + if team is not None: + conds.append(AgentSpawnSessionTable.team == team.value) + row = ( + await self.session.execute( + select( + func.coalesce(func.sum(tok), 0), + func.coalesce( + func.sum(AgentSpawnSessionTable.estimated_cost_usd), 0.0 + ), + ).where(and_(*conds)) + ) + ).first() + return (int(row[0]) if row else 0, float(row[1]) if row else 0.0) + + async def get_scorecard( + self, + agent_id: UUID | None = None, + team: Team | None = None, + days: int = 7, + ) -> Scorecard | None: + """Fused per-agent or per-cell delivery scorecard (None if agent absent).""" + since = datetime.now(UTC) - timedelta(days=days) + if agent_id is not None: + agent = ( + await self.session.execute( + select(AgentTable).where(AgentTable.id == agent_id) + ) + ).scalar_one_or_none() + if agent is None: + return None + completed, reworked = await self._completed_for_owner(agent_id, since) + avg_hours = await self._avg_cycle_hours( + owner=agent_id, team=None, since=since + ) + tokens, cost = await self._tokens_cost_for( + agent_slug=agent.slug, team=None, since=since + ) + return Scorecard( + scope="agent", + id=str(agent_id), + name=agent.name, + tasks_completed=completed, + avg_cycle_hours=avg_hours, + rework_rate=(reworked / completed if completed else 0.0), + tokens=tokens, + cost_usd=cost, + ) + if team is not None: + completed, reworked = await self._completed_reworked_counts(since, team) + avg_hours = await self._avg_cycle_hours(owner=None, team=team, since=since) + tokens, cost = await self._tokens_cost_for( + agent_slug=None, team=team, since=since + ) + return Scorecard( + scope="cell", + id=team.value, + name=team.value, + tasks_completed=completed, + avg_cycle_hours=avg_hours, + rework_rate=(reworked / completed if completed else 0.0), + tokens=tokens, + cost_usd=cost, + ) + return None + + async def _completed_for_owner( + self, agent_id: UUID, since: datetime + ) -> tuple[int, int]: + """(#completed, #reworked) tasks owned by an agent in the window.""" + base: list[Any] = [ + TaskTable.assigned_to == agent_id, + TaskTable.status == TaskStatus.COMPLETED, + TaskTable.completed_at >= since, + ] + completed = ( + await self.session.execute( + select(func.count(TaskTable.id)).where(and_(*base)) + ) + ).scalar() or 0 + reworked = ( + await self.session.execute( + select(func.count(TaskTable.id)).where( + and_(*base, TaskTable.revision_count > 0) + ) + ) + ).scalar() or 0 + return completed, reworked + + async def _avg_cycle_hours( + self, *, owner: UUID | None, team: Team | None, since: datetime + ) -> float | None: + """Average completed-task cycle time (started→completed) in hours.""" + conds: list[Any] = [ + TaskTable.completed_at >= since, + TaskTable.started_at.isnot(None), + TaskTable.status == TaskStatus.COMPLETED, + ] + if owner is not None: + conds.append(TaskTable.assigned_to == owner) + if team is not None: + conds.append(TaskTable.team == team) + avg_hours = ( + await self.session.execute( + select( + func.avg( + func.extract( + "epoch", TaskTable.completed_at - TaskTable.started_at + ) + / 3600 + ) + ).where(and_(*conds)) + ) + ).scalar() + return _as_hours(avg_hours) + # ============================================================================= # SERVICE FACTORY diff --git a/roboco/services/settings.py b/roboco/services/settings.py index 7988cfb0..dc5186f8 100644 --- a/roboco/services/settings.py +++ b/roboco/services/settings.py @@ -55,6 +55,7 @@ FEATURE_FLAGS: tuple[tuple[str, str], ...] = ( ("conventions_enabled", "Architectural conventions standard"), ("rag_auto_update_enabled", "RAG auto-update"), ("transcript_prune_enabled", "Transcript pruning"), + ("gateway_health_enabled", "Gateway-health recovery"), ) _FEATURE_FLAG_KEYS = tuple(key for key, _ in FEATURE_FLAGS) diff --git a/roboco/services/task.py b/roboco/services/task.py index be66b4be..35af15b2 100644 --- a/roboco/services/task.py +++ b/roboco/services/task.py @@ -527,6 +527,17 @@ class TaskService(BaseService): from roboco.services.audit import get_audit_service + # Rework counter: a bounce INTO needs_revision (not a re-entry) is one + # rework cycle. Incremented at this single chokepoint — every transition + # path funnels its audit through here exactly once — so the rework rate + # is an O(1) column read. Synchronous (part of this unit of work), + # unlike the fire-and-forget audit rows below. + if ( + to_status == TaskStatus.NEEDS_REVISION.value + and from_status != TaskStatus.NEEDS_REVISION.value + ): + task.revision_count = (task.revision_count or 0) + 1 + if audit_agent_id is not None: resolved_audit_agent_id: str | None = str(audit_agent_id) elif task.claimed_by is not None: @@ -534,27 +545,46 @@ class TaskService(BaseService): else: resolved_audit_agent_id = None + details = { + "from_status": from_status, + "to_status": to_status, + "agent_role": agent_role, + "team": ( + task.team.value if hasattr(task.team, "value") else str(task.team) + ), + } audit = get_audit_service() with contextlib.suppress(RuntimeError): - bg = asyncio.get_running_loop().create_task( - audit.log_task_event( - event_type=f"task.{to_status}", - task_id=str(task.id), - agent_id=resolved_audit_agent_id, - details={ - "from_status": from_status, - "to_status": to_status, - "agent_role": agent_role, - "team": ( - task.team.value - if hasattr(task.team, "value") - else str(task.team) - ), - }, + loop = asyncio.get_running_loop() + for event_type in self._audit_events_for(to_status, agent_role): + bg = loop.create_task( + audit.log_task_event( + event_type=event_type, + task_id=str(task.id), + agent_id=resolved_audit_agent_id, + details=details, + ) ) - ) - self._background_tasks.add(bg) - bg.add_done_callback(self._background_tasks.discard) + self._background_tasks.add(bg) + bg.add_done_callback(self._background_tasks.discard) + + @staticmethod + def _audit_events_for(to_status: str, agent_role: str | None) -> list[str]: + """Audit event types to emit for a transition. + + Always the generic ``task.``; plus a rejector-attributed + ``task.qa_fail`` / ``task.pr_fail`` when a reviewer bounces a task to + needs_revision — so the per-agent rework scorecard can charge the + rejection to the QA / PR-reviewer who made it (the audit row carries + their agent_id), not the developer who owns the task. + """ + events = [f"task.{to_status}"] + if to_status == TaskStatus.NEEDS_REVISION.value: + if agent_role == "pr_reviewer": + events.append("task.pr_fail") + elif agent_role == "qa": + events.append("task.qa_fail") + return events # ========================================================================= # CRUD OPERATIONS diff --git a/tests/integration/test_dashboard_routes.py b/tests/integration/test_dashboard_routes.py index 85bfe1e8..afab96b9 100644 --- a/tests/integration/test_dashboard_routes.py +++ b/tests/integration/test_dashboard_routes.py @@ -93,6 +93,55 @@ async def test_get_auditor_flags(dashboard_client: AsyncClient) -> None: assert isinstance(response.json(), list) +# --------------------------------------------------------------------------- +# Observability endpoints (0.10.0) +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_cycle_time_endpoint(dashboard_client: AsyncClient) -> None: + resp = await dashboard_client.get( + "/api/dashboard/metrics/cycle-time?days=30", headers=_HDR + ) + assert resp.status_code == HTTPStatus.OK + assert isinstance(resp.json(), list) + + +@pytest.mark.asyncio +async def test_bottlenecks_endpoint(dashboard_client: AsyncClient) -> None: + resp = await dashboard_client.get( + "/api/dashboard/metrics/bottlenecks", headers=_HDR + ) + assert resp.status_code == HTTPStatus.OK + body = resp.json() + assert "by_stage" in body and "worst_stage" in body and "active_blockers" in body + + +@pytest.mark.asyncio +async def test_rework_endpoint(dashboard_client: AsyncClient) -> None: + resp = await dashboard_client.get("/api/dashboard/metrics/rework", headers=_HDR) + assert resp.status_code == HTTPStatus.OK + body = resp.json() + assert "rate" in body and "by_team" in body and "by_agent" in body + + +@pytest.mark.asyncio +async def test_agent_scorecard_404_when_absent(dashboard_client: AsyncClient) -> None: + resp = await dashboard_client.get( + f"/api/dashboard/metrics/scorecard/agent/{uuid4()}", headers=_HDR + ) + assert resp.status_code == HTTPStatus.NOT_FOUND + + +@pytest.mark.asyncio +async def test_team_scorecard_endpoint(dashboard_client: AsyncClient) -> None: + resp = await dashboard_client.get( + "/api/dashboard/metrics/scorecard/team/backend", headers=_HDR + ) + assert resp.status_code == HTTPStatus.OK + assert resp.json()["scope"] == "cell" + + @pytest.mark.asyncio async def test_resolve_auditor_flag(dashboard_client: AsyncClient) -> None: create = await dashboard_client.post( diff --git a/tests/integration/test_metrics_observability.py b/tests/integration/test_metrics_observability.py new file mode 100644 index 00000000..ea501971 --- /dev/null +++ b/tests/integration/test_metrics_observability.py @@ -0,0 +1,283 @@ +"""0.10.0 observability metric layer: cycle-time, bottleneck, rework, scorecard. + +Seeds an audit_log journey + reworked tasks + rejector-attributed fail events + +spawn-session costs against a real Postgres and asserts the reconstructed +metrics. The named task.qa_fail / task.pr_fail events must NOT pollute the +cycle-time reconstruction (they share a timestamp with the needs_revision row). +""" + +from __future__ import annotations + +from datetime import UTC, datetime, timedelta +from typing import TYPE_CHECKING, Any +from uuid import uuid4 + +import pytest +import pytest_asyncio +from roboco.db.tables import ( + AgentSpawnSessionTable, + AgentTable, + AuditLogTable, + ProjectTable, + TaskTable, +) +from roboco.models.base import ( + AgentRole, + AgentStatus, + Complexity, + TaskNature, + TaskStatus, + TaskType, + Team, +) +from roboco.services.metrics import MetricsService + +if TYPE_CHECKING: + from collections.abc import AsyncIterator + + from sqlalchemy.ext.asyncio import AsyncSession + +_T0 = datetime(2026, 6, 20, 12, 0, 0, tzinfo=UTC) +_EXPECTED_TASKS = 2 # completed tasks seeded per rework / scorecard test +_EXPECTED_TOKENS = 1500 # 1000 input + 500 output in the scorecard spawn session + + +def _agent(role: AgentRole, team: Team, slug: str) -> AgentTable: + return AgentTable( + id=uuid4(), + name=slug, + slug=slug, + role=role, + team=team, + status=AgentStatus.ACTIVE, + model_config={}, + system_prompt="x", + capabilities=[], + permissions={}, + metrics={}, + ) + + +def _task( + project_id: Any, + created_by: Any, + *, + assigned_to: Any = None, + revision_count: int = 0, + started_hours_ago: int | None = None, +) -> TaskTable: + """A COMPLETED backend task completed `now` (in-window), optionally started.""" + started = ( + datetime.now(UTC) - timedelta(hours=started_hours_ago) + if started_hours_ago is not None + else None + ) + return TaskTable( + id=uuid4(), + title="t", + description="d", + acceptance_criteria=["ac"], + task_type=TaskType.CODE, + nature=TaskNature.TECHNICAL, + status=TaskStatus.COMPLETED, + team=Team.BACKEND, + project_id=project_id, + created_by=created_by, + assigned_to=assigned_to, + revision_count=revision_count, + estimated_complexity=Complexity.MEDIUM, + completed_at=datetime.now(UTC), + started_at=started, + ) + + +def _audit( + task_id: Any, + status: str, + ts: datetime, + *, + agent_id: Any = None, + event_type: str | None = None, +) -> AuditLogTable: + return AuditLogTable( + id=uuid4(), + event_type=event_type or f"task.{status}", + agent_id=agent_id, + target_type="task", + target_id=task_id, + severity="info", + details={"to_status": status, "from_status": "prev", "team": "backend"}, + timestamp=ts, + ) + + +def _spawn( + slug: str, *, task_id: str | None, cost: float, tokens_in: int, tokens_out: int +) -> AgentSpawnSessionTable: + return AgentSpawnSessionTable( + id=uuid4(), + agent_slug=slug, + team="backend", + role="developer", + model="claude", + task_id=task_id, + started_at=datetime.now(UTC) - timedelta(hours=1), + tokens_input=tokens_in, + tokens_output=tokens_out, + estimated_cost_usd=cost, + ) + + +@pytest_asyncio.fixture +async def obs_setup(db_session: AsyncSession) -> AsyncIterator[dict]: + dev = _agent(AgentRole.DEVELOPER, Team.BACKEND, f"be-dev-{uuid4().hex[:6]}") + qa = _agent(AgentRole.QA, Team.BACKEND, f"be-qa-{uuid4().hex[:6]}") + db_session.add_all([dev, qa]) + await db_session.flush() + project = ProjectTable( + id=uuid4(), + name="P", + slug=f"p-{uuid4().hex[:6]}", + git_url="https://example.com/r.git", + assigned_cell=Team.BACKEND, + created_by=dev.id, + ) + db_session.add(project) + await db_session.flush() + yield { + "svc": MetricsService(db_session), + "db": db_session, + "project_id": project.id, + "dev_id": dev.id, + "qa_id": qa.id, + "dev_slug": dev.slug, + } + + +@pytest.mark.asyncio +async def test_cycle_time_reconstructs_per_stage_dwell(obs_setup: dict) -> None: + db = obs_setup["db"] + tid = uuid4() + # claimed (60s) -> in_progress (3600s) -> awaiting_qa (120s) -> completed + db.add_all( + [ + _audit(tid, "claimed", _T0), + _audit(tid, "in_progress", _T0 + timedelta(seconds=60)), + _audit(tid, "awaiting_qa", _T0 + timedelta(seconds=60 + 3600)), + _audit(tid, "completed", _T0 + timedelta(seconds=60 + 3600 + 120)), + # A named fail event sharing the awaiting_qa timestamp must be ignored. + _audit( + tid, + "needs_revision", + _T0 + timedelta(seconds=60 + 3600), + event_type="task.qa_fail", + ), + ] + ) + await db.flush() + stages = {s.status: s for s in await obs_setup["svc"].get_cycle_time_by_stage()} + assert stages["claimed"].avg_seconds == pytest.approx(60.0) + assert stages["in_progress"].avg_seconds == pytest.approx(3600.0) + assert stages["awaiting_qa"].avg_seconds == pytest.approx(120.0) + # The named qa_fail event did not create a zero-length needs_revision stage. + assert "needs_revision" not in stages + + +@pytest.mark.asyncio +async def test_bottleneck_ranks_longest_cumulative_stage(obs_setup: dict) -> None: + db = obs_setup["db"] + tid = uuid4() + db.add_all( + [ + _audit(tid, "claimed", _T0), + _audit(tid, "in_progress", _T0 + timedelta(seconds=60)), + _audit(tid, "awaiting_qa", _T0 + timedelta(seconds=60 + 7200)), + _audit(tid, "completed", _T0 + timedelta(seconds=60 + 7200 + 30)), + ] + ) + await db.flush() + report = await obs_setup["svc"].get_bottleneck_distribution() + assert report.worst_stage == "in_progress" + assert report.by_stage[0].status == "in_progress" + + +@pytest.mark.asyncio +async def test_rework_rate_and_attribution(obs_setup: dict) -> None: + db = obs_setup["db"] + pid, dev_id, qa_id = ( + obs_setup["project_id"], + obs_setup["dev_id"], + obs_setup["qa_id"], + ) + clean = _task(pid, dev_id, assigned_to=dev_id) + reworked = _task(pid, dev_id, assigned_to=dev_id, revision_count=2) + db.add_all([clean, reworked]) + await db.flush() + # The QA agent bounced the reworked task once (rejector attribution). + db.add( + _audit( + reworked.id, + "needs_revision", + datetime.now(UTC) - timedelta(hours=1), + agent_id=qa_id, + event_type="task.qa_fail", + ) + ) + # A spawn session attributes the rework's cost. + db.add( + _spawn( + obs_setup["dev_slug"], + task_id=str(reworked.id), + cost=0.42, + tokens_in=100, + tokens_out=50, + ) + ) + await db.flush() + + report = await obs_setup["svc"].get_rework_metrics(days=30) + assert report.total_completed == _EXPECTED_TASKS + assert report.total_reworked == 1 + assert report.rate == pytest.approx(0.5) + assert report.rework_cost_usd == pytest.approx(0.42) + qa_row = next(a for a in report.by_agent if a.qa_fails > 0) + assert qa_row.qa_fails == 1 + + +@pytest.mark.asyncio +async def test_scorecard_agent_and_cell(obs_setup: dict) -> None: + db = obs_setup["db"] + pid, dev_id = obs_setup["project_id"], obs_setup["dev_id"] + db.add_all( + [ + _task(pid, dev_id, assigned_to=dev_id, started_hours_ago=2), + _task( + pid, dev_id, assigned_to=dev_id, revision_count=1, started_hours_ago=4 + ), + ] + ) + db.add( + _spawn( + obs_setup["dev_slug"], + task_id=None, + cost=1.25, + tokens_in=1000, + tokens_out=500, + ) + ) + await db.flush() + + card = await obs_setup["svc"].get_scorecard(agent_id=dev_id, days=7) + assert card is not None + assert card.scope == "agent" + assert card.tasks_completed == _EXPECTED_TASKS + assert card.rework_rate == pytest.approx(0.5) + assert card.tokens == _EXPECTED_TOKENS + assert card.cost_usd == pytest.approx(1.25) + + cell = await obs_setup["svc"].get_scorecard(team=Team.BACKEND, days=7) + assert cell is not None + assert cell.scope == "cell" + assert cell.tasks_completed == _EXPECTED_TASKS + + assert await obs_setup["svc"].get_scorecard(agent_id=uuid4()) is None diff --git a/tests/integration/test_migration_observability.py b/tests/integration/test_migration_observability.py new file mode 100644 index 00000000..06213e0f --- /dev/null +++ b/tests/integration/test_migration_observability.py @@ -0,0 +1,43 @@ +"""0.10.0 observability: tasks.revision_count + the audit_log query index. + +Migration 045 adds ``tasks.revision_count`` (the O(1) rework counter — +forward-only, existing rows default to 0) and the composite index +``audit_log(target_id, event_type, timestamp)`` that powers the cycle-time and +rework reconstruction queries. The real upgrade/downgrade chain is verified +separately against a throwaway Postgres (see project migration-verification +discipline); these assertions guard the resulting schema shape. +""" + +from __future__ import annotations + +import pytest +from sqlalchemy import text + + +@pytest.mark.asyncio +async def test_revision_count_defaults_to_zero(db_session) -> None: # type: ignore[no-untyped-def] + result = await db_session.execute( + text( + "SELECT column_default, is_nullable " + "FROM information_schema.columns " + "WHERE table_name = 'tasks' AND column_name = 'revision_count'" + ) + ) + row = result.first() + assert row is not None, "tasks.revision_count column must exist" + assert row[1] == "NO", "revision_count must be NOT NULL" + assert "0" in (row[0] or ""), "revision_count must default to 0" + + +@pytest.mark.asyncio +async def test_audit_log_query_index_exists(db_session) -> None: # type: ignore[no-untyped-def] + result = await db_session.execute( + text( + "SELECT indexname FROM pg_indexes " + "WHERE tablename = 'audit_log' " + "AND indexname = 'ix_audit_log_target_event_ts'" + ) + ) + assert result.first() is not None, ( + "composite index ix_audit_log_target_event_ts must exist on audit_log" + ) diff --git a/tests/integration/test_task_service_transitions.py b/tests/integration/test_task_service_transitions.py index 81151cc2..1776c25c 100644 --- a/tests/integration/test_task_service_transitions.py +++ b/tests/integration/test_task_service_transitions.py @@ -536,6 +536,32 @@ async def test_fail_qa_reassigns_to_original_developer( assert failed.assigned_to == dev_id +@pytest.mark.asyncio +async def test_fail_qa_increments_revision_count( + task_setup: dict, db_session: AsyncSession +) -> None: + """Each QA bounce to needs_revision increments the O(1) rework counter.""" + svc = task_setup["svc"] + dev_id = task_setup["agent_id"] + task = await svc.create(_req(task_setup)) + task.status = TaskStatus.AWAITING_QA + task.orchestration_markers = {"original_developer": str(dev_id)} + await db_session.flush() + assert task.revision_count == 0 + + failed = await svc.fail_qa(task.id, notes="missing tests") + assert failed is not None + assert failed.revision_count == 1 + count_after_first = failed.revision_count + + # A second QA cycle bumps it again (once per transition into needs_revision). + failed.status = TaskStatus.AWAITING_QA + await db_session.flush() + again = await svc.fail_qa(task.id, notes="still missing") + assert again is not None + assert again.revision_count == count_after_first + 1 + + @pytest.mark.asyncio async def test_fail_qa_with_no_original_dev_unassigns( task_setup: dict, db_session: AsyncSession diff --git a/tests/unit/api/test_schemas_tasks.py b/tests/unit/api/test_schemas_tasks.py index 497704fa..dc5ba7f8 100644 --- a/tests/unit/api/test_schemas_tasks.py +++ b/tests/unit/api/test_schemas_tasks.py @@ -255,6 +255,23 @@ def test_transform_update_data_handles_null_unassign() -> None: assert out["assigned_to"] is None +def test_transform_update_data_passes_sequence() -> None: + """sequence is editable via PATCH (panel task-details sequence editor).""" + new_order = 3 + update = TaskUpdate(sequence=new_order) + out = transform_update_data(update) + assert out["sequence"] == new_order + # Unset sequence is omitted (exclude_unset), so a partial PATCH never + # clobbers the existing order. + assert "sequence" not in transform_update_data(TaskUpdate(priority=1)) + + +def test_task_update_sequence_rejects_negative() -> None: + """sequence has ge=0 — a negative order is a validation error, not stored.""" + with pytest.raises(ValueError, match="sequence"): + TaskUpdate(sequence=-1) + + # --------------------------------------------------------------------------- # task_to_response / task_list_to_response # --------------------------------------------------------------------------- diff --git a/tests/unit/gateway/test_choreographer_claim_guards.py b/tests/unit/gateway/test_choreographer_claim_guards.py index a91893e9..968f598d 100644 --- a/tests/unit/gateway/test_choreographer_claim_guards.py +++ b/tests/unit/gateway/test_choreographer_claim_guards.py @@ -19,6 +19,7 @@ from uuid import uuid4 import pytest from roboco.services.gateway.choreographer import Choreographer, ChoreographerDeps +from roboco.services.gateway.claim_guards import paused_tasks_guard # #172: a developer fresh claim must carry a substantive step checklist. # Inert on re-entry/error/non-dev paths, so safe to pass everywhere. @@ -559,3 +560,137 @@ async def test_claim_doc_task_blocks_when_documenter_has_paused_task() -> None: assert body["error"] == "invalid_state" assert "resume" in body["remediate"].lower() task_svc.doc_claim.assert_not_awaited() + + +# --------------------------------------------------------------------------- +# Coordinator exemption — a PM plans + delegates many roots in parallel, so the +# single-active-task guards (already_active / paused) must NOT gate it; only a +# real upstream sequence dependency may hold a PM's root back. (Developers stay +# blocked — see the A.2 / A.3 tests above.) +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_main_pm_can_plan_second_root_despite_active_and_paused() -> None: + """A main_pm may plan a new root while it already holds one in_progress and + one paused root — the developer concurrency guards do not gate a coordinator. + """ + pm_id = uuid4() + task_id = uuid4() + target = MagicMock( + id=task_id, + status="pending", + plan=None, + assigned_to=None, + parent_task_id=None, + sequence=0, + task_type="code", + team="main_pm", + ) + claimed = MagicMock( + id=task_id, status="claimed", plan=None, assigned_to=pm_id, task_type="code" + ) + started = MagicMock( + id=task_id, + status="in_progress", + plan={"text": "x"}, + assigned_to=pm_id, + task_type="code", + ) + # The coordinator already holds one in_progress root and one paused root — + # both would trip the guards for a non-PM caller. + other_active = MagicMock(id=uuid4(), status="in_progress") + other_paused = MagicMock(id=uuid4(), status="paused") + task_svc = _task_svc_with( + target, + role="main_pm", + agent_id=pm_id, + lookups={"in_progress": [other_active], "paused": [other_paused]}, + ) + task_svc.claim.return_value = claimed + task_svc.set_plan.return_value = claimed + task_svc.start.return_value = started + deps = _make_deps(task=task_svc) + c = Choreographer(deps) + + env = await c.i_will_plan( + pm_id, + task_id, + plan="route to backend + frontend cells", + rich_plan={ + "approach": ( + "Route this root to the backend and frontend cells in parallel: " + "be-pm owns the API contract, fe-pm consumes it. No cross-cell " + "dependency for this slice, so both cells start at once." + ), + "sub_tasks": [ + { + "title": "Backend slice", + "description": ( + "be-pm decomposes the API change and assigns be-dev-1, " + "who implements with tests and opens the leaf PR for QA." + ), + } + ], + }, + ) + assert env.error is None, env.as_dict() + task_svc.start.assert_awaited() + + +@pytest.mark.asyncio +async def test_main_pm_recovers_claimed_root_with_paused_sibling() -> None: + """The live deadlock: a respawned main_pm re-enters i_will_plan on a stuck + `claimed` root while another root is paused (i_am_idle auto-paused it). The + paused guard must NOT block the coordinator's recovery — set_plan + start + runs and the root reaches in_progress. + """ + pm_id = uuid4() + task_id = uuid4() + claimed = MagicMock( + id=task_id, + status="claimed", + plan=None, + assigned_to=pm_id, + parent_task_id=None, + sequence=0, + task_type="code", + team="main_pm", + branch_name="feature/main_pm/abc", + ) + started = MagicMock( + id=task_id, + status="in_progress", + plan={"text": "x"}, + assigned_to=pm_id, + task_type="code", + ) + other_paused = MagicMock(id=uuid4(), status="paused") + task_svc = _task_svc_with( + claimed, role="main_pm", agent_id=pm_id, lookups={"paused": [other_paused]} + ) + task_svc.set_plan.return_value = started + task_svc.start.return_value = started + deps = _make_deps(task=task_svc) + c = Choreographer(deps) + + env = await c.i_will_plan( + pm_id, task_id, plan="route this root to the backend + frontend cells" + ) + assert env.error is None, env.as_dict() + task_svc.start.assert_awaited_once_with(task_id, pm_id) + + +def test_paused_tasks_guard_excludes_target() -> None: + """A paused task that IS the claim target must not self-block, mirroring + already_active_guard's target exclusion (the 2026-06-14 self-deadlock).""" + target_id = uuid4() + other_id = uuid4() + # Only the target itself is paused -> no block. + assert paused_tasks_guard([MagicMock(id=target_id)], target_id) is None + # A different paused task -> block, naming that task. + env = paused_tasks_guard([MagicMock(id=other_id)], target_id) + assert env is not None + assert str(other_id) in env.as_dict()["remediate"] + # Back-compat: with no target supplied, any paused task blocks. + assert paused_tasks_guard([MagicMock(id=other_id)]) is not None diff --git a/tests/unit/gateway/test_choreographer_impl_branches.py b/tests/unit/gateway/test_choreographer_impl_branches.py index ba3c29bd..69e9fbc0 100644 --- a/tests/unit/gateway/test_choreographer_impl_branches.py +++ b/tests/unit/gateway/test_choreographer_impl_branches.py @@ -362,10 +362,12 @@ async def test_i_will_work_on_in_progress_assigned_to_self_idempotent() -> None: @pytest.mark.asyncio -async def test_i_will_plan_pm_with_already_active_task_rejects() -> None: - """The already_active_guard still fires on i_will_plan even though - pm_cannot_execute_code is skipped. Covers _impl.py:1106-1108 - (with-briefing wrap of the guard rejection). +async def test_i_will_plan_pm_exempt_from_already_active_guard() -> None: + """A PM coordinator is exempt from already_active_guard on i_will_plan: it + plans + delegates many roots in parallel, so holding one in_progress root + must NOT block planning another. (The guard still fires for developers — see + test_choreographer_claim_guards.py.) Repurposed from the pre-fix test that + asserted the now-removed PM block. """ pm_id = uuid4() task_id = uuid4() @@ -380,12 +382,25 @@ async def test_i_will_plan_pm_with_already_active_task_rejects() -> None: parent_task_id=None, task_type="planning", ) + started = MagicMock( + id=task_id, + status="in_progress", + plan={"text": "x"}, + assigned_to=pm_id, + title="t", + team="backend", + task_type="planning", + ) busy_task = MagicMock(id=other_task_id, status="in_progress") task_svc = AsyncMock() task_svc.get.return_value = target task_svc.agent_for.return_value = MagicMock(role="cell_pm", team="backend") task_svc.list_in_progress_for_agent.return_value = [busy_task] task_svc.list_paused_for_agent.return_value = [] + task_svc.get_subtasks.return_value = [] + task_svc.claim.return_value = target + task_svc.set_plan.return_value = target + task_svc.start.return_value = started deps = _make_deps(task=task_svc) c = Choreographer(deps) env = await c.i_will_plan( @@ -412,8 +427,8 @@ async def test_i_will_plan_pm_with_already_active_task_rejects() -> None: }, ) body = env.as_dict() - assert body["error"] == "invalid_state" - assert "in_progress task" in body["message"] + assert body.get("error") is None, body + task_svc.start.assert_awaited() @pytest.mark.asyncio diff --git a/tests/unit/runtime/test_gateway_health.py b/tests/unit/runtime/test_gateway_health.py new file mode 100644 index 00000000..8699767f --- /dev/null +++ b/tests/unit/runtime/test_gateway_health.py @@ -0,0 +1,195 @@ +"""Gateway-health recovery: probe a broken-but-alive agent + reap it past grace. + +The verb-heartbeat can't tell a quiet-healthy agent from one whose MCP gateway +is broken (corrupted /app/.venv) yet whose container is up. The reaper now probes +out-of-band and, past a grace window, kills + evicts the broken container so it +falls through to release + respawn instead of being protected forever. +""" + +from __future__ import annotations + +import asyncio +from datetime import UTC, datetime, timedelta +from typing import Any, cast +from unittest.mock import AsyncMock +from uuid import uuid4 + +import pytest +from roboco.config import settings +from roboco.runtime.orchestrator import AgentOrchestrator +from roboco.services.settings import FEATURE_FLAGS + + +class _FakeProc: + def __init__(self, rc: int) -> None: + self._rc = rc + + async def wait(self) -> int: + return self._rc + + +def _orch(monkeypatch: pytest.MonkeyPatch) -> AgentOrchestrator: + orch = AgentOrchestrator.__new__(AgentOrchestrator) # bypass __init__ + orch._instances = {} + orch._gateway_broken_since = {} + monkeypatch.setattr(orch, "_resolve_agent_slug", lambda _owner: "be-dev-1") + return orch + + +# ─── probe ────────────────────────────────────────────────────────────────── + + +@pytest.mark.asyncio +async def test_probe_healthy(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr( + asyncio, "create_subprocess_exec", AsyncMock(return_value=_FakeProc(0)) + ) + assert await AgentOrchestrator._probe_gateway_health("be-dev-1") is True + + +@pytest.mark.asyncio +async def test_probe_broken(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr( + asyncio, "create_subprocess_exec", AsyncMock(return_value=_FakeProc(1)) + ) + assert await AgentOrchestrator._probe_gateway_health("be-dev-1") is False + + +@pytest.mark.asyncio +async def test_probe_infra_error_is_none(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr( + asyncio, "create_subprocess_exec", AsyncMock(side_effect=OSError("no docker")) + ) + assert await AgentOrchestrator._probe_gateway_health("be-dev-1") is None + + +# ─── recovery decision ────────────────────────────────────────────────────── + + +def _task() -> Any: + return type("T", (), {"id": uuid4(), "assigned_to": uuid4(), "claimed_by": None})() + + +@pytest.mark.asyncio +async def test_disabled_never_recovers(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(settings, "gateway_health_enabled", False) + orch = _orch(monkeypatch) + remove = AsyncMock() + monkeypatch.setattr(orch, "_remove_container", remove) + monkeypatch.setattr(orch, "_probe_gateway_health", AsyncMock(return_value=False)) + assert await orch._maybe_recover_broken_gateway(_task()) is False + remove.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_healthy_is_spared(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(settings, "gateway_health_enabled", True) + orch = _orch(monkeypatch) + orch._gateway_broken_since["be-dev-1"] = datetime.now(UTC) # stale mark + remove = AsyncMock() + monkeypatch.setattr(orch, "_remove_container", remove) + monkeypatch.setattr(orch, "_probe_gateway_health", AsyncMock(return_value=True)) + assert await orch._maybe_recover_broken_gateway(_task()) is False + remove.assert_not_awaited() + assert "be-dev-1" not in orch._gateway_broken_since # mark cleared + + +@pytest.mark.asyncio +async def test_first_broken_sighting_waits_for_grace( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(settings, "gateway_health_enabled", True) + orch = _orch(monkeypatch) + remove = AsyncMock() + monkeypatch.setattr(orch, "_remove_container", remove) + monkeypatch.setattr(orch, "_probe_gateway_health", AsyncMock(return_value=False)) + assert await orch._maybe_recover_broken_gateway(_task()) is False + remove.assert_not_awaited() + assert "be-dev-1" in orch._gateway_broken_since # grace mark recorded + + +@pytest.mark.asyncio +async def test_broken_past_grace_is_killed(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(settings, "gateway_health_enabled", True) + orch = _orch(monkeypatch) + # _gateway_health_grace is a test-only injection read via getattr(..., None); + # 0 means "past grace immediately". + monkeypatch.setattr(orch, "_gateway_health_grace", 0, raising=False) + orch._gateway_broken_since["be-dev-1"] = datetime.now(UTC) - timedelta(seconds=5) + orch._instances["be-dev-1"] = cast("Any", object()) + remove = AsyncMock() + monkeypatch.setattr(orch, "_remove_container", remove) + monkeypatch.setattr(orch, "_probe_gateway_health", AsyncMock(return_value=False)) + assert await orch._maybe_recover_broken_gateway(_task()) is True + remove.assert_awaited_once_with("roboco-agent-be-dev-1") + assert "be-dev-1" not in orch._instances # evicted + + +@pytest.mark.asyncio +async def test_inconclusive_probe_is_spared(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(settings, "gateway_health_enabled", True) + orch = _orch(monkeypatch) + remove = AsyncMock() + monkeypatch.setattr(orch, "_remove_container", remove) + monkeypatch.setattr(orch, "_probe_gateway_health", AsyncMock(return_value=None)) + assert await orch._maybe_recover_broken_gateway(_task()) is False + remove.assert_not_awaited() + + +# ─── reaper wiring ────────────────────────────────────────────────────────── + + +def _stale_task() -> Any: + return type( + "T", + (), + { + "id": uuid4(), + "last_heartbeat_at": datetime.now(UTC) - timedelta(seconds=600), + }, + )() + + +@pytest.mark.asyncio +async def test_reaper_reaps_broken_gateway_agent( + monkeypatch: pytest.MonkeyPatch, +) -> None: + orch = AgentOrchestrator.__new__(AgentOrchestrator) + orch._claim_heartbeat_ttl = 300 + monkeypatch.setattr(orch, "_assignee_has_active_instance", lambda _t: True) + monkeypatch.setattr(orch, "_maybe_kill_wedged_grok", AsyncMock(return_value=False)) + monkeypatch.setattr( + orch, "_maybe_recover_broken_gateway", AsyncMock(return_value=True) + ) + task = _stale_task() + svc = AsyncMock() + svc.list_in_progress_or_claimed.return_value = [task] + svc.unclaim_for_reaper = AsyncMock() + await orch._reap_with_service(svc) + svc.unclaim_for_reaper.assert_awaited_once_with(task.id) + + +@pytest.mark.asyncio +async def test_reaper_spares_healthy_live_agent( + monkeypatch: pytest.MonkeyPatch, +) -> None: + orch = AgentOrchestrator.__new__(AgentOrchestrator) + orch._claim_heartbeat_ttl = 300 + monkeypatch.setattr(orch, "_assignee_has_active_instance", lambda _t: True) + monkeypatch.setattr(orch, "_maybe_kill_wedged_grok", AsyncMock(return_value=False)) + monkeypatch.setattr( + orch, "_maybe_recover_broken_gateway", AsyncMock(return_value=False) + ) + svc = AsyncMock() + svc.list_in_progress_or_claimed.return_value = [_stale_task()] + svc.unclaim_for_reaper = AsyncMock() + await orch._reap_with_service(svc) + svc.unclaim_for_reaper.assert_not_awaited() + + +# ─── config flag ──────────────────────────────────────────────────────────── + + +def test_flag_defaults_on_and_is_registered() -> None: + assert settings.gateway_health_enabled is True + assert "gateway_health_enabled" in {key for key, _ in FEATURE_FLAGS} diff --git a/tests/unit/runtime/test_stale_claim_reaper.py b/tests/unit/runtime/test_stale_claim_reaper.py index d9b47b84..481c458d 100644 --- a/tests/unit/runtime/test_stale_claim_reaper.py +++ b/tests/unit/runtime/test_stale_claim_reaper.py @@ -24,7 +24,9 @@ from roboco.seeds.initial_data import AGENT_UUIDS @pytest.mark.asyncio -async def test_reap_stale_claims_releases_dead_holders() -> None: +async def test_reap_stale_claims_releases_dead_holders( + monkeypatch: pytest.MonkeyPatch, +) -> None: """A task past TTL is unclaimed; a fresh one is left alone.""" stale_id = uuid4() fresh_id = uuid4() @@ -41,6 +43,9 @@ async def test_reap_stale_claims_releases_dead_holders() -> None: )() orch = AgentOrchestrator.__new__(AgentOrchestrator) # bypass __init__ + monkeypatch.setattr( + orch, "_maybe_recover_broken_gateway", AsyncMock(return_value=False) + ) orch._claim_heartbeat_ttl = 300 svc = AsyncMock() svc.list_in_progress_or_claimed.return_value = [stale_task, fresh_task] @@ -52,12 +57,17 @@ async def test_reap_stale_claims_releases_dead_holders() -> None: @pytest.mark.asyncio -async def test_reap_stale_claims_releases_holders_with_null_heartbeat() -> None: +async def test_reap_stale_claims_releases_holders_with_null_heartbeat( + monkeypatch: pytest.MonkeyPatch, +) -> None: """A claimed task that never heartbeated (NULL column) is treated as stale.""" null_id = uuid4() null_task = type("T", (), {"id": null_id, "last_heartbeat_at": None})() orch = AgentOrchestrator.__new__(AgentOrchestrator) + monkeypatch.setattr( + orch, "_maybe_recover_broken_gateway", AsyncMock(return_value=False) + ) orch._claim_heartbeat_ttl = 300 svc = AsyncMock() svc.list_in_progress_or_claimed.return_value = [null_task] @@ -69,7 +79,9 @@ async def test_reap_stale_claims_releases_holders_with_null_heartbeat() -> None: @pytest.mark.asyncio -async def test_reap_stale_claims_swallows_unclaim_errors() -> None: +async def test_reap_stale_claims_swallows_unclaim_errors( + monkeypatch: pytest.MonkeyPatch, +) -> None: """An unclaim_for_reaper failure must not abort the reap loop.""" stale_a = uuid4() stale_b = uuid4() @@ -82,6 +94,9 @@ async def test_reap_stale_claims_swallows_unclaim_errors() -> None: )() orch = AgentOrchestrator.__new__(AgentOrchestrator) + monkeypatch.setattr( + orch, "_maybe_recover_broken_gateway", AsyncMock(return_value=False) + ) orch._claim_heartbeat_ttl = 300 svc = AsyncMock() svc.list_in_progress_or_claimed.return_value = [task_a, task_b] @@ -95,7 +110,9 @@ async def test_reap_stale_claims_swallows_unclaim_errors() -> None: @pytest.mark.asyncio -async def test_reap_spares_claims_whose_assignee_container_is_alive() -> None: +async def test_reap_spares_claims_whose_assignee_container_is_alive( + monkeypatch: pytest.MonkeyPatch, +) -> None: """A stale-heartbeat task is NOT reaped while its assignee container lives. A developer deep in a long edit/test cycle outruns the heartbeat TTL; the @@ -128,6 +145,9 @@ async def test_reap_spares_claims_whose_assignee_container_is_alive() -> None: )() orch = AgentOrchestrator.__new__(AgentOrchestrator) + monkeypatch.setattr( + orch, "_maybe_recover_broken_gateway", AsyncMock(return_value=False) + ) orch._claim_heartbeat_ttl = 300 orch._instances = { "be-dev-1": AgentInstance(agent_id="be-dev-1", state=AgentState.ACTIVE) @@ -172,6 +192,9 @@ async def test_reaper_kills_and_releases_wedged_grok_container( )() orch = AgentOrchestrator.__new__(AgentOrchestrator) + monkeypatch.setattr( + orch, "_maybe_recover_broken_gateway", AsyncMock(return_value=False) + ) orch._claim_heartbeat_ttl = 300 orch._grok_idle_kill_ttl = 900 orch._instances = {"be-dev-1": _grok_instance()} @@ -210,6 +233,9 @@ async def test_reaper_spares_grok_container_within_kill_ttl( )() orch = AgentOrchestrator.__new__(AgentOrchestrator) + monkeypatch.setattr( + orch, "_maybe_recover_broken_gateway", AsyncMock(return_value=False) + ) orch._claim_heartbeat_ttl = 300 orch._grok_idle_kill_ttl = 900 orch._instances = {"be-dev-1": _grok_instance()} @@ -249,6 +275,9 @@ async def test_reaper_never_kills_non_grok_container( claude_cfg = type("C", (), {"provider_type": "anthropic"})() orch = AgentOrchestrator.__new__(AgentOrchestrator) + monkeypatch.setattr( + orch, "_maybe_recover_broken_gateway", AsyncMock(return_value=False) + ) orch._claim_heartbeat_ttl = 300 orch._grok_idle_kill_ttl = 900 orch._instances = { @@ -292,6 +321,9 @@ async def test_reap_spares_live_container_on_registry_miss( )() orch = AgentOrchestrator.__new__(AgentOrchestrator) + monkeypatch.setattr( + orch, "_maybe_recover_broken_gateway", AsyncMock(return_value=False) + ) orch._claim_heartbeat_ttl = 300 orch._grok_idle_kill_ttl = 900 orch._instances = {} # registry lost; container still up @@ -326,6 +358,9 @@ async def test_reap_releases_on_registry_miss_when_container_gone( )() orch = AgentOrchestrator.__new__(AgentOrchestrator) + monkeypatch.setattr( + orch, "_maybe_recover_broken_gateway", AsyncMock(return_value=False) + ) orch._claim_heartbeat_ttl = 300 orch._grok_idle_kill_ttl = 900 orch._instances = {} @@ -342,7 +377,9 @@ async def test_reap_releases_on_registry_miss_when_container_gone( @pytest.mark.asyncio -async def test_registry_uninitialised_skips_docker_fallback() -> None: +async def test_registry_uninitialised_skips_docker_fallback( + monkeypatch: pytest.MonkeyPatch, +) -> None: """With `_instances` never initialised (None — the __new__ unit harness), the Docker fallback is skipped and the stale task reaps as before; no accidental Docker probing where there's no registry to be amnesiac about. @@ -361,6 +398,9 @@ async def test_registry_uninitialised_skips_docker_fallback() -> None: )() orch = AgentOrchestrator.__new__(AgentOrchestrator) + monkeypatch.setattr( + orch, "_maybe_recover_broken_gateway", AsyncMock(return_value=False) + ) orch._claim_heartbeat_ttl = 300 # _instances intentionally NOT set -> getattr yields None -> no fallback. svc = AsyncMock() diff --git a/tests/unit/services/test_metrics_hours_coercion.py b/tests/unit/services/test_metrics_hours_coercion.py new file mode 100644 index 00000000..a6c84779 --- /dev/null +++ b/tests/unit/services/test_metrics_hours_coercion.py @@ -0,0 +1,33 @@ +"""Regression: SQL avg/extract "hours" aggregates must serialize as JSON numbers. + +``EXTRACT(epoch ...)`` returns ``numeric`` on PostgreSQL 14+, which asyncpg +surfaces as a ``Decimal``. A ``Decimal`` serializes to a JSON *string*, so the +panel's ``avg_cycle_hours.toFixed(1)`` (and the other hours fields) threw +``toFixed is not a function`` against the live deploy. ``_as_hours`` coerces to a +real ``float`` so the field is always a JSON number. +""" + +from __future__ import annotations + +from decimal import Decimal + +import pytest +from roboco.services.metrics import _as_hours + + +def test_as_hours_coerces_decimal_to_float() -> None: + result = _as_hours(Decimal("1.21")) + assert result == pytest.approx(1.21) + assert isinstance(result, float) # not Decimal -> serializes as a JSON number + + +def test_as_hours_rounds_to_two_places() -> None: + assert _as_hours(Decimal("1.236")) == pytest.approx(1.24) + assert _as_hours(3.14159) == pytest.approx(3.14) + assert isinstance(_as_hours(3.14159), float) + + +def test_as_hours_none_and_zero_yield_none() -> None: + assert _as_hours(None) is None + assert _as_hours(0) is None + assert _as_hours(Decimal("0")) is None diff --git a/tests/unit/services/test_task_audit_events.py b/tests/unit/services/test_task_audit_events.py new file mode 100644 index 00000000..050068ca --- /dev/null +++ b/tests/unit/services/test_task_audit_events.py @@ -0,0 +1,44 @@ +"""TaskService._audit_events_for — the rejector-attributed audit event selection. + +A transition always emits the generic ``task.``; a reviewer bounce to +needs_revision additionally emits ``task.qa_fail`` / ``task.pr_fail`` keyed on +the acting role, so the per-agent rework scorecard can attribute the rejection. +""" + +from __future__ import annotations + +from roboco.services.task import TaskService + + +def test_generic_transition_emits_only_status_event() -> None: + assert TaskService._audit_events_for("awaiting_qa", "developer") == [ + "task.awaiting_qa" + ] + + +def test_qa_fail_adds_named_event() -> None: + assert TaskService._audit_events_for("needs_revision", "qa") == [ + "task.needs_revision", + "task.qa_fail", + ] + + +def test_pr_fail_adds_named_event() -> None: + assert TaskService._audit_events_for("needs_revision", "pr_reviewer") == [ + "task.needs_revision", + "task.pr_fail", + ] + + +def test_ceo_reject_to_needs_revision_has_no_named_event() -> None: + # A CEO rejection is a needs_revision bounce but not a QA/PR-review fail. + assert TaskService._audit_events_for("needs_revision", "ceo") == [ + "task.needs_revision" + ] + + +def test_named_event_only_on_needs_revision() -> None: + # A reviewer role on a non-needs_revision transition gets no named event. + assert TaskService._audit_events_for("awaiting_pm_review", "pr_reviewer") == [ + "task.awaiting_pm_review" + ] diff --git a/uv.lock b/uv.lock index 13de1052..2288e538 100644 --- a/uv.lock +++ b/uv.lock @@ -2404,7 +2404,7 @@ wheels = [ [[package]] name = "roboco" -version = "0.9.0" +version = "0.10.0" source = { editable = "." } dependencies = [ { name = "alembic" },