Repository navigation
feat(metrics): the objective ↔ metric join — target vs actual with freshness, spelled once; GET /objectives + get_objectives (abilityai/trinity-enterprise#666) - #2953
Merged
Conversation
…er, metric_definitions store, reconcile on create/pull/sync/start, definitions API, compat D-009 (abilityai/trinity-enterprise#477)
…5, make the declared-metrics reader total on non-string keys, register the J12 harness (abilityai/trinity-enterprise#477)
…istry # Conflicts: # src/backend/db/migrations.py
…abilityai/trinity-enterprise#477) `reconcile` copies the whole declaration payload over an existing row, so a re-declaration whose `type` was REFUSED still overwrote `status_values_json` from the refused entry. Re-declaring a `status` metric as `counter` therefore kept the stored type `status` while nulling its declared value domain, leaving a status row that nothing can interpret — and ent#478's point validator would have had to widen to "accept any string" to cope with it. The stored type stands, so the column that exists only for that type stands with it. One line in the refusal branch; `test_a_refused_type_change_keeps_the_ declared_status_values` goes red without it (verified by removing the line). Found by the ent#478 plan review (TD-15); landed here because 478 stacks on this branch.
…(abilityai/trinity-enterprise#478) Checkpoint A of ent#478 — everything below the route. The push path itself (route, settings, sweep, MCP tool) lands in the following commits. **Schema, all four tracks** (Invariant #9): `db/schema.py` DDL + three indexes, `db/tables.py` Core metadata, the SQLite `metric_points_table` migration, and Alembic `0067_metric_points <- 0066_metric_definitions`. No surrogate id: the primary key IS the point identity `(agent_name, ts, idempotency_key)`, which keeps the eventual partition key inside the only unique constraint so ent#80 can partition by month without a table rebuild. `dims` is JSONB on PostgreSQL and TEXT on SQLite through a new per-column marker rule — `dims TEXT /* pg:JSONB */` in the shared DDL, rewritten by `to_postgres_table_ddl`. One rule converges the fresh-PG path (`0001_baseline` replays that DDL), the upgrade path and SQLite, with no `ALTER ... USING` repeated in two places. `value_numeric` is `DOUBLE PRECISION`, not `REAL`: `REAL` is float4 on PostgreSQL and would round a revenue metric's cents. **Store** (`db/metric_points.py`): `insert_points` counts with `.returning` rather than `rowcount`, which is not a portable count of what survived a multi-VALUES `DO NOTHING`. The cap count and the sweep's candidate count are both LIMIT-bounded, and the count and the prune are built from ONE predicate (the ent#433 rule). The prune chunks by `ts` RANGE — there are no ids — taking boundary ties with it so a busy millisecond cannot wedge the loop, and is bounded to 20 chunks per call so a narrowed window drains over several cleanup cycles instead of blocking the others for one very long one (TD-14). **Validator** (`services/metric_points_service.py`): a pure leaf over the ent#477 definitions — no DB, no settings, no HTTP. Points validate against the STORED type, never `type_conflict` (T5), with a hint naming the author-side rename. `ts` must match the RFC 3339 shape and is range-checked as a DATETIME, not a string: `0999-01-01` normalises to `999-01-01T...`, which sorts ABOVE every real timestamp and would head the `ts DESC` read index forever. Below the retention window is refused too, so agent input cannot steer the retention guard into permanent refusal. Messages echo codes, indices and dimension KEYS only — this output lands in an LLM's context and in logs. **Model** (`models.py`): the frozen `MetricPointIn` wire shape ent#479 reads back and #536 binds a canvas chart to. A before-validator rejects `bool` (an `int` subclass Pydantic was measured to coerce to `1.0`) and non-finite floats (SQLite stores NaN as NULL, PostgreSQL as NaN — one batch, two meanings). **Cascade** (`db/agent_cleanup.py`): one CASCADE ref. The identity hash excludes `agent_name`, so a rename re-keys losslessly. Tests: 59 validation/model + 27 store (SQLite; the JSONB column-type assertion is PostgreSQL-marked so it actually runs on the PG leg), including a negative control showing what `test_schema_parity` structurally cannot see and a facade signature-parity check.
…and quota audit (abilityai/trinity-enterprise#478)
Checkpoint B — the push path itself. The MCP tool and the docs follow.
**Route** `POST /api/agents/{name}/metrics/points` in its own module (router →
service → db, Invariant #1), registered before the agents router's
single-segment routes. Gate order is load-bearing: self-gate (access-first, so
the answer cannot vary by whether a metric exists) → rate limit → size →
batch idempotency → definitions → validate → daily cap → insert.
Two idempotency layers. The ROW key is the invariant and needs nothing; the
BATCH key gives Invariant #18's "returns the first result". Where no client key
is supplied but `execution_id` resolves to this agent, the batch key is derived
from the execution — which is what dedups a batch of `ts`-less points on a
re-delivered turn, since those would otherwise take a fresh server-now
timestamp and hash to something new. With neither, a retry is a new
observation, stated rather than papered over with a body hash (a body hash
would drop the legitimate "same numbers an hour later").
Every non-2xx exit past the claim goes through one `_reject` choke point that
releases it first: a 422 that wedged the caller's key would turn one malformed
batch into a day of silently dropped metrics. DB failures are CLASSIFIED —
connectivity is 503 + `Retry-After`, a content error is a non-retryable 500,
because telling an agent to retry a batch the database will reject identically
forever is how a permanent error becomes an infinite loop.
**Settings** (C6): `metrics_retention_days` (365, a retention key with its own
guard site) and `metrics_daily_point_cap` (100000, `0` = unlimited), with a
NEW env tier — `ENV_BACKED_OPS_KEYS` + `config.resolve_ops_default` +
`settings_service.resolve_ops_setting`, opt-in per key so the ~20 keys the
retention endpoint documents as env-less keep their precedence. One semantic:
env is a live fallback, and the #2085 boot seeder SKIPS an env-set key rather
than freezing its value into a row. `get_ops_setting`, the boot log,
`GET /api/settings/retention` and the definitions `policy` all resolve through
the one chain and report which tier answered.
The ent#297 redirect is generalised (E4): the unvalidated catch-all PUT now
refuses EVERY key that has validation, not a hand-maintained subset — the cap
was reachable there, where garbage stores verbatim and then 500s the reader.
The cap reader coerces garbage to the default, never to `0`.
**Sweep** (C7) beside the sibling row sweeps, with two deliberate differences:
`floor=FLOOR_METRIC_POINTS` (100k, one agent-day at the cap), because at the
default 1000 any real ingest rate refuses every cycle and then sits blocked
behind single-use acks; and the ack is consumed only once the REMAINING
backlog is under the floor, since the prune is bounded per call. The counter
is added to all three `CleanupReport` sums including the one gating the WAL
checkpoint. The sweep also joins `retention.py::_ack_sweeps` so a narrowed
window is approvable where the operator looks (TD-12).
**Audit** (C10/TD-10): quota events only — one `METRICS` row per (agent, UTC
day) on the first cap refusal, best-effort. `audit_log` is append-only and
undeletable for a year, so a row per accepted batch would be up to 86k
permanent rows per agent per day at the cap.
**Definitions policy** (C8) now reads live and says `enforced: True`, with a
source per knob.
Deliberate reds, each flipped here: `test_ent477_settings_knobs.py` is REPLACED
by `test_ent478_settings_knobs.py` (every assertion inverts — ent#477 asserted
the knobs must not exist until an enforcer shipped, and recorded "no env tier"
as a tripwire precisely so this change could not happen silently);
`test_ent477_definitions_endpoint.py` flips to `enforced is True`; `test_297`
goes 11 → 12 retention keys and gains the generalised-redirect assertion.
Tests: 572 passed across the ent478 + ent477 suites and the guards this
touches (`test_1771a` set-equality, `test_2085` seeder, `test_297`,
`test_models_centralized`).
…s the contract lives in (abilityai/trinity-enterprise#478) Checkpoint C — the third surface (Invariant #13) and the written contract. **MCP tool** `src/mcp-server/src/tools/metrics.ts`: `record_metrics` plus `refresh_metric_definitions`, the remedy its own `metric_undeclared` hint names — shipped together so the hint points at something the agent reading it can actually call (TD-4). Agent-scoped keys only, with no agent-target parameter to spoof; both policy rows are `kind: "none"` because the backend self-gates the path agent. The tool never throws: a thrown error ends the agent's turn over what is usually a correctable mistake, and the 422 body carries a reason code per point precisely so the agent can fix the batch and re-send. Those per-point errors are surfaced verbatim. The retryable split is the load-bearing part — 503 is a store outage worth retrying, a 500 means the batch itself is unstorable and will fail identically forever, and the two 429s are distinguished because "wait a moment" and "wait until tomorrow" are different remedies. The description teaches the four rules an author can get wrong in a way the platform cannot detect afterwards: declare first, values are not coerced, identity excludes the value (so a correction is a new `ts`), and pass `execution_id` so a re-delivered turn replays. **Requirements**: §47.8 rewritten (the knobs it recorded as deliberately absent are now minted and enforced, and the env tier it recorded as non-existent is built), and a new §48 — the frozen wire shape #536/#479/#538 all defer to, the identity rule, the reason-code list ent#483 will validate parity against, the timestamp rules and the cap semantics. **Architecture**: `database.md` gets the table with the four column decisions that each go against a plausible default (no surrogate id, value outside the hash, the JSONB marker rule, DOUBLE PRECISION); `api-endpoints.md` the route; `backend.md` the router and the pure validator; `mcp-server.md` the new module; `reliability.md`'s counts move 10→11 guard sites and 11→12 retention keys, the env-layer sentence is corrected, and the ack-gated sweep list is now three. **Guide**: the "keep writing metrics.json until then" instruction is replaced by the actual `record_metrics` contract, and the retention/cap paragraph now says both are enforced and where to read the live numbers. Plus the feature flow's write-path section, the journey catalog (J12 gains 478 and stays `built: false` — the promise needs ent#479's read; JOURNEYS.md regenerated, not hand-edited), `tests/registry.json`, and `METRICS_RATE_LIMIT` / `METRICS_RETENTION_DAYS` / `METRICS_DAILY_POINT_CAP` in `.env.example` and both compose files. Tests: 19/19 in `metrics.test.ts`; `tsc --noEmit` clean; the full MCP suite is 477 pass / 1 fail, that failure being a pre-existing `run_agent_loop` gating test (confirmed identical with these changes reverted). Doc guards green (journey catalog, architecture split, testing-docs consolidation).
…retention test stubs, and close the review's small findings (abilityai/trinity-enterprise#478)
…ule, MCP get_metrics (abilityai/trinity-enterprise#479) Checkpoint A — the read half of ent#476's "one read of a business number". `services/metric_read_service.py` is new and holds the platform's SINGLE definition of freshness: `stale ⟺ cadence declared AND now − last_point_at > 2 × cadence`, strict `>`, with a future point clamped to age 0. `freshness()` is pure and importable without `database`, so #2927's role card and ent#666 call it rather than deriving a fourth threshold (a test pins that property). No cadence → `stale: None` / `no_cadence`; no points → `no_points`, not stale. `GET /api/agents/{name}/metrics` keeps its URL and is re-backed by the ent#478 point store, replacing the agent-server `metrics.json` proxy (deleted with its `__init__` export; zero importers). The read is store-only, so a STOPPED agent answers exactly like a running one — the proxy used to say "Agent must be running to read metrics", which made every number disappear at the moment an operator most wanted to see it. Contract: uniform-404 dependency then the agent self-gate (an agent key reads only its own numbers); `window ∈ auto/24h/7d/30d/90d` plus `since`/`until`, `auto` sized to the declared cadence; dimensioned metrics come back as one series per dims tuple with `latest` folded by the declared `aggregation` (TD-2); the default read ships ≤120 buckets per metric and raw points only on the single-metric path, newest-first (`ASC + LIMIT` would have kept last week and dropped today). Named refusals: 422 `window_invalid`, 422 `metric_undeclared` (never 404 — the MCP classifier reads 404 as not-authorized), a retired name carrying the `include_retired` hint (TD-10), 503 `metric_store_unavailable`. Read rate limit 240/min per agent (TD-6). D-010's persisted finding is echoed with `findings_evaluated_at`, so "no finding" can be told from "not evaluated" (TD-1). Store: `latest_points_for` seeks per declared name with an `(ts DESC, idempotency_key DESC)` tiebreak — a `row_number()` partition would number every row the agent ever recorded, and without the tiebreak SQLite and PostgreSQL may disagree about which point is "latest". `get_agent_health` gains an INFORMATIONAL `metrics` block, attached in the router after both build paths (never inside the fleet loop) and wrapped so a store failure yields `null` rather than a failed health check. It never touches `aggregate_status` or `issues`. MCP `get_metrics` (agent-scoped, no target parameter), `client.getAgentMetrics`, the `access.ts` policy row, and the health block passed through `monitoring.ts`. Tests: 45 backend unit tests (freshness table + boundaries, read store on `db_backend`, route gates/refusals/shape/D-010 echo, health informational-only) and 9 new MCP node:test cases. 603 passed across `test_ent47{7,8,9}_*` + `test_compatibility_checks.py`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…to declared metrics (abilityai/trinity-enterprise#479) Checkpoint B (backend half) + the compat half of checkpoint C. **D-010** (SOFT, static) names `metrics.json` as superseded rather than the read silently omitting what it holds. Serving the file when the store is empty was rejected deliberately — two sources for one number is what ent#476 exists to prevent — so the file becomes a finding instead. The collector now reads it WITH content (small flat JSON, existing size cap) so the detail can name which keys have no `template.yaml metrics:` entry: "you still write this file" is advice, "these numbers are declared nowhere" is a fix. Keys only, charset bounded, 25 max — `checks_json` is persisted and rendered. The route committed in the previous checkpoint already echoes this finding with `findings_evaluated_at`. Catalog 90 → 91; `docs/agent-validation-spec.md` gains its row and section. **The `metric:` binding**: a `dashboard.yaml` metric/status/progress widget may carry `metric: <name>` and is filled from the registry on every read — `value`, `color` (from the declared status `values[].color`, or thresholds for numeric types), `history` from the bucketed series, `stale`, `last_point_at`. Order is cache → snapshot → bind → history-enrich, on BOTH the live and the cached path, and the snapshot writer and the history enrichment both SKIP bound widgets through one shared `is_bound` predicate: a bound number in `agent_dashboard_values` would be a second source for a value the registry owns, and the two would disagree the moment the poll and the recording cadence drift apart. An undeclared name yields `binding_error` and NO value (a wrong number is worse than no number); a store outage degrades per widget and never 5xxs a dashboard. **The agent-server validator** (Invariant #5) no longer requires `value` — or `color` on a status widget — when `metric:` is set, so an author stops having to write a fake number forever. Older images keep the strict rule, so the guide will tell authors to keep a placeholder `value:`; the backend overwrites it when the binding resolves, and a test pins that. **`GET /api/agent-dashboard/{name}/exists`** now answers `{has_dashboard, has_declared_metrics}` in one DB-only request, so the tab can appear for an agent with declared metrics and no `dashboard.yaml` — and it is gated with the uniform-404 dependency, which it was not before: a bare `get_current_user` made it a fleet-wide existence oracle for any logged-in principal (TD-9 / E-S2). Tests: 23 new cases (D-010 shapes incl. both legacy file spellings and the charset bound; binding value/placeholder-overwrite/undeclared/outage/status colour/threshold colour; the two skips; the validator relaxation exercised against the REAL agent-image module). 624 passed across `test_ent47{7,8,9}_*` + `test_compatibility_checks.py`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…dashboard probe in the agent view (abilityai/trinity-enterprise#479) Checkpoint B, frontend half — the surfaces that read what ent#477/#478 record. **`DeclaredMetricsTiles.vue`** is a SIBLING of `DashboardPanel`, not an arm inside it. The panel's state machine ends in "Agent Not Running" and "No Dashboard Defined"; the tiles are store-backed and must render for a stopped agent with no `dashboard.yaml` at all, which is precisely ent#439's sensible default and precisely the agent the panel refuses to render anything for. Own fetch, own interval, own `viewState`, so a metrics outage cannot take the dashboard down or vice versa. Every tile states WHEN its point was recorded — relative on the face, absolute on hover — because a number with no time is a claim about now and for a recorded metric that is usually false. A stale metric keeps showing its last known value with a warning chip naming what it was late against; it is never rendered as current. A metric with no declared cadence gets a neutral "No cadence declared" chip and is never called stale. The declared-but-empty copy is the backend's, so the tile and the route cannot name different next actions (`/update-dashboard` when the agent has that playbook, `record_metrics` when it does not). **The stale rule is not implemented here.** `stale` / `freshness` / `stale_after` arrive decided by `metric_read_service.freshness`; a browser-side recomputation would be a second rule the first time either was edited. **Two flags, one probe.** `GET …/exists` now answers `has_dashboard` AND `has_declared_metrics`, and `buildTabs` shows the Dashboard tab for EITHER. The probe lost its running-only condition at all four call sites: both halves are DB-only, and a stopped agent's recorded numbers are exactly what an operator wants. `buildTabs` moved to `utils/agentTabs.js` — it was tested by brace-matching its TEXT out of the SFC and `new Function`-ing it, which is a real execution but only existed because a spec could not import an SFC (#2918). **Two defects found while wiring it up, both in the previous checkpoint:** 1. `GET /api/agent-dashboard/{name}/exists` answered **422 to every caller**, owner included. `AuthorizedAgentByName` declares its path parameter as `agent_name`, so a route spelling it `{name}` left the dependency with nothing to bind. The probe was dead — the frontend's `catch` swallowed it into the boot ladder, so the tab still appeared for a running agent with a dashboard and `has_declared_metrics` was simply never true. The URL is unchanged; only the binding name moved. `test_ent479_exists_gate.py` went red on three of four cases before the fix and is the gate test the rider asked for: an absent and an inaccessible agent are byte-identical 404s (#186), and that cannot be proved by the sibling route suites, which override the dependency away to reach their handlers' own gates. 2. `bind_dashboard_widgets` wrote `widget["history"]` as a bare LIST while `_enrich_widgets_with_history` writes `{values, trend, …}` and `DashboardPanel.vue` reads `history.values`. A bound widget would silently lose its sparkline and trend arrow with every backend assertion green. One consumer, one shape, now pinned. `BoundMetricMark.vue` carries the bound widget's chip, point time, stale mark and binding error for all three arms that take a binding — its own component because a 735-line panel is already an #1031 candidate. A binding error renders the REASON, never a number: the backend removes `value` for an undeclared name, and a bare dash tells an operator nothing. `utils/metricFormat.js` lifts the panel's formatters so the two surfaces cannot drift, and makes them type-aware (5400 is `1h 30m`, not `5,400`) and direction-aware (rising revenue is good, rising error rate is not; a `neutral` metric gets no verdict at all). Tests: 4 new frontend specs, two of them MOUNTED under jsdom (the stale mark, the empty copy, in-place refresh keeping DOM identity and scrollTop, a failed refresh keeping the data, the three bound arms, the binding-error copy) plus the pure formatter and tab-gate suites. 3405 passed across the whole frontend unit suite on three consecutive runs, ratchets included; `check:tokens` clean; `npm run build` clean. Backend: 24 + 4 in the ent#479 binding and gate suites. Mutation-checked — inverting the tab gate and the freshness chip turns 8 of these red. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… guide, J12 gains 479, bundled templates call record_metrics (abilityai/trinity-enterprise#479) Checkpoint C — writing down what ent#479 decided, and stopping the repo's own templates from teaching the retired path. **Requirements §49** is the new section: the one staleness rule as a four-row table with the two boundaries a reader guesses wrong (exactly 2× is not stale; a future point clamps to fresh, and `no_cadence` is `null` rather than `false` because "not stale" and "unanswerable" are different claims); the route with its gate order, window contract and named errors; why the default read is bucketed; why `metric=` on a retired name is a 422 and not a 200; the health block being informational and never a verdict; the tiles as the default dashboard; the widget binding and the one `is_bound` skip that stops `agent_dashboard_values` becoming a second source. **§47.9 is rewritten**: the `metrics.json` read is gone, the file is not a fallback when the store is empty — deliberately, because "serve the file when we have nothing better" is exactly how two sources drift apart — and D-010 names it instead. **The guide** stops telling agents to write `metrics.json`. The old "metrics.json Format" section becomes a four-step migration; the complete example records with `record_metrics`; the CLAUDE.md snippet does too. It gains a "reading your own metrics back" section whose point is that declaring a `cadence:` is what buys you a staleness answer at all, and the **placeholder rule** (TD-3): an agent on a base image built before this issue still enforces the strict validator, so keep `value: 0` alongside `metric:` until it is rebuilt — the backend overwrites it whenever the binding resolves, so the placeholder is never what an operator sees. The Agent Dashboard section now opens by saying you may not need a `dashboard.yaml` at all. **Both feature flows** are rewritten around the completed loop rather than the legacy proxy: `agent-custom-metrics.md` replaces its metrics.json/MetricsPanel diagram and dead file table with the declare → record → read flow, the read contract and the retirement; `agent-dashboard.md` documents the binding, its three per-widget failure modes, the deprecation of snapshotting an author-written `value:` for a business number, and the two-flag gated `/exists` probe — including the path-parameter trap that made it 422 for everyone. **Architecture rows** for `metric_read_service` (the one stale rule, the call-time `db` resolution that keeps `freshness` importable, the `history` shape the panel reads, `is_bound`), the read endpoint, the third `metrics.ts` tool, the tiles/`metricFormat`/`agentTabs` frontend seam, the gated `/exists`, and a database note that the read adds no column and no index — the existing `(agent_name, metric, ts DESC)` btree already prefixes every access, and `0067_metric_points` stays head. **J12** carries `[477, 478, 479]` and stays `built: false`: the promise is whole in code, what is missing is the walk. The harness docstring gains the three steps that make it a journey rather than a registry test — record and read back with a real freshness verdict, read again with the agent STOPPED and get the same answer, and find D-010 rather than a served `metrics.json`. **TD-7** — the four in-repo templates that still instructed agents to write `metrics.json` (`demo-analyst`, `demo-researcher`, `dd-intake` and the system agent's `/update-dashboard` playbook) now call `record_metrics`. Text only, no playbook redesign — that is ent#482. Without this every demo and the system agent would show "declared, no points yet" on a fresh install. `tests/registry.json` gains all six ent#479 backend suites (none were registered by the earlier checkpoints) and the MCP row picks up `get_metrics`. 290 passed: `test_ent479_*` + `test_2338_journey_catalog` + `test_compatibility_checks`. 94 more across the testing-docs, model-placement, enumeration-uniformity and admin-gate guards. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…mpty-state copy, retire the agent-server metrics.json route, readable D-010 detail, refuse retired bindings, fold charts across series (abilityai/trinity-enterprise#479)
I1: `_bucket` is fed the newest N points, not the window, so a point older
than `since` got a negative index and was CLAMPED into bucket 0 — the read
opened with a fabricated spike (the fold of every excluded point) stamped
before `since`, and `stats`/`sparklineMax`/a bound widget's `history` were
computed from it. Filter to `[since, until]` before indexing; keep the
`SERIES_BUCKETS - 1` clamp only so `ts == until` lands in the last bucket.
Each bucket now carries its index `i`, which is what makes cross-series
folding well defined.
I2: `has_playbook` was reachable only by its default, so the ruled
"schedule `/update-dashboard`" copy was unrenderable. The route cannot
compute it store-only (the playbook catalog is a container probe, and
`agent_skills` knows only library assignments while the bundled templates
carry the playbook in `.claude/commands/`), so the parameter is dropped and
one sentence names both actions. A signature pin keeps a dead flag from
coming back.
I3: the agent-server `GET /api/metrics` no longer reads `metrics.json` (or
`template.yaml`). Route kept per 49.7, answering 410 with
`{has_metrics: false, superseded_by, finding: "D-010"}`.
I4: D-010's `detail` is `{keys, undeclared}`; the tile spelled it through
`toDisplayString` as JSON braces and the spec pinned a string the backend
never emits. Rendered as a sentence, fixture corrected.
I5: a widget bound to a RETIRED metric now refuses with
`binding_error_code: metric_retired` + `retired_at` and no value — TD-10's
rule applied to a reader that cannot pass `include_retired`. Every refusal
gains a machine code beside its sentence.
I6: `chart` is one bucket list shared by the sparkline, `stats` and a bound
widget's `history`, so they describe the same thing as `latest.value`: the
cross-series fold for `sum`/`avg`, and for `last` the single series named by
`chart.dims`, which the tile labels.
Plus the bucket-vs-window clamp class in learnings.md.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…9 body (abilityai/trinity-enterprise#479) The read is store-only now: `status` / `has_metrics` / `values` were the metrics.json proxy's body (retired by D-010), so the suite asserted a shape the route no longer emits. It now asserts the ent#479 fields — `declared`, `window`, `stale_rule`, `findings`, per-metric `latest`/`freshness`/`stale` — pins the retired fields as ABSENT, and drops the 'stopped agents have no metrics' expectation, since a stopped agent now answers exactly like a running one.
…spelled once (abilityai/trinity-enterprise#666) An objective (Tandem §3.4) names a metric by name; §47 says what that name means; §48 says what it currently reads; §49's `freshness()` says whether to believe it. `services/objective_join_service.py` is the join of those four, and it is the ONE join — the role card (ent#527), the project hub (ent#661) and proactivity (ent#605) consume it rather than each growing their own, which is the defect epic ent#476 exists to remove. Two halves in one module. The pure half imports at module top with no store or Docker behind it: `canon_root`, `parse_objective`, `objective_concerns`, `objective_is_active`, `select_objectives`, `resolve_direction`, `gap`, `join_objectives`. The async half goes through the agent door — files are truth and they live in the container (E7/E13), so a stopped agent answers `unavailable: agent_stopped` with copy naming the fix rather than a number that was true once. `actual` is the tile's folded latest, not a second computation (TD-1). `_latest_entry` is extracted from `_compose_metric` and both it and the new `latest_by_metric` go through it, so "the objective's actual" and "the number on the tile" are the same fact by construction — pinned by a parity test including a dimensioned `sum` metric, where a re-implemented fold would plausibly diverge. `latest_by_metric` does no series, bucket or stats work and takes `definitions=` / `names=`, so the join pays one registry read and one point read for the names an objective actually references, and none at all when there is nothing to join. Direction falls through to the objective file when the registry has no opinion (TD-2): `neutral` is the column DEFAULT and no bundled template declares `direction:`, so registry-only would have shipped a feature where every gap is uncomputable on day one. Two DECLARED directions that disagree is a `direction_mismatch` finding and the registry wins. `hold` is the grammar's third direction, mapped to the registry's `neutral` but told apart from silence by `direction_source` — and it has its own gap arm, `on_target` within `tolerance` and `off_target` otherwise, never behind/ahead, because a value that should be held has no good side to be on. `gap.status` is POSITION relative to target given direction, never pace (TD-3). `by` and `horizon` ride every row so ent#605 can judge pace itself; nothing here computes one. `stale` is orthogonal: a stale metric still gets its gap, with `stale: true` beside it — the card renders "30 / 35 · stale" and the consumer decides. Never a blank. An objective naming an undeclared metric is a `metric_undeclared` finding with the fix attached; a SUPPORTING agent gets `metric_not_declared_here` instead and is not counted as undeclared, because "declare it and refresh" is advice it cannot take (the registry is per-agent; cross-agent reads are ent#80). A retired metric withholds its last value and says why. A file that will not parse is `objective_invalid` with its path, not a silent `continue`. Hardening the fan-out costs less than recovering from it: objective reads run ≤ 2 in flight and abort to `agent_unreachable` on the first typed transport failure, so one dead agent-server cannot drive open the circuit breaker that chat rides on. The scan lists up to 100 files, filters, and caps the OUTPUT at 20 — capping the listing first hides an agent's own objective behind twenty foreign ones in a shared fleet canon. Author values are normalised at parse: `.nan` / `.inf` / lists / bools become `target_text`, so no bare `NaN` reaches a body that `JSON.parse` would throw on. Tests: 107 unit cases across the gap table (nine positions × three directions, the `hold` arm, tolerance, NaN/inf/list/bool targets), the join (undeclared, not-declared-here, retired, stale, no-points, no-cadence, mismatch, duplicates, summary arithmetic), the composition (three unavailable states, zero-config with no store call, scan bounds, concurrency bound, fan-out abort, traversal refusal) and the parity + wiring guards.
…ityai/trinity-enterprise#666)
The two doors onto the join, plus the docs that make it a contract rather than
an implementation detail.
`GET /api/agents/{name}/objectives` copies `/metrics`'s gate order verbatim —
uniform-404 dependency, then the agent self-gate (403; an agent key reads only
its own, cross-agent is ent#80's grant), then the limiter on the name the gate
has already validated. It gets its OWN rate knob, `OBJECTIVES_READ_RATE_LIMIT`
at 60/min rather than the 240 copied from a store-only read: this route drives
the container — a listing plus up to 100 small file reads through the agent
door — and ten open role cards at a 30 s poll is 20/min, so 60 clears normal
traffic and still stops a loop from pinning an agent-server the platform also
needs for chat.
A store outage is the only non-2xx below the gates (503 + `Retry-After: 30`).
Everything else is a named field on a 200, because "this agent is stopped" is
an answer, not an error.
The model is the contract, structurally. `ObjectiveJoinRead` is the
`response_model` and the service's dict is returned unchanged; a key-parity
test walks that dict against every model's fields recursively, so an additive
service key fails the build instead of being silently filtered out of every
response — the 2026-07-27 `response_model`-is-an-allowlist trap closed by a
guard rather than by remembering. Author-shaped targets (`.nan`, `.inf`,
lists, bools, a 4 KB string) are pushed through the real route and parsed with
`parse_constant` armed, since a bare `NaN` on the wire is precisely what
blanks the card the join exists to fill.
MCP `get_objectives` (Invariant #13) is `get_metrics`'s shape again:
agent-scoped via `getAgentName`, no agent-target parameter to spoof,
`kind: "none"` in the access policy because the backend self-gates the path
agent, body returned verbatim, never throws. Its description carries the three
things an agent will otherwise invent — that `gap.status` is position and not
pace, that `stale: true` means record a fresh point rather than report the
gap, and that an undeclared metric arrives as a named finding with the remedy
(or `metric_not_declared_here`, where there is nothing for the caller to fix).
It says outright not to re-derive a gap from `get_metrics` plus an objective
file, because two answers to one question is the defect ent#476 removes.
Docs: requirements §50 (the §3.4 grammar including `hold` and the `active`
filter, the four sources and which owns what, the gap semantics table, the
direction fallback matrix, the findings catalog with the fix each names, the
gate order, the response shape, the bounds and why each is where it is, the
zero-config path, the consumer list with the no-second-join rule, and the two
known costs — the 60-file fleet canon and the two doors onto template.yaml);
the feature flow gains an Objectives section with the dataflow and the PR
#2927 rebase note (what `role_card.py` drops, the slim portal projection per
TD-4, the limiter the portal route gains, and both Alembic landing orders);
architecture rows in backend / api-endpoints / mcp-server; a guide section
telling an agent that an objective names its metric BY NAME and what that
implies; J12 gains 666.
…for hold, fan-out budget, skipped-file and invalid-id findings, executable rebase note (abilityai/trinity-enterprise#666)
…istry # Conflicts: # src/backend/db/migrations.py
…0068, isolate the start-hook tests' task set, drive the Alembic PG test on TEST_POSTGRES_URL (abilityai/trinity-enterprise#477) Mechanical fixes applied on the member branch, per the merge-train note on #2942: - 0066_metric_definitions → 0069_metric_definitions on top of 0068_agent_shared_files_audience. dev landed 0066_public_user_memory_writes, 0067_agent_role_readiness and 0068_agent_shared_files_audience after this branch's last re-chain, so it was a second #2068 fork (alembic-head-watch was red). check_alembic_heads.py: 70 revisions, 1 head. - test_ent477_git_refresh_hook: the two start-hook tests gather metric_registry._inflight_refresh_tasks, a module-level set that other tests leave stale tasks in (spawned on a loop that closed before the done-callback fired). Under CI's random ordering asyncio.gather then raised 'The future belongs to a different loop'. Each test now monkeypatches a fresh set. - test_the_alembic_revision_builds_the_table_on_postgres called upgrade_to_head() on the DATABASE_URL default (SQLite), where 0004's 'ADD COLUMN IF NOT EXISTS' cannot parse; schema-parity was red on every PR in the stack that touches db/. It now points DATABASE_URL at TEST_POSTGRES_URL, resets the schema, upgrades to head and reads back. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…o feature/478-record-metrics # Conflicts: # src/backend/services/cleanup_service.py # tests/registry.json
… feature/479-metrics-read # Conflicts: # docs/memory/learnings.md
…eature/666-objective-join
…er on git.py (abilityai/trinity-enterprise#477) Mechanical, per the /review findings recorded on #2942: - SOURCES was a constant nobody read. _reconcile now refuses a source outside it (ValueError) before anything reaches the store, and a test drives the refusal. - definition_hash hashed the entry INCLUDING the definition_hash key that _reconcile writes into it, so hashing one list twice drifted and would have reported every metric as updated. The hash now excludes its own key; a test pins the stability. - routers/git.py gains its first-line '# mcp:' header (Invariant #13) — this PR touches three of its handlers and the file had none. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…dim/skew limits (abilityai/trinity-enterprise#478) Mechanical, per the /review findings recorded on #2948: - db/metric_points.py: metric_points_remaining and build_point_row had no caller in src/ or tests/ (the sweep calls count_metric_points_candidates directly; the service builds its own row dict). - services/metric_points_service.py: MAX_DIM_VALUE_LEN and TS_FUTURE_SKEW_SECONDS duplicated METRIC_DIM_VALUE_MAX_LEN and METRIC_TS_FUTURE_SKEW_SECONDS in models.py; the service now reads the models.py values, so the wire contract has one spelling. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…le (abilityai/trinity-enterprise#479) Mechanical, per the /validate-pr config-packaging check (§4.9) recorded on #2951: the knob was read via os.getenv in routers/agent_files.py but declared in neither compose file nor .env.example, so the tuning lever was inert on every deployment. Default unchanged (240/min). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…rs, hosted compose mirrors prod's METRICS block, lint-clean sys.modules poisoning (abilityai/trinity-enterprise#478) Mechanical, surfaced by the full unit suite on the merged stack (none of these ran in CI — the stacked base never triggers backend-unit-test): - tests/unit/test_cleanup_inner_sweeps.py: _configure_db never set count_metric_points_candidates / prune_metric_points, so the ent#478 sweep stored a MagicMock in report.metric_points_pruned and the WAL-checkpoint sum raised "'>' not supported between MagicMock and int". - docker-compose.hosted.yml: test_2280_hosted_compose_parity requires the backend environment to match prod; the three METRICS_* knobs were added to prod only. - tests/unit/test_ent478_point_validation.py: bare sys.modules pop/assign/del → monkeypatch.setitem + undo (tests/lint_sys_modules.py). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…n vocabulary, snapshot skip proven on a live engine, hosted compose + lint + three nits (abilityai/trinity-enterprise#479) Per the /review findings recorded on #2951: - metric_read_service._threshold_color judged direction as 'up' / 'down', which the registry never emits (DIRECTIONS = up_good / down_good / neutral). Every declared metric therefore fell into the 'up' arm, so an up_good revenue widget turned red for EXCEEDING its critical threshold and a neutral metric was judged at all — while the tile beside it, via utils/metricFormat.js::thresholdClasses, did the opposite. The backend now spells the tile's rule: down_good breaches at-or-above, up_good at-or-below, neutral declines. The pinning test used direction='up' and was green for the wrong reason; three tests now drive both arms and neutral. - The snapshot-writer skip for a bound widget was pinned by inspect.getsource; it is now executed through the real engine (capture_dashboard_snapshot writes the plain widget and not the bound one). - docker-compose.hosted.yml mirrors prod's METRICS_READ_RATE_LIMIT line (test_2280_hosted_compose_parity). - test_ent479_freshness_rule: bare sys.modules.pop → monkeypatch.delitem. - Nits: unused Optional import in agent_server/routers/info.py, the refreshMetricDefinitions JSDoc in client.ts sat above getAgentMetrics, routers/monitoring.py gains its '# mcp:' header (Invariant #13). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… turn the #2927 rebase note into the ent#676 follow-up note (abilityai/trinity-enterprise#666) Mechanical, per the /validate-pr findings recorded on #2953: - OBJECTIVES_READ_RATE_LIMIT was read via os.getenv in routers/agent_files.py and declared nowhere (both compose files, the hosted mirror, .env.example) — §4.9. Default unchanged (60/min). - docs/testing/JOURNEYS.md regenerated from the merged catalog (test_2338_journey_catalog was red on the merged stack). - The feature flow's 'rebase note for PR #2927' assumed ent#666 would land first. #2927 landed first, so the role-card cut-over (drop its own join and 30-day metrics stale rule, slim portal projection, shared limiter key) is a follow-up on dev — filed as trinity-enterprise#676 — and the two Alembic orders it prescribed are obsolete: the train re-chained the metrics revisions as 0069/0070 behind dev's 0066–0068. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Contributor
Author
|
merge-train — pushed to
Left for the author (needs intent): the Validation of the stack as a whole: full backend unit suite (17.7k tests), frontend vitest (155 files / 3436 tests) and the |
vybe
marked this pull request as ready for review
September 22, 2026 14:50
added 3 commits
September 22, 2026 16:02
…s byte-identical to the 477 head this branch already carries
…s byte-identical to the 478 head this branch already carries
added 2 commits
September 22, 2026 16:16
…s byte-identical to the 479 head this branch already carries
trinity-ability
approved these changes
Sep 22, 2026
trinity-ability
left a comment
trinity-ability
left a comment
Contributor
There was a problem hiding this comment.
merge-train: last member of the ent#477→478→479→666 stack — /validate-pr + /review; full unit suite (17703), frontend vitest (3436), MCP tsc + tests and the requires_postgres tier green on this exact tip; role-card cut-over tracked as trinity-enterprise#676.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes abilityai/trinity-enterprise#666 — sub-issue 4 of epic abilityai/trinity-enterprise#476. Stacked on #2951 (ent#479) → #2948 → #2942 — base is
feature/479-metrics-read; the stack merges together (operator, 2026-09-22). Draft until the chain is in.What
One shared read of target vs actual per objective the agent owns or supports, so the role card (ent#527 / #2927), the project hub (ent#661) and the proactivity trigger (ent#605) never re-derive it.
services/objective_join_service.py— a pure half (canon §3.4 grammar helpers,gap(),join_objectives()) and an async half that readstemplate.yaml+<canon>/objectives/*.yamlthrough the agent door (own rate knobOBJECTIVES_READ_RATE_LIMIT60/min, ≤ 2 reads in flight, abort toagent_unreachableon the first transport failure, scan ≤ 100 files → filter by role → cap with counts stated).actual= the tile's folded latest vialatest_by_metric, extracted from the ent#479 read and sharing_latest_entrywith it (parity-tested);stale/freshnesscome from ent#479'sfreshness(), imported, not re-derived.GET /api/agents/{name}/objectives— uniform-404 → agent self-gate → rate limit; per objective: metric, target,by,horizon,actual,last_point_at,stale,direction+direction_source,gap{status, delta, reason}. Every failure is a named finding (metric_undeclared,metric_retired, unreadable/invalid file, path traversal, duplicate ids,direction_mismatch); a stopped or unreachable agent answersunavailablewith the role card's existing copy. Metrics with no objective are absent, not flagged.get_objectives— agent-scoped, never throws.role_card.pydeletions, slim portal projection, portal limiter), architecture rows, guide section, J12 gains 666, registry.No schema change —
0067_metric_pointsstays the single head. No frontend.Rulings carried (orchestrator, on the operator's behalf — plan file)
TD-1
actual= folded latest · TD-2 direction: registry first, fallback to the objective file'sdirectionwithdirection_source: objective, two declared directions that disagree →direction_mismatch(registry wins) · TD-3behind / on_target / ahead= position, never pace (by+horizonon the row for ent#605 to judge pace) · TD-4 slim portal projection lives in #2927 · TD-5 no single-flight. Canon grammar verified against the Tandem framework (objectives/<id>.yaml):owner: role:<id>,supporting_agents,metrics[{name, direction: up|down|hold, target, by}],horizon,status—holdmaps toneutralwith its own gap arm (on_targetwithin optionaltolerance, elseoff_target, never behind/ahead); onlyactiveobjectives are joined (missing → active, out-of-enum → not active). Supporting-agent objectives whose metric is declared by the owner's agent → informationalmetric_not_declared_here, not an error.Review + security
/review(Fable, report-only) against the stacked base: MERGEABLE AFTER FIXES, no criticals — all rider checks hold: one join,actualvialatest_by_metricsharing_latest_entrywith the 479 tile (parity-tested),freshness()imported not re-derived,holdnever says behind/ahead, supporting-agent objectives →metric_not_declared_here, fan-out bounded (2 in flight, 60/min Redis-backed knob, abort on first typed transport failure), route order uniform-404 → self-gate → limiter, MCP tool agent-scoped with no name parameter and never throws, no schema change, single Alembic head. Fixed in2d02e9b8: I1 parse findings from non-active and foreign objectives leaked onto this agent's read (contradicting §50) + the composed test; I4 the #2927 rebase note now carries both Alembic orders inline; I5holdemitsdirection: neutral+direction_source: objectiveper the ruling (the gap arm keeps the hold semantics); I2 filenames outside the safe pattern are counted and reported; I3 30 s fan-out budget + 5 s per read; I7 dead parameters dropped; I8 an invalididis a named finding./cso --diff: nothing at or above the daily gate; no new privilege reach.Tests
tests/unit/test_ent666_{objective_gap,objective_join,objectives_route}.py+ parity cases intest_ent479_metrics_route.py; with the 479 set + compat: 382 passed; MCPmetrics.test.ts34 pass;tsc --noEmitclean; root-placement lint OK. Not run here: the full island (per-PR CI); no live-stack/objectivestest (plan gap, recorded); J12 staysbuilt: falseuntil a container with canon files exists.Before merge
_agent_getreadsresponse.textbefore the 256 KB YAML cap; the 5 s per-read timeout bounds it partly, aContent-Lengthcheck would close it — a natural companion for the feat(workspace): the role card in Agent details — role, objectives with metric freshness, readiness (abilityai/trinity-enterprise#527) #2927 rebase.Handoffs
#2927 (rebase per the feature-flow note; the join moves out of
role_card.py) · ent#605 (readsgap+by+horizon) · ent#661 (project hub reads this route) · ent#482 / ent#483 (agent + validator halves).🤖 Generated with Claude Code