MET-WP1-12: Add Metrics registry administration - #896
Draft
ja573 wants to merge 2 commits into
Draft
Conversation
Adds the SUPERUSER-only administrative GraphQL surface for the three MET-WP1-01 registries metric_platform, metric_measure and metric_platform_measure (MET-WP1-12, issue #894), on the feature/metrics programme integration branch. Six mutations and three bounded administrative lookups: createMetricPlatform / updateMetricPlatform createMetricMeasure / updateMetricMeasure createMetricPlatformMeasure / updateMetricPlatformMeasure metricPlatformByCode / metricMeasureByCode / metricPlatformMeasureByCodes All nine authorize through require_superuser() before any mutation-specific database read, row lock or write, so anonymous callers, publisher users and admins, and machine roles including DISSEMINATION_WORKER are denied leaving both the registry and the audit history untouched. No Metrics service role, capability shortcut, authorization policy, ThothError variant or GraphQL error type is introduced, and policy.rs is untouched. Administration addresses rows by their stable code, and a mapping by its (platformCode, measureCode) pair, never by a database-generated UUID, so the migration-owned title_sessions and net_units measures are administrable without out-of-band identifier discovery. Codes are exact PostgreSQL TEXT at every entry point: create stores what was supplied, and lookup and update selectors compare literally, with no trimming, case folding, ILIKE, Unicode or whitespace normalisation or aliasing, so case and whitespace variants remain distinct. Patch inputs are complete replacements of the approved mutable field set rather than sparse patches, so for the nullable publicDescription and methodologyVersion both omission and explicit null store SQL NULL. The immutable fields - a platform's code and ownershipClass, a measure's code, category, unit, allowNegative, additiveAcrossTime and additiveAcrossWorks, and a mapping's platform/measure identity - are absent from the patch inputs and from every SET clause. There is no delete mutation; retirement is enabled = false, an ordinary audited update. Migration 20260908_v1.9.0 adds the closed enums metric_registry_history_entity and metric_registry_history_action and the append-only metric_registry_history table, carrying entity, entity_id, action, actor, nullable before_state, required after_state and created_at. It deliberately has no updated_at and no trigger, no foreign key on the polymorphic entity_id so audit evidence is never cascade-deleted with the registry state it describes, no DELETE action because no delete mutation exists, and no secondary index because no approved audit access path exists yet. No timestamp column is added to metric_platform_measure. Every committed create and update writes its canonical row and exactly one audit row in one PostgreSQL transaction, recording the exact persisted state read back from the database rather than the request. Rejected, unauthorized, failed and genuinely no-op requests write neither, and a no-op moves no canonical timestamp. Concurrency is serialized last-write-wins: each update takes exactly one application-requested FOR UPDATE lock on the canonical row being updated, mapping updates resolve their platform and measure codes through ordinary non-locking reads and lock only the mapping row, no joined multi-table FOR UPDATE exists, and creates take no application lock. PostgreSQL remains authoritative for code uniqueness, pair uniqueness, foreign-key validity, the nonblank CHECKs and the supported_grains CHECK. The bounded Metrics constraint mappings added to thoth-errors/src/database_errors.rs keep those failures from reaching a client as raw PostgreSQL text, constraint names, SQL or driver diagnostics. The GraphQL change is strictly additive - three object types, six inputs, the additive exposure of four existing closed database enums, six mutations and three queries - with zero deleted or replaced SDL lines against the base. The audit table is exposed nowhere in the schema. The surface remains additive and inactive: no platform, platform-measure mapping, source, source account or OPERAS mapping is seeded, and no collection, ingestion, rollup, synchronization, dashboard or production behaviour is enabled or triggered by a registry change.
The two contended-row concurrency tests asserted their transition order positionally over audit rows returned by `audit_rows`, which orders by `created_at`. PostgreSQL evaluates the column's `CURRENT_TIMESTAMP` default at transaction start, not at commit, so a transaction can start first, lose the race for the canonical `FOR UPDATE` row lock and commit second. Ordering audit rows that way could therefore invert the real serialized order, and the secondary key is a random v4 UUID that orders nothing. The evidence was scheduler-dependent. The tests now reconstruct the transition chain from the exact `before_state`/`after_state` values, through a new `serialized_update_chain` helper, and never from `created_at`, the primary key, the thread spawn order or the `join()` order. That chain is authoritative because each update coordinator holds the canonical row's `FOR UPDATE` lock through commit: a competing update cannot read the row, and so cannot form its `before_state`, until the holder has committed. The reconstruction fails the test on a gap, a duplicate or a fork rather than tolerating it, and each test additionally asserts that exactly two UPDATE transitions exist beside the CREATE, that the chain starts at the created row and ends at the final persisted row, and that both actors and both requested values appear exactly once and correctly paired. No production behaviour changes: the coordinators, migration SQL, GraphQL surface, error boundary and schema are untouched, and the audit schema was not extended with a sequence or a commit timestamp to make the tests easier. Also reconciles the implementation report: section 11.4 now states how the ordering evidence is established and that `created_at` is not the oracle, section 18.1 records the correction, section 19 notes that the audit table records no commit order, and the cumulative diff statistics are recalculated.
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
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.
MET-WP1-12 — Metrics registry administration GraphQL foundation
DRAFT. This PR requires a fresh independent exact-head source review before
it may be marked ready. Merge, deployment, staging/production migration
execution, release and production activation are not authorized by anything
in this PR.
Closes nothing on its own. Owning issue: #894. Parent programme: #766.
Binding
MET-WP1-12PROGRAMME_INTEGRATIONfeature/metrics @ b5ca7069c9ac21a42092fdbd7400ec01b069b99edevelop4546cb632428872b961ad6c17282984d298e3ade75dc0537781a876875d9914f91666944f78d3bd5feature/metricsthoth-api/migrations/20260908_v1.9.0Authoritative specification, in precedence order (later controls earlier):
5574219884;5581498851;5584094951;5584211971.Both branch SHAs, the absence of the task ref and of any matching PR, and the
absence of any
20260908_*migration onfeature/metrics,developandmasterwere reverified immediately before the commit and again immediatelybefore the push.
What this adds
A bounded, SUPERUSER-only administrative GraphQL surface over exactly the three
MET-WP1-01registriesmetric_platform,metric_measureandmetric_platform_measure, plus the Metrics-local append-only audit table thatrecords every committed change.
Migration
20260908_v1.9.0creates the closed enumsmetric_registry_history_entity(PLATFORM,MEASURE,PLATFORM_MEASURE) andmetric_registry_history_action(CREATE,UPDATE) and the append-onlymetric_registry_historytable (entity,entity_id,action,actor,nullable
before_state, requiredafter_state,created_at). It deliberatelyhas no
updated_atand no trigger, no foreign key on the polymorphicentity_id, noDELETEaction, and no secondary index. No timestampcolumn is added to
metric_platform_measure.Authorization — SUPERUSER only, fail-closed
All nine operations authorize through the repository-authoritative
require_superuser()guard before any mutation-specific database read, rowlock or write. Anonymous callers, authenticated users with no applicable role,
PUBLISHER_USER,PUBLISHER_ADMIN,WORK_LIFECYCLE,CDN_WRITEand theDISSEMINATION_WORKERmachine role are all denied, and a denied call changesneither registry nor audit state.
No Metrics service role, capability shortcut, authorization policy,
ThothErrorvariant or GraphQL error type is introduced.
thoth-api/src/policy.rsandthoth-errors/src/lib.rsare untouched.Evidence: 63 denied executions (7 principals × 9 operations) each returning
NO_ACCESS/Unauthorized, followed by assertions that the registry and audittables are unchanged. Separately, a context whose pool points at an
unreachable database still returns the fail-closed denial for every
operation rather than a connection error — a behavioural proof that the guard
precedes database access.
Audit — append-only, atomic, exact
Every committed create and update writes its canonical row and exactly one audit
row in one PostgreSQL transaction. The recorded state is the row read back
from the database, not the request, so generated identifiers and
database-authored timestamps are exact.
CREATErecordsbefore_state = NULL;UPDATErecords the exact state it overwrote.Rejected, unauthorized and failed requests write neither. A genuine no-op
update runs no
UPDATEstatement, moves no canonical timestamp and records noaudit row.
Atomicity is proved rather than asserted: a blank actor violates
metric_registry_history_actor_checkafter the canonical INSERT has alreadysucceeded inside the transaction, and neither row survives. Deleting a
referenced registry row leaves its audit evidence in place, which is what the
absent foreign key is there to deliver.
Concurrency — serialized last-write-wins, one row lock
Each UPDATE requests exactly one application
FOR UPDATElock, on thecanonical row being updated. Platform-measure updates resolve their platform and
measure codes through ordinary non-locking reads and lock only the mapping
row. No joined multi-table
FOR UPDATEexists anywhere, no coordinator takes asecond lock target, and CREATE takes no application lock at all.
Evidence from real multi-connection PostgreSQL: two competing updates serialize
and the audit chain matches actual commit order with no gap; a mapping update
completes while another connection holds a genuine
FOR UPDATElock on thereferenced platform row (it would block if the coordinator locked its parents);
concurrent platform and mapping updates both succeed with no coordinator-created
lock cycle. Source-shape assertions additionally pin the single lock target.
Stable codes — exact, never normalized
Codes are ordinary PostgreSQL
TEXT. Create stores exactly what was supplied;lookup, update selectors and platform-measure code resolution all compare with
plain equality. There is no trimming, case folding,
ILIKE, Unicodenormalization, whitespace normalization or aliasing at any entry point.
Mixed_Case,mixed_case,leading,trailing,inner space,unícodeand
UNÍCODEall coexist as distinct stored codes, andMIXED_CASE,leading,trailing,innerspaceandunicoderesolve to nothing rather than foldingonto one of them. The seeded
title_sessionsandnet_unitsmeasures areadministrable by their stable codes with no out-of-band UUID discovery, and
creating
Title_Sessionsneither collides with nor rewrites the seed.Replacement semantics and immutability
Patch...is a complete replacement of the approved mutable field set, not asparse patch: for the nullable
publicDescriptionandmethodologyVersion,omission and explicit
nullboth store SQLNULL.Immutable fields are enforced structurally, not by validation — they are absent
from the
Patch...inputs and from everySETclause: a platform'scodeandownershipClass; a measure'scode,category,unit,allowNegative,additiveAcrossTimeandadditiveAcrossWorks; and a mapping's platform/measureidentity. There is no delete mutation; retirement is
enabled = false.Sanitized database-error boundary
PostgreSQL remains authoritative for code uniqueness, pair uniqueness,
foreign-key validity, the nonblank CHECKs and the
supported_grainsCHECK. Onlythoth-errors/src/database_errors.rswas extended, with eleven bounded mappings(the nine Amendment 2 baseline constraints plus the two new audit CHECKs).
Five real failures driven through the GraphQL boundary return their exact
bounded message, and the whole rendered response is asserted to contain none of:
constraint names,
duplicate key value,violates,INSERT INTO,SELECT,DETAIL:,CONTEXT:,pg_,postgres://,cardinality(orDatabase error.The two
metric_platform_measureforeign keys are deliberately not mapped:the coordinator resolves both codes inside its transaction, so an unknown code
fails as
EntityNotFoundand never reaches the FK. Recorded as a reviewedconclusion in the implementation report, §10.1.
No seeds, no runtime activation
No platform, platform-measure mapping, source, source account, OPERAS mapping or
event URI is seeded or guessed. Nothing is enqueued, backfilled, imported,
collected, rolled up, exported, reconciled, deployed or activated. Changing
directCollectionis a configuration write only; no WP1 runtime path reads ityet.
GraphQL contract — strictly additive
Base and final SDL were both generated and compared. A line-level opcode
analysis reports 0 deleted, 0 replaced, 183 inserted — the base SDL is an
exact subsequence of the final SDL, so no existing type, field, input, enum
variant, nullability, argument, default or description changed.
b5ca7069091e11f293132fdec784de420e3addf251f5020ba7e387889b292a066be15d8e75dc0537d5ac40b8f381f12f62e8c507f24130199189b40750c040f5a6f41997e1e4963bAdded: 3 object types, 6 inputs, 4 additive exposures of existing closed
database enums, 6 mutations, 3 queries. The audit table is exposed nowhere.
Downstream impact assessed for
thoth-app,thoth-pyramid,thoth-dissemination, standalonethoth-client/thothlibrary, the internalRust
thoth-client/thoth-export-server,metrics-dashboard,metrics-widget,baboonand Sphinx: no consumer requires a change, because the schema changeis provably additive. No downstream repository was modified.
Path containment
Exactly 25 of 25 authorized paths; no 26th path, no deletion, rename or
move.
thoth-errors/src/lib.rs,thoth-api/src/policy.rs,thoth-api/src/graphql/tests.rs,thoth-api/src/graphql/mutation_guard.rs,Cargo.toml,Cargo.lock, every existing migration, all workflows/scripts andevery downstream repository are untouched.
Local verification
MET-WP1-12adds 83 tests: 8 audit-table shape/migration, 21/20/22 registrycoordinator, 12 GraphQL surface, plus one inline mapping test in
thoth-errors.Migration evidence
Run against a disposable PostgreSQL 17.10 database with the embedded runner, in
the exact sequence CI uses:
Additionally, with the audit table populated, a targeted revert/reapply left all
19 predecessor Metrics tables and both migration-owned measure seeds intact.
Lock profile, measured not asserted: the migration takes no lock on any
pre-existing table. The only lock held is
AccessExclusiveLockon its own newprimary-key index, an object no other session can see before commit. This is
stronger than the usual Metrics-table profile precisely because the audit table
has no foreign key, so there is no parent to take
SHARE ROW EXCLUSIVEon;concurrent reads and writes to every existing table proceed throughout.
Reverting is a genuinely lossy downgrade of audit evidence — stated explicitly
in
down.sql— accepted because the table is empty at deployment and thecanonical registry rows are untouched by the revert.
Known limitations and deferred scope
operator cannot inspect the trail through the API yet.
notice. The audit chain records what happened; nothing prevents it. This is
the approved Amendment 1 §K trade-off.
Patchis a full replacement, so omitting a nullable field nulls it — a realfoot-gun for a hand-written mutation.
ownership_classhas no in-band repair path; correction needs separatelyreviewed work.
docs/metrics/migration-inventory.mdis not updated — it is not in theauthorized budget, and the predecessor slice did not update it either.
Deferred to separately specified work: publisher-platform approval
administration; source/source-account/checkpoint administration; the
service/dashboard registry queries
metricPlatformsandmetricMeasures; anydelete or bulk operation; any audit-history query; and any real platform,
platform-measure, source, source-account or OPERAS seed.
Review note
One
MET-WP1-11guard,no_reconciliation_enum_trigger_procedure_or_graphql_surface_was_introduced,asserts the generated SDL never contains the word "reconciliation". It is a
whole-SDL substring check and cannot distinguish a real surface from a denial of
one, so it was tripped by this slice's own
createMetricPlatformMeasuredescription ("…Starts no collection, import, export or reconciliation"). The fix
was to reword this slice's description;
thoth-api/src/model/metric_reconciliation_run/tests.rsremains byte-identicalto the authorized base and no write-budget amendment was required.
Gates
This PR is DRAFT and has not been independently reviewed. The next gate
is a fresh independent exact-head source review bound to
75dc0537781a876875d9914f91666944f78d3bd5; any subsequent commit invalidatesit. Merge requires separate explicit CTO authorization bound to the exact
reviewed head. Deployment, staging/production migration execution, release and
production activation remain separate, unauthorized gates.
The full evidence record is in
docs/engineering/ai-delivery/implementation-reports/MET-WP1-12-implementation-report.md.