feat(metrics): declared-metric registry — template.yaml metrics: reader, store, reconcile, definitions API, compat D-009 (abilityai/trinity-enterprise#477) - #2942
Merged
Conversation
added 3 commits
September 21, 2026 13:33
…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
This was referenced Sep 22, 2026
…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.
vybe
marked this pull request as draft
September 22, 2026 11:00
|
Regression details (head_sha: `a94a2bd25dc9876626c9c2eed5c8ab5499ca2db0`)Seed Backend unit-suite regression diffPer-XML totals
❌ New failures introduced by HEAD (2)Tests failing under HEAD that did not fail under BASE in any seed:
🟢 Failures fixed by HEAD (1)
Legend: [F] = assertion failure, [E] = collection or fixture error. Reproduce locally: |
|
|
…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>
…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>
Contributor
Author
|
merge-train — pushed to
Left for the author (needs intent, not blocking): a Docker-daemon fault surfaces as |
vybe
marked this pull request as ready for review
September 22, 2026 14:50
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: validated as the base of the ent#477→478→479→666 stack — /validate-pr + /review, full unit suite (17703 passed), frontend vitest (3436), requires_postgres tier on real postgres:16 all green on the merged tip; Alembic re-chained to 0069 on top of dev's 0068.
vybe
pushed a commit
that referenced
this pull request
Sep 22, 2026
…s byte-identical to the 477 head this branch already carries
vybe
added a commit
that referenced
this pull request
Sep 22, 2026
…d idempotent write path, retention knobs + sweep, MCP tool (Abilityai/trinity-enterprise#478) (#2948) * feat(metrics): declared-metric registry — template.yaml metrics: reader, metric_definitions store, reconcile on create/pull/sync/start, definitions API, compat D-009 (Abilityai/trinity-enterprise#477) * fix(metrics): re-chain the registry migration to 0066 after dev's 0065, make the declared-metrics reader total on non-string keys, register the J12 harness (Abilityai/trinity-enterprise#477) * fix(metrics): a refused metric type change keeps status_values_json (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. * feat(metrics): metric_points schema, store, validator and wire model (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. * feat(metrics): record_metrics route, Settings knobs, retention sweep 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`). * feat(metrics): record_metrics MCP tool, requirements §48, and the docs 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). * fix(metrics): bind the batch idempotency key to the body, repair the retention test stubs, and close the review's small findings (Abilityai/trinity-enterprise#478) * merge-train: re-chain the registry revision to 0069 after dev's 0066–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> * merge-train: make SOURCES live, hash-stable definition_hash, mcp header 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> * merge-train: drop two unreferenced store helpers, one source for the 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> * merge-train: the cleanup suite's mock learns the metric-point accessors, 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> --------- Co-authored-by: Trinity Agent (trinity) <trinity-agent@ability.ai> Co-authored-by: sim <sim@example.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
vybe
added a commit
that referenced
this pull request
Sep 22, 2026
…, one stale rule, get_metrics, declared-metric tiles, metric: widget binding, D-010 (Abilityai/trinity-enterprise#479) (#2951) * feat(metrics): declared-metric registry — template.yaml metrics: reader, metric_definitions store, reconcile on create/pull/sync/start, definitions API, compat D-009 (Abilityai/trinity-enterprise#477) * fix(metrics): re-chain the registry migration to 0066 after dev's 0065, make the declared-metrics reader total on non-string keys, register the J12 harness (Abilityai/trinity-enterprise#477) * fix(metrics): a refused metric type change keeps status_values_json (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. * feat(metrics): metric_points schema, store, validator and wire model (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. * feat(metrics): record_metrics route, Settings knobs, retention sweep 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`). * feat(metrics): record_metrics MCP tool, requirements §48, and the docs 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). * fix(metrics): bind the batch idempotency key to the body, repair the retention test stubs, and close the review's small findings (Abilityai/trinity-enterprise#478) * feat(metrics): re-back GET /metrics with the point store, one stale rule, 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> * feat(metrics): retire metrics.json via D-010, bind dashboard widgets 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> * feat(metrics): declared-metric tiles, bound widgets and the two-flag 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> * docs(metrics): §49 the read contract, D-010 + placeholder rule in the 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> * fix(metrics): drop out-of-window points before bucketing, reachable empty-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> * test(metrics): align the live-tier agent-metrics test with the ent#479 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. * merge-train: re-chain the registry revision to 0069 after dev's 0066–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> * merge-train: make SOURCES live, hash-stable definition_hash, mcp header 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> * merge-train: drop two unreferenced store helpers, one source for the 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> * merge-train: wire METRICS_READ_RATE_LIMIT into compose and .env.example (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> * merge-train: the cleanup suite's mock learns the metric-point accessors, 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> * merge-train: the bound-widget threshold reads the registry's direction 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> --------- Co-authored-by: Trinity Agent (trinity) <trinity-agent@ability.ai> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: sim <sim@example.com>
vybe
added a commit
that referenced
this pull request
Sep 22, 2026
…eshness, spelled once; GET /objectives + get_objectives (Abilityai/trinity-enterprise#666) (#2953) * feat(metrics): declared-metric registry — template.yaml metrics: reader, metric_definitions store, reconcile on create/pull/sync/start, definitions API, compat D-009 (Abilityai/trinity-enterprise#477) * fix(metrics): re-chain the registry migration to 0066 after dev's 0065, make the declared-metrics reader total on non-string keys, register the J12 harness (Abilityai/trinity-enterprise#477) * fix(metrics): a refused metric type change keeps status_values_json (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. * feat(metrics): metric_points schema, store, validator and wire model (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. * feat(metrics): record_metrics route, Settings knobs, retention sweep 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`). * feat(metrics): record_metrics MCP tool, requirements §48, and the docs 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). * fix(metrics): bind the batch idempotency key to the body, repair the retention test stubs, and close the review's small findings (Abilityai/trinity-enterprise#478) * feat(metrics): re-back GET /metrics with the point store, one stale rule, 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> * feat(metrics): retire metrics.json via D-010, bind dashboard widgets 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> * feat(metrics): declared-metric tiles, bound widgets and the two-flag 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> * docs(metrics): §49 the read contract, D-010 + placeholder rule in the 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> * fix(metrics): drop out-of-window points before bucketing, reachable empty-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> * test(metrics): align the live-tier agent-metrics test with the ent#479 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. * feat(metrics): the objective join — target vs actual with freshness, 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. * feat(metrics): the objectives read, MCP get_objectives, and §50 (Abilityai/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. * fix(metrics): findings only for joined objectives, neutral direction for hold, fan-out budget, skipped-file and invalid-id findings, executable rebase note (Abilityai/trinity-enterprise#666) * merge-train: re-chain the registry revision to 0069 after dev's 0066–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> * merge-train: make SOURCES live, hash-stable definition_hash, mcp header 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> * merge-train: drop two unreferenced store helpers, one source for the 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> * merge-train: wire METRICS_READ_RATE_LIMIT into compose and .env.example (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> * merge-train: the cleanup suite's mock learns the metric-point accessors, 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> * merge-train: the bound-widget threshold reads the registry's direction 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> --------- Co-authored-by: Trinity Agent (trinity) <trinity-agent@ability.ai> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: sim <sim@example.com>
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#477 — sub-issue 1 of epic abilityai/trinity-enterprise#476 (declared business metrics; operator one-way ruling of 2026-09-21 on the epic).
What
template.yaml metrics:gets its missing backend reader and becomes the metric registry — the schema every later step of the chain (record_metrics store ent#478, read + freshness ent#479, objective join ent#666) validates against.services/template_metrics.py— total (never raises), named errors with did-you-mean,x-keys kept, caps (50 metrics / 50 status values / 10 dimensions), cadence grammar normalized to seconds.db/metric_definitions.py+ dual-track schema (db/schema.py,db/migrations.py, Alembic0066_metric_definitions,db/tables.py): one row per definition,UNIQUE(agent_name, name),status active|retired,definition_hash,type_conflict;AgentRefcascade.services/metric_registry.py— set-diff upsert (insert / update / revive / retire); live read through the compat collector's boundedcatdoor.POST /api/agents/{name}/metrics/definitions/refresh.GET /api/agents/{name}/metrics/definitions(?include_retired), gated byAuthorizedAgentByName— same reach as the existing/metrics.tests/journeys/catalog.yaml+test_j12_declared_metrics_journey.py(registered intests/registry.json).Rulings carried (orchestrator, on the operator's behalf — recorded in the plan file and on ent#476)
policy: retention_days 365, daily_point_cap 100000, enforced: false, enforced_by ent#478) andtest_ent477_settings_knobs.pyasserts the knobs are absent. T3 MCP definitions tool: no (ent#479get_metricscovers). T4 crud.py hotspot: deferred — D1/D2 registered in trinity-devDEBT_INBOX.md@ 97b7bb0. T5 metric type change on re-sync: refused — the row keeps its type,type_conflictexposes the declared one; a type change is a new metric name.Migration order — read before merging
0066_metric_definitions←0065_agent_skills_delivery_status(dev, PR #2920). This PR lands ahead of the open stacked PRs #2924 (0066_public_user_memory_writes) and #2927 (0067_agent_role_readiness): both must renumber behind it (0067_public_user_memory_writes ← 0066_metric_definitions,0068_agent_role_readiness ← 0067_…) before they merge.alembic-head-watchis advisory, so this is a human step.scripts/ci/check_alembic_heads.pyon the merged graph: 1 head.Review + security
/review(Fable, report-only): MERGEABLE AFTER FIXES → both criticals fixed in the second commit (C1 the fork above; C2 the reader raisedTypeErroron non-string YAML keys —key=stron two sorts, regression cases, Hypothesis key strategy widened)./cso --diff: clean (secrets / PII / hosts / injection / access); S1 = C2, S2 = C1. Informational, left as follow-ups: start hook not readiness-gated like #2069 (I2); retention literal in the router until ent#478 flipsenforced(I4); pull / reset / sync hooks awaited inline, worst-case +20 s (I5); pre-existing sync Docker call in an async path (I6).Tests
tests/unit/test_ent477_*.py(8 files) + updatedtest_compatibility_checks.py,test_2338_journey_catalog.py; PG leg of the Alembic test is@requires_postgres(schema-parity job). Unit island (per-PR CI job) is the authoritative run — the engineer has no docker.Handoffs
ent#478 (knobs + sweep,
type_conflictvalidation, the two facts posted there) · ent#479 (re-back/metricson the store, one stale rule, retire themetrics.jsonread) · ent#666 (objective join) · PRD + epic: journey id is J12.🤖 Generated with Claude Code