feat(metrics): a corrected value restates its metric point (abilityai/trinity-enterprise#729)

[webmixgamer]

Summary

• Ruling R45 ("Accept corrections"). When record_metrics gets a point whose (metric, ts, dims) already exists but whose value differs, it now updates that row in place instead of dropping the new value. An identical value is still a dedup and writes nothing.
• What a correction changes. revision goes up by 1. recorded_at and execution_id follow the correcting write. ts (the period), created_at (what the daily cap counts) and dims never move. Freshness and every read are unchanged: the read path selects explicit columns and computes freshness from ts.
• The receipt gains corrected beside recorded and deduplicated, on both the REST 201 and the MCP record_metrics result. All contract text that taught "a correction is a new ts" is amended.

Changes

• src/backend/db/metric_points.py
  • The insert becomes ON CONFLICT … DO UPDATE … WHERE the stored value IS DISTINCT FROM the incoming one. That comparison is null-safe.
  • RETURNING revision gives a portable three-way count, returned as a PointWriteCounts NamedTuple.
  • The store refuses two rows with one identity in a single call. Postgres raises an error on that and SQLite would apply both, so the two databases would disagree.
  • Rows are written in (ts, key) order, so two overlapping batches can't deadlock on Postgres.
• Schema (both tracks): revision BIGINT NOT NULL DEFAULT 0 and recorded_at TEXT. recorded_at is nullable with no backfill; NULL means "written before ent#729".
  • SQLite: db/migrations.py::metric_points_restatement.
  • Postgres: Alembic 0086_metric_points_restatement, off 0085_ent720_email_identity.
  • DDL in schema.py and tables.py.
• models.py::MetricPointsResult.corrected defaults to 0, so idempotency snapshots stored before this change still replay.
• routers/metric_points.py
  • The counts are read outside the store try, so a shape error can't be answered as a retryable 503.
  • One INFO log line per batch that corrected anything. It logs the count only, never a value or a dimension.
• MCP (src/mcp-server/src/tools/metrics.ts, client.ts)
  • The tool description teaches the correction rule.
  • The result carries corrected: result.corrected ?? 0, so an older backend reads as 0 (it can never correct).
• Docs
  • Requirements: §48.2, §48.3 and §48.6, plus a new §48.9 that states the limits: last write wins by arrival order, the replaced value isn't kept, and an identical earlier batch replays over a later correction.
  • Agent guide and user docs.
  • Architecture: the database, api-endpoints and mcp-server entries.
  • The custom-metrics feature flow.
  • A learnings fragment and the /cso --diff report (0 findings).

Test Plan

• New tests: cd tests/unit && pytest test_ent729_metric_restatement.py -v. They use real rows on SQLite. Under requires_postgres they also run on real PostgreSQL 16, including the Alembic upgrade 0085 → head over an existing row. Coverage:
  • restating a row in place, and an identical repeat that writes nothing;
  • a mixed batch with deliberately uneven counts;
  • a value that moves between the numeric and text columns;
  • execution_id following the correcting write;
  • the duplicate-identity guard and the write order;
  • the migration and rollback safety;
  • the daily cap left unmoved;
  • record, age, correct, and check that stale hasn't moved, through the real read_agent_metrics;
  • read parity: a corrected store reads exactly like a born-correct one, through get_metrics' composition and the objective join's latest_by_metric.
• Route and MCP receipt tests: test_ent478_record_route.py and src/mcp-server/src/tools/metrics.test.ts. They cover corrected in the 201, an old snapshot replaying, the documented replay limit, and an older backend reading as 0.
• Mutations: each of the nine lines that applies the rule turns a test red when broken. The lines are the WHERE clause, revision + 1, the sort, the duplicate guard, recorded_at, execution_id, created_at, the null-safe comparison, and the route's corrected.
• Review round (30b73184f), from an independent backend / test-quality / contract review:
  • The migration test runs the registered MIGRATIONS entry; deleting the registration used to pass every suite.
  • Five more mutants now turn a test red.
  • A store-shape refusal or an unreadable store result is a non-retryable 500 that releases the idempotency claim.
  • The tool description now covers the in-turn replay limit.
• Existing ent#478 store, route and validation suites updated for the three counts; the schema-parity, Alembic-parity, Alembic-heads and auth guards pass unchanged.
• CI Postgres tier (pytest tests/unit -m requires_postgres against real PostgreSQL): 60 passed. The 6 skips are environmental and predate this branch.
• /verify-local --skip-agent:
  • The backend image builds and import main works inside it.
  • The sibling stack boots healthy on a fresh database, with every migration applied.
  • Integration: 70 passed, 0 failed.
  • In the full unit run, the only failures were the known order-dependent ent666/ent477/retention_floor set. Those files pass on their own.
• Manual check on a live dev stack:
  • On dev, a correction came back deduplicated: 1. On this branch it came back corrected: 1, and an identical repeat came back deduplicated: 1.
  • The row shows revision 1, with recorded_at newer than created_at and ts unchanged. The tile shows the new value.
  • Back on dev, the old code inserts fine with the new columns present.

Migration numbering: 0086 is also claimed by #3145, #3021 and #3022, and #2984 holds 0087. Whichever merges second needs to re-parent. alembic-head-watch flags a fork.

Fixes abilityai/trinity-enterprise#729

🤖 Generated with Claude Code

[vybe]

merge-train: batch validated on train #3172