fix(api): order same-instant audit events by a durable monotonic key, not a random Guid

[mforce]

Split out of the #494 review (PR #503, codex round 3). Accepted there as a documented residual because the fix needs a migration, which #494 was specified not to do.

The defect

AuditEventRepository.GetProvenanceChunkAsync picks the last change with:

ORDER BY e."EntityId", e."OccurredAtUtc" DESC, e."Id" DESC

AuditWriter mints Guid.NewGuid() — a random v4 — so "Id" carries no chronology. When two reportable changes for one entity share an OccurredAtUtc, the tiebreak resolves arbitrarily and the wrong actor can be named as the last changer. The timestamp displayed is correct either way; only the name is at risk.

ListAsync uses the same tiebreak. There it is merely arbitrary rather than wrong — paging only needs a deterministic order — but the Audit page can still display a same-instant pair in the wrong order.

Why it is reachable, not theoretical

Most concurrent changes to one record cannot collide: the aggregate Version concurrency token serialises them, and the loser gets a 409 having written nothing (audit shares the transaction).

RecordBirdMovementHandler escapes that argument. It writes an audit event keyed to the flocks id while only inserting a BirdMovement row — it never mutates Flock, so no Version bump, so it does not serialise against Flock.Update. Two such requests can land in the same microsecond against the same flock.

What is needed

A durable monotonic ordering value on AuditEvents — a bigint GENERATED ALWAYS AS IDENTITY or a sequence-backed column — used as the tiebreak in both queries.

Note the alternatives that do not work, so they are not re-proposed:

• UUIDv7 for new ids — only orders to millisecond granularity and keeps a random tail, so it does not resolve a microsecond collision; and existing rows are v4, so a mixed table orders meaninglessly.
• ctid — physical location, not durable across VACUUM FULL.

Cost

A migration (#407: its own dotnet ef migrations add, never folded into InitialCreate), a docs/schema/ regeneration committed in the same PR (#417), and a sim-harness pass (#370).

Deliberately not done in #503

No test pins the current arbitrary outcome — pinning a known loss turns "did not fix" into "spec". The behaviour is recorded in a comment at the query and here.

[mforce]

Findings from reading the code after #503 merged (4ffa7f17)

Four things the issue body should carry before anyone picks this up.

1. Raising the timestamp's precision is a non-fix — add it to the list of rejected alternatives

OccurredAtUtc is already timestamptz, which Postgres stores at microsecond precision, and AuditWriter reads clock.UtcNow per call rather than once per unit of work. So the collision is not "the column is too coarse". Two writers can legitimately produce the same microsecond, and no number of extra digits creates an ordering between them — only a shared monotonic counter does. Belongs beside UUIDv7 and ctid in What is needed, because "just store a finer timestamp" is the first thing anyone will reach for.

2. There are FIVE tiebreak sites, not two

The body says "both queries". AuditEventRepository actually has five, and a fix that updates fewer leaves the defect live in whatever it skips:

Line	Site	Collision reachable?
:33	ListAsync (LINQ, the admin /audit viewer)	yes — display order only
:96	created	no — at most one .Create per entity
:223	creator CTE	no — same reason
:241	outer last-change	yes — this is the defect
:265	promoted	no — neither aggregate has a path back to Draft

Only two can actually resolve wrongly today, but all five should move to the new column: the three that cannot collide are not guaranteed not to, they are merely unreachable given today's actions, and leaving a random-Guid tiebreak behind re-creates the defect the moment that changes.

3. Prefer GENERATED BY DEFAULT AS IDENTITY over GENERATED ALWAYS

The body specifies ALWAYS. BY DEFAULT is the better choice here: ALWAYS rejects an explicit value outright, which blocks any future backfill or data migration that needs to preserve an order it computed. Nothing in the app writes the column either way — EF marks it ValueGeneratedOnAdd and never sends it — so ALWAYS buys no real protection while costing that escape hatch.

4. The migration rewrites the table — which argues for doing it now, not later

ALTER TABLE … ADD COLUMN … AS IDENTITY has a sequence-backed default, so Postgres cannot use the fast constant-default path: it rewrites the whole table and takes an ACCESS EXCLUSIVE lock for the duration. Two consequences:

• Existing rows get their values in heap order, which is not insert order after any update or vacuum — i.e. arbitrary. That is not a regression (those rows are arbitrary today), but nobody should read the backfilled values as historical truth.
• It is cheap now, while AuditEvents is small and pre-1.0, and it only gets more expensive. If it does wait until the table is large, the shape becomes: add a nullable bigint with a sequence default, backfill in batches ordered by (OccurredAtUtc, Id), then SET NOT NULL — more steps, no rewrite lock.

One caveat to state in the fix, not discover later

A sequence gives insert order, not commit order. Two concurrent transactions can take sequence values in one order and commit in the other. That is still strictly better than a random v4 and correct for every case the History column cares about — but the fix should not claim to establish a true total order for auditing purposes, or the next person will rely on something it does not provide.

Confirmed non-impacts

• CSV export / full backup — AuditEvents is not one of the exported datasets, so no export contract changes (per Daily entry: require grading to reconcile sellable eggs before submit #394, checked by reading the callers rather than by running).
• AuditVocabularyCoverageTests — keys on actions and entity types, untouched by a new column.