From ee63d057e0077edc876e05d99886d68f0d4a0868 Mon Sep 17 00:00:00 2001 From: James Dunnam <7660553+jimador@users.noreply.github.com> Date: Mon, 31 Aug 2026 10:00:26 -0400 Subject: [PATCH 1/7] Define the extraction-run lifecycle and store contract The state machine lives in one place: save accepts only RUNNING runs, and ExtractionRunTransition applied by the store under compare-and-set is the only way a run reaches a terminal status. Replaying the same terminal payload succeeds; an incompatible rewrite is rejected by insert-or-compare on a fingerprint of the transition payload alone, so a replay survives an interleaved invocation record while a materially different retry cannot pass as one. Every scoped read is bounded and scopes before it limits; lineage walks are bounded and cycle-safe, and all runs of a root resolve in one read off the denormalized ref. Each kind of state has exactly one owner. Header writes carry a version and take it under compare-and-set, and they write header fields only: whatever invocation list a caller hands to save is carried through untouched, and an insert starts with no invocation rows at all. recordInvocation is the only door onto invocation state, insert-or-compare on the invocation's own key. A header save therefore cannot create, update or delete an invocation row, and a stale header carrying an old snapshot leaves a newer stored record alone. Run state accumulates from independent writers, the way a lineage event stream does; one writer's row is never another writer's to rewrite. The cross-backend contract suite carries the discriminating cases, so the Drivine store inherits these semantics with nothing left to remember: interleaved replay, lineage disagreement, the terminal matrix, transition races that prove which write landed, ownership of invocation rows across both doors, and concurrent writers on separate attempts both landing. Every field of a run and of an invocation record is pinned through the door that writes it, including clearing a stored value back to null. Terminal records are locked alike whatever outcome they carry, through recordInvocation. Closed value sets are exercised by iterating their own entries, so a newly declared constant is covered the day it is added. Signed-off-by: James Dunnam <7660553+jimador@users.noreply.github.com> --- CHANGELOG.md | 211 ++ .../AbstractExtractionRunStoreContractTest.kt | 1968 +++++++++++++++++ .../InMemoryExtractionRunStoreContractTest.kt | 28 + .../extraction/ExtractionInvocationRecord.kt | 5 + .../proposition/extraction/ExtractionRun.kt | 37 +- .../extraction/ExtractionRunFingerprint.kt | 175 ++ .../extraction/ExtractionRunStore.kt | 444 ++++ .../extraction/ExtractionRunTransition.kt | 238 ++ .../extraction/InMemoryExtractionRunStore.kt | 331 +++ .../extraction/ExtractionRunContractTest.kt | 5 +- .../ExtractionRunFingerprintTest.kt | 307 +++ .../extraction/ExtractionRunFixtures.kt | 34 + .../extraction/ExtractionRunLifecycleTest.kt | 599 +++++ .../extraction/ExtractionRunStoreReadsTest.kt | 372 ++++ .../extraction/ExtractionRunValueTypesTest.kt | 8 + docs/design/extraction-runs.md | 435 +++- 16 files changed, 5163 insertions(+), 34 deletions(-) create mode 100644 dice-storage/src/test/kotlin/com/embabel/dice/storage/AbstractExtractionRunStoreContractTest.kt create mode 100644 dice-storage/src/test/kotlin/com/embabel/dice/storage/InMemoryExtractionRunStoreContractTest.kt create mode 100644 dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunFingerprint.kt create mode 100644 dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunStore.kt create mode 100644 dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunTransition.kt create mode 100644 dice/src/main/kotlin/com/embabel/dice/proposition/extraction/InMemoryExtractionRunStore.kt create mode 100644 dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunFingerprintTest.kt create mode 100644 dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunLifecycleTest.kt create mode 100644 dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunStoreReadsTest.kt diff --git a/CHANGELOG.md b/CHANGELOG.md index 9bf13348..8ddc9a98 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1344,3 +1344,214 @@ and the consumer PRs that deliver it). because core `Usage` does not report them. The record stays its own type: `Usage` is final and carries the native SDK object this record deliberately does not store. **Compatibility: additive.** One new factory method, nothing existing changes. + +- **EXPERIMENTAL.** The extraction run lifecycle and store contract in `dice` core, plus the + in-memory reference implementation — the state machine DICE #67's Drivine store and coordinator + build on. A run starts `RUNNING` and ends `COMPLETED`, `FAILED` or `CANCELLED`; there are no other + edges, a terminal run never re-opens, and it never moves from one terminal state to another. + **`ExtractionRunStore` splits its writes along that line.** `save` records a running run and + rejects any other status, so a terminal status cannot enter through the door that also accepts new + keys — and rejects a running run carrying a `finishedAt`, since `ExtractionRun` leaves + status-and-timing pairing to the state machine and a record that reads as running and as finished + at once makes every page meeting it guess which. `transition` is the only writer of a terminal + status and is compare-and-set. That makes + `COMPLETED`'s rule enforceable rather than advisory: it asserts every product the run's request + called for is durably persisted or terminally disposed, so it is written only after persistence — + by the coordinator once `persistAndProject` returns on the legacy path, and inside the commit + transaction on the #68 path. A run whose persistence never finished stays `RUNNING` and retryable, + a partial-success commit leaves it `RUNNING` so a terminal run never has re-committable products + behind it, and a run with zero products completes vacuously. **A transition derives the terminal + run; `ExtractionRun` gains no mutators.** `ExtractionRunTransition` carries the terminal status, + the finish instant and optionally the final counts and failures, and its `applyTo` is the only + place a terminal run is derived. `counts` and + `failures` are nullable and follow one rule — null keeps what the run recorded, a value replaces + it — and an empty failure list is a value. A `FAILED` transition need not carry a failure and a + `COMPLETED` one may, because a run that retried past a failed attempt and finished still happened. + **Idempotency is insert-or-compare on a canonical payload fingerprint, never `MERGE … SET` + overwrite.** A store records the fingerprint of the write that ended a run; a repeat with the same + fingerprint replays as success and changes nothing, a repeat with a different one throws + `ExtractionRunConflictException`, and a `save` against a terminal run is rejected outright. + Overwriting is safe for a record still being written and wrong for one that is finished, because it + lets a late or duplicated writer silently rewrite how a run ended. The fingerprint covers the + terminal write and not the run, so a coordinator that recorded another attempt between a terminal + write it never saw the answer to and its retry still replays. `ExtractionRunFingerprint` specifies + the encoding rather than leaving it to a serializer: the length-prefixed, count-prefixed, sorted + SHA-256 convention `MetamodelVersion.contentHash` already uses, chosen against the three RFC 8785 + failure modes — key order, number rendering and insignificant text — with instants rendered fixed + width, absent distinguished from empty, a version tag on the input, and a golden-literal test + pinning the digest of a fixed payload because it is a persisted format. Two mechanisms from the + idempotency prior art are deliberately not adopted: no epoch or writer generation of the Kafka + kind, since epochs fence zombie writers across systems and the compare-and-set inside one store + transaction already decides two writers racing on one row; and no key expiry of the Stripe kind, + since a run header is a permanent audit row and a 24-hour prune would delete the evidence rather + than the bookkeeping. **Every read is tenant-scoped and bounded.** `findRun` and `invocationsOf` + take a key; `runsInContext`, `childrenOf` and `runsOfRoot` take a positive limit, the first and + last also an optional `since` window; `ancestorsOf` walks the parent chain bounded and cycle-safe. + Scope is pushed into the query and applied before the limit — a page that limited first would drop + a tenant's runs behind a busier neighbour's and report the shortfall as an empty tenant — which is + why none of the scoped reads has a default body. The `ContextId`-typed overloads do have default + bodies and only forward to the `String`-typed override point, which exists because `ContextId` is + a Kotlin value class whose methods get mangled JVM names; `ExtractionRunKey` gains a matching + `of(contextIdValue, runId)` factory for the same reason. Pages come back newest first by start + time, tie-broken by run id, so a page is repeatable when two runs share an instant. Every lookup, + page, chain walk and aggregate fails closed across tenants, and the chain walk stops rather than + crossing: a parent that resolves only in another tenant is treated as unresolved. `runsOfRoot` is + the read the denormalized root reference exists for — a whole lineage in one indexed read. + **A save never deletes an invocation record, and neither write puts a terminal one back to + outstanding.** Invocation records live as child rows with their own lifecycle, so a caller updating counts + from a run it loaded before an attempt was recorded keeps that attempt. `save` and + `recordInvocation` share one merge rule for the records they carry, and that merge preserves the + stored list's own order: an existing id is always replaced at its stored position, and only a + genuinely new id is appended, because `ExtractionRun.equals` compares this list by position and a + reordering replay would read as a change it is not. While an id's stored record is `IN_FLIGHT`, an + incoming record for it updates in place; once the stored record is terminal it is locked, and an + incoming record for it is accepted only when it equals the stored one exactly (a no-op replay) — + every other write for that id is rejected with `ExtractionRunConflictException`, whether it claims + a different outcome or the same outcome with different timing, usage or provider facts. That + closes a delayed `IN_FLIGHT` write racing behind the terminal one, which would otherwise put a + succeeded or failed attempt back to outstanding, and closes the narrower case of a delayed write + that repeats the correct outcome and omits the facts the terminal write actually carried — either + way the record of how the attempt ended survives. Two residuals remain open and are documented + here, because an `IN_FLIGHT` record is last-writer-wins, whole-record, until it turns terminal: a save + that carries an old, non-empty invocation snapshot can overwrite dispatch details a + `recordInvocation` call filled in since that snapshot was read, and two `recordInvocation` calls + racing on the same still-`IN_FLIGHT` id can just as easily overwrite each other's disjoint + dispatch facts: the in-place update replaces the whole record wholesale and never merges fields + from the one it displaces. A save that carries an empty invocation list, the default, leaves every + stored record untouched, and that is the pattern the `save` KDoc now recommends for a caller updating the header + alone; the `recordInvocation` KDoc states its own half of the residual directly, since no + caller-side pattern avoids two writers genuinely racing on one attempt's dispatch facts. + **`ExtractionRun` gains a `version` field, and a header save is compare-and-set on it.** Two + callers can hold a run at once — one updating counts, one recording a source revision it just read + — and whichever saves second must not silently put the header back the way it looked before the + first save. `save` accepts a write only when it names the version currently stored and rejects it + with `ExtractionRunConflictException` otherwise, naming both versions so the caller can read the + run again and rebuild its update; a save whose content is already exactly what is stored replays + as a no-op regardless of the version it names, so a retry that never learned its first attempt + landed is never told it conflicted. The version names the CAS generation the header is currently + at: a run that has never been saved, and the run its first accepted save produces, both carry `0`, + since that first save inserts the row and there is no earlier + generation for it to raise past; a first save naming any other value is rejected; each later save + that actually changes the header raises it by one, while a no-op replay is accepted too and leaves + it where it stood; `recordInvocation` never moves it because an invocation write is not a header + write; and a terminal write carries it across unchanged because a terminal run takes no more + saves. An + earlier version of this change tried a field-by-field merge, letting each save keep whatever the + other did not touch, and a review round found it could not be made correct: two writers' + independent count contributions cannot be recovered by keeping the larger number, since either may + have counted disjoint work the other could not see; a union of source revisions can reverse the + order they were read in, which is the field's own documented meaning; and combining fields from + two different saves can produce a header no writer ever actually held. Compare-and-set never + combines two writers' data — an accepted save replaces the whole header at once, and a stale one + is rejected and left for the caller to retry with its own new work added on top of what is now + stored, which is where the domain knowledge to combine them correctly lives. + **`InMemoryExtractionRunStore` ships in main sources**, following the convention + `InMemoryCollectorTraceStore` set, and its compare-and-set is real: every write and read runs + inside one monitor, so a status read and the write that changes it cannot interleave. It publishes + no unscoped read anywhere, test helpers included, because one instance holds every tenant's runs. A + durable backend gets atomicity from its own transaction, and it has to encode the version compare + and the once-terminal-stays-terminal check itself, the way this store's `synchronized` block + encodes them for the in-memory case — a conditional `WHERE`/`MATCH` on the stored version, and one + on the stored outcome. That conditional check has to sit behind a content-equality check: a + byte-identical resend has to succeed as a no-op at any version it names, so a + backend answers success without writing anything when every field already matches what is stored, + and only reaches the version-gated write when something genuinely differs. `ExtractionRunConflictException`'s + class doc now names all five rejection cases the mechanism produces: an incompatible terminal + rewrite, a write against an already-ended run through either `save` or `recordInvocation`, a save + disagreeing with the stored lineage or start time (tenant is half of the key, so it cannot + disagree without addressing a different run entirely), a save naming a stale header version, and + an invocation write, through either door, that differs from an attempt already terminal. + `AbstractExtractionRunStoreContractTest` in `dice-storage` is the cross-backend suite the Drivine + store will inherit, mirroring `AbstractMetamodelVersionStoreContractTest`'s arrangement; the cases + a durable backend is most likely to diverge on are pinned here, in the cross-backend suite, and + mirrored in a `dice`-local test — replay after an interleaved invocation record (which a backend re-deriving the fingerprint from the + stored run fails), lineage and start-time save rejection separately, kept-versus-replaced counts, + the full terminal-to-terminal matrix, child-row survival, a first save rejecting a nonzero + version, a stale header save rejected outright, an accepted save replacing the whole header, a + byte-identical resend replaying as a no-op at a stale version and at a reordered invocation list, + the terminal lock exercised through `save` and through `recordInvocation` for every conflict + shape it produces, the version check exercised through `save`, the only door a header write can + reach it through, and three concurrent races — `save` + against `save`, `recordInvocation` against `recordInvocation`, and `save` against + `recordInvocation` — each asserting exactly one write lands. Design note: + [docs/design/extraction-runs.md](docs/design/extraction-runs.md). + **Compatibility: source-additive; two Kotlin default-argument entry points need a recompile.** No + existing class loses a member and no signature moves. `ExtractionRun` gains an eighteenth + constructor parameter, `version: Long = 0`, appended after `failures`; the companion `of()` + factory gains the same parameter in the same position. Every existing call site, Kotlin or Java, + keeps compiling unchanged — a source compatibility guarantee. Binary compatibility is narrower, + confirmed by comparing `javap` on the compiled class and its companion before and after this + parameter landed. Before `version`, both the primary constructor and `of()` carried seventeen + parameters, four required (the tenant, lineage, status and start time) and thirteen optional; both + now carry eighteen, fourteen optional. Two synthetic bridges changed descriptor, one per entry + point: the constructor's own bitmask-carrying synthetic constructor, and `of()`'s static + `of$default` bridge, each growing by one `long`. Kotlin's own default-argument call syntax — a + constructor call or an `of()` call omitting any one of the thirteen previously optional parameters, + through either entry point — routes through one of these two bridges regardless of which parameter + is omitted, so a class file already compiled against the previous `ExtractionRun`, using that + calling convention through either door, throws `NoSuchMethodError` against this jar until it is + recompiled — the same category of exposure `SourceRevisionBinaryCompatibilityTest` pins for other + types in this module, though no equivalent test exists yet for `ExtractionRun`. Every other + pre-existing call shape is unaffected: `@JvmOverloads` still publishes the previous + seventeen-argument constructor and the previous seventeen-argument `of()` — every parameter through + `failures`, naming none of the new one — as their own overloads, and both overloads' descriptors + are untouched, so a Kotlin call giving all seventeen previous arguments explicitly to either entry + point, or a Java call at any of the shorter `@JvmOverloads` arities on either, still links without + recompiling. Nothing outside `dice` and `dice-storage` constructs an `ExtractionRun` today, so a + recompiled dependent closes both gaps. + A caller that saves a run once and never again sees no difference. A caller that saves the same run + a second time, building the update without reading back what the first `save` returned, sees + its default `version = 0` conflict with the `1` the store now holds — the same + read-modify-save discipline `save`'s own KDoc already asks of a caller recovering from any other + conflict. No real caller does this today: nothing outside a test calls `save` at all yet, so the + migration this describes is the contract the wiring slice builds against; running code is + unaffected. `ExtractionRunKey` gains a `companion object` with a `@JvmStatic of(String, String)` + factory, which adds API and changes none: the data class + keeps its generated constructor, `copy` and `componentN` unchanged, so source, binary and Java + compatibility all hold. Everything else is new types in `com.embabel.dice.proposition.extraction`. + No stored data changes and no migration is required: nothing persists a run outside the in-memory + store yet, and the first thing that will is the Drivine slice. The fingerprint encoding is a + persisted format from the moment a durable store records one, so it is pinned by a golden literal + in this slice, ahead of the first row a durable store will ever write. Every new type carries + `@ApiStatus.Experimental`, added + to the same class-file assertion the run model uses, and the shapes may still move while the + remaining #67 slices land. + +- **EXPERIMENTAL, reworks the entry above.** Invocation records get their own write door and their + own concurrency control, closing a lost-update window review found in the entry above's design: + `save` no longer merges the invocation records it is handed into the ones already stored. + `recordInvocation` is now the sole door onto invocation state, insert-or-compare on the record's + own `(invocationIndex, attempt)` key. **What was wrong.** Versioning the header protects header + fields; it did nothing for invocation rows folded in by identity, because `recordInvocation` never + advanced the header's version. A header save built well before a later `recordInvocation` call + landed could still name the version currently stored, be accepted as a genuine header change, and + carry a stale invocation snapshot in on the same write — silently replacing `IN_FLIGHT` dispatch + details, or a settled terminal outcome, that call had already recorded, with no conflict raised on + either side. Two independent header writers, each merging an attempt recorded before the other's + own read, could lose each other's facts the same way. That is the lost-update window the finding + on PR #98 named: the header's compare-and-set generation cannot fence state that recording an + attempt does not move it for. **What changed.** `save` now writes header fields only — + `ExtractionRun.invocations` on the run it is handed plays no part in what it accepts, rejects, or + replays as a no-op, and it cannot originate, update or remove an invocation row under any + circumstance. Every invocation write goes through `recordInvocation`, which keeps its existing + in-place-while-`IN_FLIGHT`, locked-once-terminal behavior, entirely off the header's generation: + two attempts on different keys never contend with each other or with a concurrent header save. Run + state here follows the model lineage systems such as OpenLineage use for a run's events — + independent writers contribute rows, and no write rewrites a row another writer owns — the same + shape the header's own compare-and-set already gave header fields in the entry above, now + extended to invocation rows on their own key. **The contract test suite changed with it.** The cases in + `AbstractExtractionRunStoreContractTest` that pinned a header save merging, updating or preserving + invocation children through its own payload are gone, because they encoded the defect; new cases + cover the corrected contract — a stale header save carrying an old invocation snapshot leaves a + newer stored invocation intact, a header save embedding a brand-new, a changed, or an emptied + invocation list never creates, updates or deletes a row, and two concurrent `recordInvocation` + writers on different attempts both land. `ExtractionRunConflictException`'s class doc now names + four rejection cases, down from five: a save can no longer raise the invocation-terminal-conflict + case, since it no longer reads or writes that state. **Compatibility: behavioral, no signature + change.** `ExtractionRunStore` and `InMemoryExtractionRunStore` keep every method signature; a + caller that only ever updated the header through `save` and recorded attempts through + `recordInvocation` sees no difference. A caller relying on the old, defective behavior — a header + save silently carrying an invocation update in on the same write as a header change — stops seeing + that update land through `save` and has to call `recordInvocation` directly instead; no shipped + caller does this today, since nothing outside a test calls `save` with a non-empty invocation list + yet. Design note: [docs/design/extraction-runs.md](docs/design/extraction-runs.md). diff --git a/dice-storage/src/test/kotlin/com/embabel/dice/storage/AbstractExtractionRunStoreContractTest.kt b/dice-storage/src/test/kotlin/com/embabel/dice/storage/AbstractExtractionRunStoreContractTest.kt new file mode 100644 index 00000000..97a57556 --- /dev/null +++ b/dice-storage/src/test/kotlin/com/embabel/dice/storage/AbstractExtractionRunStoreContractTest.kt @@ -0,0 +1,1968 @@ +/* + * Copyright 2024-2026 Embabel Pty Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.embabel.dice.storage + +import com.embabel.agent.core.ContextId +import com.embabel.dice.proposition.extraction.ExtractionActorRef +import com.embabel.dice.proposition.extraction.ExtractionCohortRef +import com.embabel.dice.proposition.extraction.ExtractionContentProfileRef +import com.embabel.dice.proposition.extraction.ExtractionExperimentRef +import com.embabel.dice.proposition.extraction.ExtractionFailure +import com.embabel.dice.proposition.extraction.ExtractionFailureCode +import com.embabel.dice.proposition.extraction.ExtractionFailureStage +import com.embabel.dice.proposition.extraction.ExtractionInvocationId +import com.embabel.dice.proposition.extraction.ExtractionInvocationOutcome +import com.embabel.dice.proposition.extraction.ExtractionInvocationRecord +import com.embabel.dice.proposition.extraction.ExtractionModelUsage +import com.embabel.dice.proposition.extraction.ExtractionProviderResponseFacts +import com.embabel.dice.proposition.extraction.ExtractionReplayFidelity +import com.embabel.dice.proposition.extraction.ExtractionRequestedModelConfig +import com.embabel.dice.proposition.extraction.ExtractionRun +import com.embabel.dice.proposition.extraction.ExtractionRunConflictException +import com.embabel.dice.proposition.extraction.ExtractionRunCounts +import com.embabel.dice.proposition.extraction.ExtractionRunFingerprints +import com.embabel.dice.proposition.extraction.ExtractionRunKey +import com.embabel.dice.proposition.extraction.ExtractionRunLineage +import com.embabel.dice.proposition.extraction.ExtractionRunNotFoundException +import com.embabel.dice.proposition.extraction.ExtractionRunRef +import com.embabel.dice.proposition.extraction.ExtractionRunStatus +import com.embabel.dice.proposition.extraction.ExtractionRunStore +import com.embabel.dice.proposition.extraction.ExtractionRunSubjectRefs +import com.embabel.dice.proposition.extraction.ExtractionRunTransition +import com.embabel.dice.proposition.extraction.ExtractionRunTransitionOutcome +import com.embabel.dice.proposition.extraction.ExtractionRuntimeIdentity +import com.embabel.dice.provenance.SourceRevisionRef +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertNull +import org.junit.jupiter.api.Assertions.assertThrows +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test +import java.time.Instant + +/** + * Cross-backend contract for [ExtractionRunStore]: the lifecycle state machine, terminal-write + * idempotency, and the scoping and bounding rules on every read. Each subclass supplies a store and + * inherits the whole suite, so a backend that disagrees with the in-memory reference fails at + * authoring time instead of in production. + * + * The cases here are the ones a durable backend gets wrong in a way a single-backend test would + * miss. A `MERGE … SET` upsert passes "a terminal write is recorded" and fails "an incompatible + * terminal rewrite is rejected", because overwriting is what MERGE does. A finder that filters in + * memory passes every single-tenant read and fails "a page scopes before it limits". A chain walk + * written as a recursive Cypher pattern passes on a healthy graph and hangs on a cycle. + */ +abstract class AbstractExtractionRunStoreContractTest { + + /** A store holding nothing for the tenants below. */ + protected abstract fun store(): ExtractionRunStore + + private val tenant = ContextId("contract-tenant") + private val neighbour = ContextId("contract-neighbour") + private val startedAt: Instant = Instant.parse("2026-08-31T10:15:30Z") + private val finishedAt: Instant = Instant.parse("2026-08-31T10:15:47Z") + + private fun running( + runId: String, + contextId: ContextId = tenant, + startedAt: Instant = this.startedAt, + lineage: ExtractionRunLineage = ExtractionRunLineage.root(ExtractionRunRef(runId)), + ): ExtractionRun = ExtractionRun( + contextId = contextId, + lineage = lineage, + status = ExtractionRunStatus.RUNNING, + startedAt = startedAt, + ) + + private fun key(runId: String, contextId: ContextId = tenant) = + ExtractionRunKey(contextId, ExtractionRunRef(runId)) + + // ---- the state machine ---- + + @Test + fun `a saved run starts running and is readable by its key`() { + val store = store() + val run = running("contract-start") + + store.save(run) + + assertEquals(run, store.findRun(run.key())) + assertEquals(ExtractionRunStatus.RUNNING, store.findRun(run.key())?.status) + } + + @Test + fun `a first save must name version 0`() { + val store = store() + val run = ExtractionRun( + contextId = tenant, + lineage = ExtractionRunLineage.root(ExtractionRunRef("contract-first-save-version")), + status = ExtractionRunStatus.RUNNING, + startedAt = startedAt, + version = 7, + ) + + val thrown = assertThrows(IllegalArgumentException::class.java) { store.save(run) } + assertTrue(thrown.message.orEmpty().contains("version 0")) + assertNull(store.findRun(run.key())) + } + + @Test + fun `each terminal edge is reachable from running`() { + listOf( + ExtractionRunStatus.COMPLETED to ExtractionRunTransition.completed(finishedAt), + ExtractionRunStatus.FAILED to ExtractionRunTransition.failed(finishedAt), + ExtractionRunStatus.CANCELLED to ExtractionRunTransition.cancelled(finishedAt), + ).forEach { (expected, transition) -> + val store = store() + val run = running("contract-edge-$expected") + store.save(run) + + val result = store.transition(run.key(), transition) + + assertEquals(ExtractionRunTransitionOutcome.APPLIED, result.outcome) + assertEquals(expected, result.run.status) + assertEquals(finishedAt, result.run.finishedAt) + assertEquals(expected, store.findRun(run.key())?.status) + } + } + + @Test + fun `a terminal status cannot enter through save`() { + val store = store() + val completed = ExtractionRun( + contextId = tenant, + lineage = ExtractionRunLineage.root(ExtractionRunRef("contract-direct")), + status = ExtractionRunStatus.COMPLETED, + startedAt = startedAt, + finishedAt = finishedAt, + ) + + assertThrows(IllegalArgumentException::class.java) { store.save(completed) } + assertNull(store.findRun(completed.key())) + } + + @Test + fun `no edge leaves a terminal state`() { + val terminals = listOf( + ExtractionRunTransition.completed(finishedAt), + ExtractionRunTransition.failed(finishedAt), + ExtractionRunTransition.cancelled(finishedAt), + ) + + terminals.forEach { first -> + val store = store() + val run = running("contract-matrix-${first.status}") + store.save(run) + store.transition(run.key(), first) + + terminals.filter { it.status != first.status }.forEach { second -> + assertThrows(ExtractionRunConflictException::class.java) { + store.transition(run.key(), second) + } + } + assertEquals(first.status, store.findRun(run.key())?.status) + } + } + + @Test + fun `a terminal run is not re-openable and takes no more records`() { + val store = store() + val run = running("contract-closed") + store.save(run) + store.transition(run.key(), ExtractionRunTransition.completed(finishedAt)) + + assertThrows(ExtractionRunConflictException::class.java) { store.save(run) } + assertThrows(ExtractionRunConflictException::class.java) { + store.recordInvocation(run.key(), ExtractionInvocationRecord.planned(0)) + } + assertEquals(ExtractionRunStatus.COMPLETED, store.findRun(run.key())?.status) + } + + @Test + fun `a terminal write against a run nobody started is rejected`() { + val store = store() + + assertThrows(ExtractionRunNotFoundException::class.java) { + store.transition(key("contract-absent"), ExtractionRunTransition.completed(finishedAt)) + } + } + + @Test + fun `an empty run completes`() { + val store = store() + val run = running("contract-empty") + store.save(run) + + val result = store.transition(run.key(), ExtractionRunTransition.completed(finishedAt)) + + assertEquals(ExtractionRunStatus.COMPLETED, result.run.status) + assertTrue(result.run.invocations.isEmpty()) + assertTrue(store.invocationsOf(run.key()).isEmpty()) + } + + // ---- idempotency ---- + + @Test + fun `the same terminal write replays as success`() { + val store = store() + val run = running("contract-replay") + store.save(run) + val transition = ExtractionRunTransition.completed( + finishedAt = finishedAt, + counts = ExtractionRunCounts(propositionsPersisted = 7), + ) + + val first = store.transition(run.key(), transition) + val second = store.transition(run.key(), transition) + + assertEquals(ExtractionRunTransitionOutcome.APPLIED, first.outcome) + assertEquals(ExtractionRunTransitionOutcome.REPLAYED, second.outcome) + assertEquals(first.run, second.run) + // Both calls could return matching objects while the replay quietly rewrote the stored row. + assertEquals(first.run, store.findRun(run.key())) + } + + @Test + fun `an incompatible terminal rewrite is rejected and changes nothing`() { + val store = store() + val run = running("contract-rewrite") + store.save(run) + val recorded = ExtractionRunTransition.completed( + finishedAt = finishedAt, + counts = ExtractionRunCounts(propositionsPersisted = 7), + ) + val applied = store.transition(run.key(), recorded).run + + assertThrows(ExtractionRunConflictException::class.java) { + store.transition(run.key(), ExtractionRunTransition.failed(finishedAt)) + } + assertThrows(ExtractionRunConflictException::class.java) { + store.transition( + run.key(), + ExtractionRunTransition.completed( + finishedAt = finishedAt, + counts = ExtractionRunCounts(propositionsPersisted = 8), + ), + ) + } + assertEquals(applied, store.findRun(run.key())) + } + + @Test + fun `a retry differing only in finish time is an incompatible rewrite, for every terminal status`() { + // docs/design/extraction-runs.md:432 promises finish time participates in the fingerprint + // like every other terminal field: minting a fresh finishedAt on a retry produces an + // incompatible rewrite that gets rejected. Every other terminal test either holds finish + // time fixed while varying status or counts, or varies finish time alongside counts, so + // none of them would notice a comparator that ignores finish time. Holding status and + // counts fixed here and varying only finishedAt is what isolates the promise, for each + // terminal status in turn. + val terminalFactories: List ExtractionRunTransition>> = listOf( + "COMPLETED" to { at, counts -> ExtractionRunTransition.completed(at, counts = counts) }, + "FAILED" to { at, counts -> ExtractionRunTransition.failed(at, counts = counts) }, + "CANCELLED" to { at, counts -> ExtractionRunTransition.cancelled(at, counts = counts) }, + ) + terminalFactories.forEach { (statusName, transitionOf) -> + val store = store() + val run = running("contract-finish-only-$statusName") + store.save(run) + val counts = ExtractionRunCounts(propositionsPersisted = 7) + val applied = store.transition(run.key(), transitionOf(finishedAt, counts)).run + + assertThrows( + ExtractionRunConflictException::class.java, + { store.transition(run.key(), transitionOf(finishedAt.plusSeconds(1), counts)) }, + "a $statusName retry naming a different finish time and nothing else must be rejected", + ) + assertEquals( + applied, + store.findRun(run.key()), + "the rejected finish-only $statusName retry must not have changed the persisted run, finish time included", + ) + } + } + + @Test + fun `a replay after an interleaved invocation record is still a replay`() { + // The discriminating case for what the fingerprint covers. A backend that re-derived the + // digest from the stored run rather than recording the string the transition computed sees + // a run whose invocation list has grown, produces a different digest, and rejects a correct + // retry as an incompatible rewrite. It passes every other case in this suite. + val store = store() + val run = running("contract-interleaved") + store.save(run) + store.recordInvocation(run.key(), ExtractionInvocationRecord.planned(0)) + val transition = ExtractionRunTransition.completed(finishedAt) + val applied = store.transition(run.key(), transition) + + val replayed = store.transition(run.key(), transition) + assertEquals(ExtractionRunTransitionOutcome.REPLAYED, replayed.outcome) + // The outcome alone does not prove the replay left storage untouched. + assertEquals(applied.run, store.findRun(run.key())) + } + + @Test + fun `a rebuilt terminal write with the same payload replays`() { + // A retry after a crash rebuilds the payload rather than keeping the object. The comparison + // is on the fingerprint, so it still replays — and a backend comparing object identity or a + // stored timestamp of the write would fail here. + val store = store() + val run = running("contract-rebuilt") + store.save(run) + val counts = ExtractionRunCounts(propositionsPersisted = 7) + val applied = store.transition(run.key(), ExtractionRunTransition.completed(finishedAt, counts = counts)) + + val replayed = store.transition( + run.key(), + ExtractionRunTransition.completed( + finishedAt, + counts = ExtractionRunCounts(propositionsPersisted = 7), + ), + ) + assertEquals(ExtractionRunTransitionOutcome.REPLAYED, replayed.outcome) + assertEquals(applied.run, store.findRun(run.key())) + } + + @Test + fun `keeping counts and replacing them with the same values are different terminal writes`() { + // Null means keep and a value means replace, so they are different claims even when they + // land on the same numbers. A backend that normalised null to the run's stored counts before + // fingerprinting would replay the second as the first. + val store = store() + val run = ExtractionRun( + contextId = tenant, + lineage = ExtractionRunLineage.root(ExtractionRunRef("contract-kept")), + status = ExtractionRunStatus.RUNNING, + startedAt = startedAt, + counts = ExtractionRunCounts(propositionsPersisted = 4), + ) + store.save(run) + val originalTransition = ExtractionRunTransition.completed(finishedAt, counts = null) + val kept = store.transition(run.key(), originalTransition).run + + assertThrows(ExtractionRunConflictException::class.java) { + store.transition( + run.key(), + ExtractionRunTransition.completed( + finishedAt, + counts = ExtractionRunCounts(propositionsPersisted = 4), + ), + ) + } + // The rejected second write must not have touched storage, even though it names the same + // numbers the first write kept. + assertEquals(kept, store.findRun(run.key())) + // findRun alone cannot tell which fingerprint is actually stored: both transitions produce + // the same visible counts, so a backend that recorded the rejected write's fingerprint + // internally, while still throwing and leaving the visible run alone, would pass the + // assertion above. Replaying the original transition surfaces the stored fingerprint + // directly: it must still be recognised as the same write that already landed. + assertEquals( + ExtractionRunTransitionOutcome.REPLAYED, + store.transition(run.key(), originalTransition).outcome, + ) + } + + @Test + fun `a terminal write through the store preserves every field it does not own, header version included`() { + // ExtractionRunTransition.applyTo is pinned field-by-field in dice's own + // ExtractionRunLifecycleTest; this pins the same promise through a store's transition(), + // which is what a backend that maps rows in and out actually has to get right. + val store = store() + val run = ExtractionRun( + contextId = tenant, + lineage = ExtractionRunLineage.root(ExtractionRunRef("contract-transition-carries")), + status = ExtractionRunStatus.RUNNING, + startedAt = startedAt, + profile = ExtractionContentProfileRef("house-style", "v3"), + sourceRevisions = listOf(SourceRevisionRef("uri:doc-a", "rev-11")), + fingerprints = ExtractionRunFingerprints( + promptTemplateFingerprint = "sha256:6d1f0a2b", + schemaFingerprint = "sha256:a7c40e19", + metamodelFingerprint = "sha256:0b93cc55", + ), + runtime = ExtractionRuntimeIdentity(extractor = "LlmPropositionExtractor", extractorVersion = "0.2.0"), + requestedModel = ExtractionRequestedModelConfig(requestedModel = "model-large", temperature = 0.2), + subjectRefs = ExtractionRunSubjectRefs(actor = ExtractionActorRef("actor:7f19aa02")), + experimentRef = ExtractionExperimentRef("exp:prompt-v3"), + cohortRef = ExtractionCohortRef("cohort:treatment"), + replayFidelity = ExtractionReplayFidelity.strongest(), + counts = ExtractionRunCounts(propositionsPersisted = 4), + ) + store.save(run) + // A genuine header change, accepted at the version the first save left, so the run this + // test transitions is not left at the version-0 default by coincidence. + val bumped = store.save( + ExtractionRun( + contextId = run.contextId, + lineage = run.lineage, + status = run.status, + startedAt = run.startedAt, + profile = run.profile, + sourceRevisions = run.sourceRevisions, + fingerprints = run.fingerprints, + runtime = run.runtime, + requestedModel = run.requestedModel, + subjectRefs = run.subjectRefs, + experimentRef = run.experimentRef, + cohortRef = run.cohortRef, + replayFidelity = run.replayFidelity, + counts = ExtractionRunCounts(propositionsPersisted = 9), + version = run.version, + ), + ) + // A bare planned() record has every observation field at its default, so a terminal mapper + // that erases outcome, service, timing, usage and provider response would satisfy the + // assertions below by coincidence. This one carries a value in every field transition() + // does not own, so the same mapper has something real to lose. + val recorded = store.recordInvocation( + run.key(), + ExtractionInvocationRecord( + id = ExtractionInvocationId.planned(0), + outcome = ExtractionInvocationOutcome.SUCCEEDED, + configuredService = "service-carried", + startedAt = startedAt, + finishedAt = finishedAt, + usage = ExtractionModelUsage(inputTokens = 42, outputTokens = 7), + providerResponse = ExtractionProviderResponseFacts(responseModel = "model-carried"), + ), + ) + + val terminal = store.transition( + run.key(), + ExtractionRunTransition.completed(finishedAt, counts = null, failures = null), + ).run + + assertEquals(ExtractionRunStatus.COMPLETED, terminal.status) + assertEquals(finishedAt, terminal.finishedAt) + assertEquals(run.lineage, terminal.lineage) + assertEquals(run.contextId, terminal.contextId) + assertEquals(run.startedAt, terminal.startedAt) + assertEquals(run.profile, terminal.profile) + assertEquals(run.sourceRevisions, terminal.sourceRevisions) + assertEquals(run.fingerprints, terminal.fingerprints) + assertEquals(run.runtime, terminal.runtime) + assertEquals(run.requestedModel, terminal.requestedModel) + assertEquals(run.subjectRefs, terminal.subjectRefs) + assertEquals(run.experimentRef, terminal.experimentRef) + assertEquals(run.cohortRef, terminal.cohortRef) + assertEquals(run.replayFidelity, terminal.replayFidelity) + assertEquals(bumped.counts, terminal.counts) + assertEquals(recorded.invocations, terminal.invocations) + assertEquals(bumped.version, terminal.version) + + // What transition() returns can diverge from what a durable backend actually wrote, so + // every field is checked again against an independent read. + val persisted = store.findRun(run.key())!! + assertEquals(ExtractionRunStatus.COMPLETED, persisted.status) + assertEquals(finishedAt, persisted.finishedAt) + assertEquals(run.lineage, persisted.lineage) + assertEquals(run.contextId, persisted.contextId) + assertEquals(run.startedAt, persisted.startedAt) + assertEquals(run.profile, persisted.profile) + assertEquals(run.sourceRevisions, persisted.sourceRevisions) + assertEquals(run.fingerprints, persisted.fingerprints) + assertEquals(run.runtime, persisted.runtime) + assertEquals(run.requestedModel, persisted.requestedModel) + assertEquals(run.subjectRefs, persisted.subjectRefs) + assertEquals(run.experimentRef, persisted.experimentRef) + assertEquals(run.cohortRef, persisted.cohortRef) + assertEquals(run.replayFidelity, persisted.replayFidelity) + assertEquals(bumped.counts, persisted.counts) + assertEquals(recorded.invocations, persisted.invocations) + assertEquals(bumped.version, persisted.version) + } + + @Test + fun `null counts and failures on a transition keep what the stored run held, and values replace them`() { + // The existing fingerprint-distinction test proves null and a value are different claims; + // this one proves what each claim actually leaves behind in the store, checked by reading + // back what is persisted, alongside what is returned. Both counts objects below set disjoint + // fields on purpose: a field-by-field merge + // would land on the same numbers a genuine keep produces for the "kept" case, but would + // splice old and new fields together for the "replaced" case, where only whole-value + // replacement clears the old fields entirely. + val originalCounts = ExtractionRunCounts(sourcesRead = 2, chunksProcessed = 5, propositionsExtracted = 3) + val accumulated = listOf(ExtractionFailure(ExtractionFailureCode.RATE_LIMITED, ExtractionFailureStage.MODEL_CALL)) + val kept = running("contract-transition-keeps").let { + ExtractionRun( + contextId = it.contextId, + lineage = it.lineage, + status = it.status, + startedAt = it.startedAt, + counts = originalCounts, + failures = accumulated, + ) + } + val storeA = store() + storeA.save(kept) + val terminalKept = storeA.transition( + kept.key(), + ExtractionRunTransition.completed(finishedAt, counts = null, failures = null), + ).run + assertEquals(originalCounts, terminalKept.counts) + assertEquals(accumulated, terminalKept.failures) + val persistedKept = storeA.findRun(kept.key())!! + assertEquals(originalCounts, persistedKept.counts) + assertEquals(accumulated, persistedKept.failures) + + val replaced = running("contract-transition-replaces").let { + ExtractionRun( + contextId = it.contextId, + lineage = it.lineage, + status = it.status, + startedAt = it.startedAt, + counts = originalCounts, + failures = accumulated, + ) + } + val replacementCounts = ExtractionRunCounts(propositionsPersisted = 7, entitiesResolved = 4) + val replacementFailures = listOf(ExtractionFailure(ExtractionFailureCode.DECODE_FAILED, ExtractionFailureStage.RESPONSE_DECODE)) + val storeB = store() + storeB.save(replaced) + val terminalReplaced = storeB.transition( + replaced.key(), + ExtractionRunTransition.completed( + finishedAt, + counts = replacementCounts, + failures = replacementFailures, + ), + ).run + assertEquals(replacementCounts, terminalReplaced.counts) + assertEquals(replacementFailures, terminalReplaced.failures) + val persistedReplaced = storeB.findRun(replaced.key())!! + assertEquals(replacementCounts, persistedReplaced.counts) + assertEquals(replacementFailures, persistedReplaced.failures) + } + + @Test + fun `a save that disagrees with the stored run's start time is rejected`() { + val store = store() + val run = running("contract-identity") + store.save(run) + + assertThrows(ExtractionRunConflictException::class.java) { + store.save(running("contract-identity", startedAt = startedAt.plusSeconds(30))) + } + assertEquals(run, store.findRun(run.key())) + } + + @Test + fun `a save that disagrees with the stored run's lineage is rejected`() { + // Separate from the start-time case on purpose: a backend guarding one and not the other + // passes a combined test that only varies the start time. + val store = store() + val run = running("contract-lineage") + store.save(run) + + assertThrows(ExtractionRunConflictException::class.java) { + store.save( + running( + "contract-lineage", + lineage = ExtractionRunLineage.root( + runRef = ExtractionRunRef("contract-lineage"), + supersedesRunRef = ExtractionRunRef("contract-something-else"), + ), + ), + ) + } + assertEquals(run, store.findRun(run.key())) + } + + @Test + fun `a running run carrying a finish time is rejected`() { + val store = store() + val finishedWhileRunning = ExtractionRun( + contextId = tenant, + lineage = ExtractionRunLineage.root(ExtractionRunRef("contract-finished-running")), + status = ExtractionRunStatus.RUNNING, + startedAt = startedAt, + finishedAt = finishedAt, + ) + + assertThrows(IllegalArgumentException::class.java) { store.save(finishedWhileRunning) } + assertNull(store.findRun(finishedWhileRunning.key())) + } + + @Test + fun `a header save embedding a brand-new invocation never creates the row`() { + // recordInvocation is the sole door onto invocation state. An id a save's payload names, + // that recordInvocation never wrote, must not appear afterward — a save cannot originate a + // row any more than it can update or delete one. + val store = store() + val run = running("contract-save-never-creates") + val saved = store.save(run) + val neverRecorded = ExtractionInvocationRecord(id = ExtractionInvocationId.planned(0)) + + store.save( + ExtractionRun( + contextId = tenant, + lineage = run.lineage, + status = ExtractionRunStatus.RUNNING, + startedAt = startedAt, + invocations = listOf(neverRecorded), + version = saved.version, + ), + ) + + assertTrue(store.invocationsOf(run.key()).isEmpty()) + assertTrue(store.findRun(run.key())!!.invocations.isEmpty()) + } + + @Test + fun `a header save embedding a changed invocation never updates the stored row`() { + val store = store() + val run = running("contract-save-never-updates") + val saved = store.save(run) + val id = ExtractionInvocationId.planned(0) + val original = ExtractionInvocationRecord(id = id, configuredService = "service-original") + store.recordInvocation(run.key(), original) + + store.save( + ExtractionRun( + contextId = tenant, + lineage = run.lineage, + status = ExtractionRunStatus.RUNNING, + startedAt = startedAt, + invocations = listOf(original.copy(configuredService = "service-changed-by-save")), + version = saved.version, + ), + ) + + val record = store.invocationsOf(run.key()).single { it.id == id } + assertEquals("service-original", record.configuredService) + } + + @Test + fun `a header save embedding an empty invocation list never deletes a stored row`() { + // A durable backend gets this for free — a header write never touches a child row — and an + // in-memory one that replaced the stored run wholesale would silently undo recordInvocation. + // The two have to agree, so the rule is asserted here rather than in one backend's own test. + val store = store() + val run = running("contract-child-rows") + store.save(run) + store.recordInvocation(run.key(), ExtractionInvocationRecord.planned(0)) + + val stale = ExtractionRun( + contextId = tenant, + lineage = run.lineage, + status = ExtractionRunStatus.RUNNING, + startedAt = startedAt, + counts = ExtractionRunCounts(propositionsExtracted = 5), + ) + val saved = store.save(stale) + + assertEquals(5, saved.counts.propositionsExtracted) + assertEquals( + listOf(ExtractionInvocationId(0, 1)), + store.invocationsOf(run.key()).map { it.id }, + ) + } + + @Test + fun `a stale header save carrying an old invocation snapshot leaves the newer stored invocation intact`() { + // The defect this contract closes: recordInvocation never moves the header's version, so a + // header save built on a run read before a later recordInvocation call landed can still name + // the version currently stored and be accepted. A merged write let that stale, non-empty + // invocation snapshot silently replace what the later call wrote; this contract's save never + // looks at the snapshot at all, so the newer row survives regardless. + val store = store() + val run = running("contract-stale-invocation-snapshot") + val saved = store.save(run) + val id = ExtractionInvocationId.planned(0) + store.recordInvocation( + run.key(), + ExtractionInvocationRecord(id = id, configuredService = "service-early"), + ) + val staleSnapshot = store.findRun(run.key())!! + + store.recordInvocation( + run.key(), + ExtractionInvocationRecord(id = id, configuredService = "service-current"), + ) + + // A caller holding the earlier snapshot saves a genuine header change, still naming the + // current version and still carrying the old invocation list. + store.save( + ExtractionRun( + contextId = tenant, + lineage = run.lineage, + status = ExtractionRunStatus.RUNNING, + startedAt = startedAt, + counts = ExtractionRunCounts(propositionsExtracted = 3), + invocations = staleSnapshot.invocations, + version = saved.version, + ), + ) + + val record = store.invocationsOf(run.key()).single { it.id == id } + assertEquals("service-current", record.configuredService) + } + + @Test + fun `a stale header save is rejected, and the header it read is left in place`() { + // Writer A reads the run at version 0, does real work, and saves what it found. That save + // is accepted and the header moves to version 1. Writer B read the run before any of that + // landed, so its own save still names version 0 — the version the store has already moved + // past. + val store = store() + val run = running("contract-stale-header") + val inserted = store.save(run) + assertEquals(0L, inserted.version) + + val profile = ExtractionContentProfileRef("profile-a", "v1") + val revision = SourceRevisionRef("source-a", "rev-1") + val failure = ExtractionFailure.of(ExtractionFailureCode.MODEL_TIMEOUT, ExtractionFailureStage.MODEL_CALL) + val advanced = ExtractionRun( + contextId = tenant, + lineage = run.lineage, + status = ExtractionRunStatus.RUNNING, + startedAt = startedAt, + profile = profile, + sourceRevisions = listOf(revision), + fingerprints = ExtractionRunFingerprints(promptTemplateFingerprint = "prompt-digest"), + counts = ExtractionRunCounts(propositionsPersisted = 12), + failures = listOf(failure), + version = 0, + ) + val savedByA = store.save(advanced) + assertEquals(1L, savedByA.version) + + val stale = running("contract-stale-header") // version defaults to 0, the read A made stale + assertThrows(ExtractionRunConflictException::class.java) { + store.save(stale) + } + + val current = store.findRun(run.key()) + assertEquals(profile, current?.profile) + assertEquals(listOf(revision), current?.sourceRevisions) + assertEquals("prompt-digest", current?.fingerprints?.promptTemplateFingerprint) + assertEquals(12, current?.counts?.propositionsPersisted) + assertEquals(listOf(failure), current?.failures) + assertEquals(1L, current?.version) + } + + @Test + fun `an accepted header save replaces the whole header at once`() { + // A field-by-field merge would keep the first profile, union the source revisions, and take + // the larger count. A whole-header replace does none of that: the second save's values win + // outright, proving the accepted write is not quietly combined with what came before it. + // Every field a save owns is varied here, covering the full set beyond the four most + // obvious ones, and the second save clears four nullable fields back to null and two + // non-nullable ones back to their empty default — the shape a field-by-field merge is + // most likely to get wrong, since "the new value is absent" and "keep what I don't + // mention" look the same to it. + val store = store() + val run = running("contract-header-replace") + val first = store.save(run) + + val firstUpdate = ExtractionRun( + contextId = tenant, + lineage = run.lineage, + status = ExtractionRunStatus.RUNNING, + startedAt = startedAt, + profile = ExtractionContentProfileRef("profile-old", "v1"), + sourceRevisions = listOf(SourceRevisionRef("source-a", "rev-1")), + fingerprints = ExtractionRunFingerprints(promptTemplateFingerprint = "prompt-old"), + runtime = ExtractionRuntimeIdentity(extractor = "extractor-old"), + requestedModel = ExtractionRequestedModelConfig(requestedModel = "model-old"), + subjectRefs = ExtractionRunSubjectRefs(actor = ExtractionActorRef("actor-old")), + experimentRef = ExtractionExperimentRef("exp-old"), + cohortRef = ExtractionCohortRef("cohort-old"), + replayFidelity = ExtractionReplayFidelity.APPROXIMATE, + counts = ExtractionRunCounts(propositionsPersisted = 12), + failures = listOf(ExtractionFailure.of(ExtractionFailureCode.MODEL_TIMEOUT, ExtractionFailureStage.MODEL_CALL)), + version = first.version, + ) + val second = store.save(firstUpdate) + assertEquals(1L, second.version) + + val secondUpdate = ExtractionRun( + contextId = tenant, + lineage = run.lineage, + status = ExtractionRunStatus.RUNNING, + startedAt = startedAt, + profile = null, + sourceRevisions = listOf(SourceRevisionRef("source-b", "rev-1")), + fingerprints = ExtractionRunFingerprints(promptTemplateFingerprint = "prompt-new"), + runtime = ExtractionRuntimeIdentity(), + requestedModel = null, + subjectRefs = ExtractionRunSubjectRefs(), + experimentRef = null, + cohortRef = null, + replayFidelity = ExtractionReplayFidelity.NONE, + counts = ExtractionRunCounts(propositionsPersisted = 3), + failures = emptyList(), + version = second.version, + ) + val third = store.save(secondUpdate) + + assertNull(third.profile) + assertEquals(listOf(SourceRevisionRef("source-b", "rev-1")), third.sourceRevisions) + assertEquals("prompt-new", third.fingerprints.promptTemplateFingerprint) + assertEquals(ExtractionRuntimeIdentity(), third.runtime) + assertNull(third.requestedModel) + assertEquals(ExtractionRunSubjectRefs(), third.subjectRefs) + assertNull(third.experimentRef) + assertNull(third.cohortRef) + assertEquals(ExtractionReplayFidelity.NONE, third.replayFidelity) + assertEquals(3, third.counts.propositionsPersisted) + assertEquals(emptyList(), third.failures) + assertEquals(2L, third.version) + + // save() could return the correctly assembled replacement while persisting a field-merged + // or under-versioned row, so what is actually stored is checked field for field too, + // including the ones that were cleared this time around. + val persisted = store.findRun(run.key())!! + assertEquals(third, persisted) + assertNull(persisted.profile) + assertEquals(ExtractionRuntimeIdentity(), persisted.runtime) + assertNull(persisted.requestedModel) + assertEquals(ExtractionRunSubjectRefs(), persisted.subjectRefs) + assertNull(persisted.experimentRef) + assertNull(persisted.cohortRef) + assertEquals(ExtractionReplayFidelity.NONE, persisted.replayFidelity) + } + + @Test + fun `a save whose content already matches what is stored replays as a no-op at a stale version`() { + // The promise: a byte-identical resend is a no-op regardless of the version it names. A + // caller retrying a save it never learned had landed must not be told it conflicted. + val store = store() + val run = running("contract-replay-stale-version") + val inserted = store.save(run) + + val advanced = ExtractionRun( + contextId = tenant, + lineage = run.lineage, + status = ExtractionRunStatus.RUNNING, + startedAt = startedAt, + counts = ExtractionRunCounts(propositionsPersisted = 1), + version = inserted.version, + ) + val afterAdvance = store.save(advanced) + assertEquals(1L, afterAdvance.version) + + // The caller's own copy still names the version it read the run at before the update + // above landed, but the content it holds is exactly what that update produced. + val resend = ExtractionRun( + contextId = tenant, + lineage = run.lineage, + status = ExtractionRunStatus.RUNNING, + startedAt = startedAt, + counts = ExtractionRunCounts(propositionsPersisted = 1), + version = inserted.version, + ) + val saved = store.save(resend) + + assertEquals(afterAdvance.version, saved.version) + assertEquals(afterAdvance, saved) + // save() could hand back the correct no-op value while a check-then-write backend actually + // re-persisted the row, moving its generation. + assertEquals(afterAdvance, store.findRun(run.key())) + } + + @Test + fun `a stale-version resend replays as a no-op even with a terminal invocation recorded since`() { + // A stale-version header resend still has to be recognised as a no-op on its header content + // alone. What a save's own invocations field carries is not part of that comparison at all + // now, so a terminal child record recorded since the resend was built plays no part in the + // decision either way. + val store = store() + val run = running("contract-replay-stale-version-with-terminal-child") + store.save(run) + val invocationId = ExtractionInvocationId.planned(0) + val terminal = ExtractionInvocationRecord( + id = invocationId, + outcome = ExtractionInvocationOutcome.SUCCEEDED, + configuredService = "service-alpha", + ) + store.recordInvocation(run.key(), terminal) + + val advanced = store.save( + ExtractionRun( + contextId = tenant, + lineage = run.lineage, + status = ExtractionRunStatus.RUNNING, + startedAt = startedAt, + counts = ExtractionRunCounts(propositionsPersisted = 1), + version = 0L, + ), + ) + assertEquals(1L, advanced.version) + assertEquals(listOf(terminal), advanced.invocationsInPlanOrder()) + + // The stale resend: version 0, header content identical to what is stored, and an + // invocations field the store will not even look at. + val resend = ExtractionRun( + contextId = tenant, + lineage = run.lineage, + status = ExtractionRunStatus.RUNNING, + startedAt = startedAt, + counts = ExtractionRunCounts(propositionsPersisted = 1), + version = 0L, + ) + val saved = store.save(resend) + + assertEquals(advanced.version, saved.version) + assertEquals(advanced, saved) + assertEquals(advanced, store.findRun(run.key())) + } + + @Test + fun `recording an invocation does not move the header version`() { + val store = store() + val run = running("contract-version-invocation") + val saved = store.save(run) + + store.recordInvocation(run.key(), ExtractionInvocationRecord.planned(0)) + + assertEquals(saved.version, store.findRun(run.key())?.version) + + // A save built on the header as it stood before that recording still names the current + // version and is accepted; the recorded attempt survives because save never touches + // invocation rows at all. + val update = ExtractionRun( + contextId = tenant, + lineage = run.lineage, + status = ExtractionRunStatus.RUNNING, + startedAt = startedAt, + counts = ExtractionRunCounts(propositionsExtracted = 5), + version = saved.version, + ) + val afterUpdate = store.save(update) + + assertEquals(5, afterUpdate.counts.propositionsExtracted) + assertEquals(listOf(ExtractionInvocationId(0, 1)), afterUpdate.invocations.map { it.id }) + } + + // ---- concurrency ---- + + @Test + fun `two threads racing to end one run produce exactly one applied transition`() { + // Compare-and-set is what the contract is named for, and a backend delivers it from its + // transaction rather than from anything in this suite. A read-then-write with no isolation + // passes every sequential case above and produces two APPLIED here, or one APPLIED and one + // spurious conflict. + repeat(20) { attempt -> + val store = store() + val run = running("contract-race-$attempt") + store.save(run) + val transition = ExtractionRunTransition.completed(finishedAt) + + val start = java.util.concurrent.CountDownLatch(1) + val pool = java.util.concurrent.Executors.newFixedThreadPool(2) + try { + val outcomes = (0 until 2).map { + pool.submit { + start.await() + runCatching { store.transition(run.key(), transition).outcome } + .getOrElse { it } + } + } + start.countDown() + val results = outcomes.map { it.get() } + + assertEquals( + 1, + results.count { it == ExtractionRunTransitionOutcome.APPLIED }, + "exactly one thread ends the run: $results", + ) + assertEquals( + 1, + results.count { it == ExtractionRunTransitionOutcome.REPLAYED }, + "the loser replays rather than conflicting or applying: $results", + ) + } finally { + pool.shutdownNow() + } + // Counting outcomes proves one thread applied and one replayed; it does not prove the + // persisted row is what the transition actually derives. The comparison is against + // applyTo's own pure computation, independent of any store, so a race that corrupts the + // write under contention — landing a status without its matching finishedAt, say — is + // caught even though the outcome counts above still look right. + assertEquals(transition.applyTo(run), store.findRun(run.key())) + } + } + + @Test + fun `two threads racing to save the same header produce exactly one accepted write`() { + // The version compare has to be atomic. A check-then-write backend with no isolation lets + // both threads read version 0, both pass the comparison, and both land. + repeat(20) { attempt -> + val store = store() + val run = running("contract-race-save-$attempt") + val inserted = store.save(run) + + val start = java.util.concurrent.CountDownLatch(1) + val pool = java.util.concurrent.Executors.newFixedThreadPool(2) + try { + val submitted = (0 until 2).map { thread -> + pool.submit> { + start.await() + val update = ExtractionRun( + contextId = tenant, + lineage = run.lineage, + status = ExtractionRunStatus.RUNNING, + startedAt = startedAt, + counts = ExtractionRunCounts(propositionsPersisted = thread + 1), + version = inserted.version, + ) + runCatching { store.save(update) } + } + } + start.countDown() + val results = submitted.map { it.get() } + + assertEquals(1, results.count { it.isSuccess }, "one save is accepted: $results") + assertEquals( + 1, + results.count { it.exceptionOrNull() is ExtractionRunConflictException }, + "the other names a version the accepted save already moved past: $results", + ) + // Counting successes proves one save landed; it does not prove which content is the + // one actually stored. A backend that silently applied the losing thread's counts and + // reported the winner's version would still pass every assertion above. + val winner = results.single { it.isSuccess }.getOrThrow() + assertEquals(winner, store.findRun(run.key())) + } finally { + pool.shutdownNow() + } + assertEquals(1L, store.findRun(run.key())?.version) + } + } + + @Test + fun `two threads racing to record conflicting terminal outcomes for one attempt produce exactly one accepted write`() { + repeat(20) { attempt -> + val store = store() + val run = running("contract-race-record-$attempt") + store.save(run) + val id = ExtractionInvocationId.planned(0) + + val start = java.util.concurrent.CountDownLatch(1) + val pool = java.util.concurrent.Executors.newFixedThreadPool(2) + try { + val outcomes = listOf( + ExtractionInvocationOutcome.SUCCEEDED, + ExtractionInvocationOutcome.FAILED, + ) + val submitted = outcomes.map { outcome -> + pool.submit> { + start.await() + val record = ExtractionInvocationRecord( + id = id, + outcome = outcome, + startedAt = startedAt, + finishedAt = finishedAt, + ) + runCatching { store.recordInvocation(run.key(), record) } + } + } + start.countDown() + val results = submitted.map { it.get() } + + assertEquals(1, results.count { it.isSuccess }, "one terminal write lands: $results") + assertEquals( + 1, + results.count { it.exceptionOrNull() is ExtractionRunConflictException }, + "the other meets an attempt already terminal under a different outcome: $results", + ) + // Counting a success and a conflict proves one write landed; it does not prove which + // outcome, SUCCEEDED or FAILED, is the one actually stored. A backend that applied + // the losing outcome and reported conflict for the winner would still pass both + // assertions above. + val winner = results.single { it.isSuccess }.getOrThrow() + assertEquals( + winner.invocationsInPlanOrder().single { it.id == id }, + store.invocationsOf(run.key()).single { it.id == id }, + ) + } finally { + pool.shutdownNow() + } + assertEquals(1, store.invocationsOf(run.key()).size) + } + } + + @Test + fun `a header save racing with a recordInvocation write settles independently and neither is lost`() { + // save no longer contends for invocation state at all, so a header save and a + // recordInvocation write racing on the same run never conflict with each other — each + // settles on its own key. The header change and the invocation write both land. + repeat(20) { attempt -> + val store = store() + val run = running("contract-race-save-record-$attempt") + val inserted = store.save(run) + val id = ExtractionInvocationId.planned(0) + + val start = java.util.concurrent.CountDownLatch(1) + val pool = java.util.concurrent.Executors.newFixedThreadPool(2) + try { + val viaSave = pool.submit { + start.await() + val header = ExtractionRun( + contextId = tenant, + lineage = run.lineage, + status = ExtractionRunStatus.RUNNING, + startedAt = startedAt, + counts = ExtractionRunCounts(propositionsPersisted = 1), + version = inserted.version, + ) + store.save(header) + } + val viaRecord = pool.submit { + start.await() + val record = ExtractionInvocationRecord( + id = id, + outcome = ExtractionInvocationOutcome.FAILED, + startedAt = startedAt, + finishedAt = finishedAt, + ) + store.recordInvocation(run.key(), record) + record + } + start.countDown() + val savedHeader = viaSave.get() + val recordedInvocation = viaRecord.get() + + assertEquals(1, savedHeader.counts.propositionsPersisted) + assertEquals( + recordedInvocation, + store.invocationsOf(run.key()).single { it.id == id }, + ) + assertEquals(1, store.findRun(run.key())?.counts?.propositionsPersisted) + } finally { + pool.shutdownNow() + } + } + } + + @Test + fun `two concurrent IN_FLIGHT writers on different attempts both land, losing neither`() { + // Each invocation row is decided on its own (invocationIndex, attempt) key, so two writers + // recording different attempts contend for nothing shared: each is decided purely against + // its own row, on a key entirely independent of the header generation. Both writes have to + // survive. + repeat(20) { attempt -> + val store = store() + val run = running("contract-race-disjoint-attempts-$attempt") + store.save(run) + + val start = java.util.concurrent.CountDownLatch(1) + val pool = java.util.concurrent.Executors.newFixedThreadPool(2) + try { + val first = pool.submit { + start.await() + val record = ExtractionInvocationRecord( + id = ExtractionInvocationId.planned(0), + configuredService = "service-zero", + startedAt = startedAt, + ) + store.recordInvocation(run.key(), record) + record + } + val second = pool.submit { + start.await() + val record = ExtractionInvocationRecord( + id = ExtractionInvocationId.planned(1), + configuredService = "service-one", + startedAt = startedAt, + ) + store.recordInvocation(run.key(), record) + record + } + start.countDown() + val expected = listOf(first.get(), second.get()).sortedBy { it.id.invocationIndex } + + assertEquals( + expected, + store.invocationsOf(run.key()), + "both attempts must land, and neither writer's facts may be lost", + ) + } finally { + pool.shutdownNow() + } + } + } + + // ---- invocation records ---- + + @Test + fun `records are keyed by invocation and attempt and read back in plan order`() { + val store = store() + val run = running("contract-records") + store.save(run) + val first = ExtractionInvocationRecord(id = ExtractionInvocationId.planned(1)) + + listOf(2, 0).forEach { store.recordInvocation(run.key(), ExtractionInvocationRecord.planned(it)) } + store.recordInvocation(run.key(), first) + store.recordInvocation(run.key(), first) + store.recordInvocation(run.key(), first.retry()) + + assertEquals( + listOf( + ExtractionInvocationId(0, 1), + ExtractionInvocationId(1, 1), + ExtractionInvocationId(1, 2), + ExtractionInvocationId(2, 1), + ), + store.invocationsOf(run.key()).map { it.id }, + ) + } + + @Test + fun `a changed IN_FLIGHT record updates in place through recordInvocation, clearing an omitted fact`() { + // The existing plan-order test replays the identical object twice, and adding a new fact on + // top of an old one cannot tell field-merging apart from whole-record replacement — both + // land on the same value for a field present in both writes. Omitting startedAt, usage and + // providerResponse on the second write is the case that discriminates: only whole-record + // replacement clears them: a backend merging fields would keep the first write's values + // because the second write never mentioned them. Read back through invocationsOf — the + // read path a durable backend actually has to get right — independent of the object + // recordInvocation happens to return. + val store = store() + val run = running("contract-inflight-update") + store.save(run) + val id = ExtractionInvocationId.planned(0) + store.recordInvocation( + run.key(), + ExtractionInvocationRecord( + id = id, + configuredService = "service-alpha", + startedAt = startedAt, + usage = ExtractionModelUsage(inputTokens = 10, outputTokens = 5), + providerResponse = ExtractionProviderResponseFacts(responseModel = "model-alpha"), + ), + ) + + store.recordInvocation(run.key(), ExtractionInvocationRecord(id = id, configuredService = "service-beta")) + + val record = store.invocationsOf(run.key()).single { it.id == id } + assertEquals("service-beta", record.configuredService) + assertNull(record.startedAt) + assertNull(record.usage) + assertNull(record.providerResponse) + assertEquals(1, store.invocationsOf(run.key()).count { it.id == id }) + } + + @Test + fun `recording against a run nobody started is rejected`() { + val store = store() + + assertThrows(ExtractionRunNotFoundException::class.java) { + store.recordInvocation(key("contract-absent"), ExtractionInvocationRecord.planned(0)) + } + } + + @Test + fun `a delayed IN_FLIGHT write does not replace a terminal record for the same attempt`() { + val store = store() + val run = running("contract-invocation-terminal") + store.save(run) + val id = ExtractionInvocationId.planned(0) + val terminal = ExtractionInvocationRecord( + id = id, + outcome = ExtractionInvocationOutcome.SUCCEEDED, + startedAt = startedAt, + finishedAt = finishedAt, + ) + store.recordInvocation(run.key(), terminal) + + // The call that raced ahead of the terminal write finally lands. It must not put the + // attempt back to IN_FLIGHT and erase what the terminal write recorded. + val delayed = ExtractionInvocationRecord(id = id) + + assertThrows(ExtractionRunConflictException::class.java) { + store.recordInvocation(run.key(), delayed) + } + assertEquals(listOf(terminal), store.invocationsOf(run.key())) + } + + @Test + fun `repeating the same terminal invocation write is idempotent`() { + // Carries a full providerResponse, so the replay round-trips that field through + // recordInvocation unchanged too, alongside the other fields the invocation tests touch. + val store = store() + val run = running("contract-invocation-replay") + store.save(run) + val id = ExtractionInvocationId.planned(0) + val terminal = ExtractionInvocationRecord( + id = id, + outcome = ExtractionInvocationOutcome.FAILED, + startedAt = startedAt, + finishedAt = finishedAt, + providerResponse = ExtractionProviderResponseFacts( + responseModel = "model-a", + responseId = "resp-1", + finishReason = "stop", + systemFingerprint = "fp-1", + ), + ) + store.recordInvocation(run.key(), terminal) + + val replayed = store.recordInvocation(run.key(), terminal) + + assertEquals(listOf(terminal), replayed.invocationsInPlanOrder()) + assertEquals(listOf(terminal), store.invocationsOf(run.key())) + assertEquals(terminal.providerResponse, store.invocationsOf(run.key()).single().providerResponse) + } + + @Test + fun `a terminal record's identical replay through recordInvocation lands back at its stored position`() { + // docs/design/extraction-runs.md:302 promises an identical replay lands back at the + // position the stored record already held. The idempotent-replay test above holds only + // one record, where position is unobservable: replaying the only entry in a one-element + // list cannot tell "stayed in place" apart from "removed and appended", since both produce + // the same list. A backend that gets save's multi-record case right could still implement + // recordInvocation's own replay as a remove-and-append. Three records, replaying the + // first one, is what makes that reorder visible. + val store = store() + val run = running("contract-record-replay-position") + store.save(run) + val first = ExtractionInvocationRecord( + id = ExtractionInvocationId.planned(0), + outcome = ExtractionInvocationOutcome.SUCCEEDED, + startedAt = startedAt, + finishedAt = finishedAt, + ) + val second = ExtractionInvocationRecord(id = ExtractionInvocationId.planned(1)) + val third = ExtractionInvocationRecord(id = ExtractionInvocationId.planned(2)) + store.recordInvocation(run.key(), first) + store.recordInvocation(run.key(), second) + store.recordInvocation(run.key(), third) + + store.recordInvocation(run.key(), first) + + val persisted = store.findRun(run.key())!! + assertEquals(listOf(first, second, third), persisted.invocations) + } + + @Test + fun `a same-outcome write that differs from a terminal record is rejected too`() { + // A different outcome is one way to erase what a terminal record holds. A delayed duplicate + // claiming the same outcome, missing the usage and provider facts the first write actually + // carried, would erase them just as surely if it were allowed to update in place. + val store = store() + val run = running("contract-invocation-sparse") + store.save(run) + val id = ExtractionInvocationId.planned(0) + val terminal = ExtractionInvocationRecord( + id = id, + outcome = ExtractionInvocationOutcome.SUCCEEDED, + configuredService = "service-alpha", + startedAt = startedAt, + finishedAt = finishedAt, + usage = ExtractionModelUsage(inputTokens = 100, outputTokens = 20), + ) + store.recordInvocation(run.key(), terminal) + + val sparse = ExtractionInvocationRecord(id = id, outcome = ExtractionInvocationOutcome.SUCCEEDED) + + assertThrows(ExtractionRunConflictException::class.java) { + store.recordInvocation(run.key(), sparse) + } + assertEquals(listOf(terminal), store.invocationsOf(run.key())) + } + + @Test + fun `a CANCELLED terminal record is locked the same as SUCCEEDED and FAILED, through recordInvocation`() { + // ExtractionRunStore.kt:204 locks SUCCEEDED, FAILED and CANCELLED alike, but every other + // terminal-lock test in this suite happens to store a SUCCEEDED or FAILED record. A backend + // that locks only those two outcomes and leaves CANCELLED writable would pass every one of + // them. + val store = store() + val run = running("contract-invocation-cancelled-record") + store.save(run) + val id = ExtractionInvocationId.planned(0) + val terminal = ExtractionInvocationRecord( + id = id, + outcome = ExtractionInvocationOutcome.CANCELLED, + configuredService = "service-alpha", + startedAt = startedAt, + finishedAt = finishedAt, + ) + store.recordInvocation(run.key(), terminal) + + assertThrows(ExtractionRunConflictException::class.java) { + store.recordInvocation(run.key(), terminal.copy(configuredService = "service-beta")) + } + assertEquals(listOf(terminal), store.invocationsOf(run.key())) + } + + // ---- per-field delta matrix: a field being present in a record is not the same as a test + // discriminating on it. Every case below changes exactly one field from a fully-populated stored + // record and asserts the one outcome that field's difference must produce, through every door + // that can write it. A backend comparing or copying only some fields still passes a test that + // varies several fields together; it fails here, because nothing except the field under test + // differs from what is already stored. ---- + + private fun richTerminalInvocation(id: ExtractionInvocationId) = ExtractionInvocationRecord( + id = id, + outcome = ExtractionInvocationOutcome.SUCCEEDED, + configuredService = "service-base", + startedAt = startedAt, + finishedAt = finishedAt, + usage = ExtractionModelUsage(inputTokens = 10, outputTokens = 5), + providerResponse = ExtractionProviderResponseFacts(responseModel = "model-base"), + ) + + private fun richInFlightInvocation(id: ExtractionInvocationId) = ExtractionInvocationRecord( + id = id, + outcome = ExtractionInvocationOutcome.IN_FLIGHT, + configuredService = "service-base", + startedAt = startedAt, + usage = ExtractionModelUsage(inputTokens = 10, outputTokens = 5), + providerResponse = ExtractionProviderResponseFacts(responseModel = "model-base"), + ) + + /** Every field a terminal write's equality check must compare, one at a time — each as a + * non-null-to-non-null change and, where constructible, as a non-null-to-null clearing, since + * an `incoming ?: stored` implementation passes the first shape and only the second exposes it: + * `?:` only substitutes when the incoming side is null, so a non-null incoming value reaches + * the comparison either way. `finishedAt` differing alone is a legal record, since nothing but + * the terminal-lock check itself ties it to `outcome`, and it may clear to null even on a + * terminal record — nothing requires a terminal write to have finished at a known time. + * `startedAt` has no clearing entry in this table, but it is not exempt: this fixture's + * `finishedAt` is non-null, and `ExtractionInvocationRecord`'s own init check requires a + * non-null `startedAt` alongside a non-null `finishedAt`, so it is only this particular + * fixture that blocks the clearing entry here. `ExtractionInvocationRecord.kt` documents + * timing as an observation that a terminal record may lack entirely, so `startedAt` alone + * clearing against a terminal record whose `finishedAt` is already absent is a genuine, + * separately-covered case — see `a terminal record with absent timing rejects a resend that + * clears its start time` below, through both doors. `outcome` has no clearing entry and is + * the only field genuinely exempt from one: `val outcome: ExtractionInvocationOutcome` carries + * no `?`, so the type itself admits no null value to clear to. */ + private val terminalInvocationDeltas: List ExtractionInvocationRecord>> = listOf( + "outcome" to { r -> r.copy(outcome = ExtractionInvocationOutcome.FAILED) }, + "configuredService" to { r -> r.copy(configuredService = "service-changed") }, + "configuredService-cleared" to { r -> r.copy(configuredService = null) }, + "startedAt" to { r -> r.copy(startedAt = startedAt.plusSeconds(1)) }, + "finishedAt" to { r -> r.copy(finishedAt = finishedAt.plusSeconds(1)) }, + "finishedAt-cleared" to { r -> r.copy(finishedAt = null) }, + "usage" to { r -> r.copy(usage = ExtractionModelUsage(inputTokens = 99, outputTokens = 1)) }, + "usage-cleared" to { r -> r.copy(usage = null) }, + "providerResponse" to { r -> r.copy(providerResponse = ExtractionProviderResponseFacts(responseModel = "model-changed")) }, + "providerResponse-cleared" to { r -> r.copy(providerResponse = null) }, + ) + + /** The same shape for an IN_FLIGHT update, non-null-to-null clearing entries included for the + * same `incoming ?: stored` reason. `finishedAt` is absent here, and the reason holds up as a + * genuine exemption: `ExtractionInvocationRecord`'s own init block requires + * `outcome != IN_FLIGHT || finishedAt == null`, so as long as a record's `outcome` stays + * `IN_FLIGHT` its `finishedAt` cannot be anything but `null` — there is no non-null value here + * to clear, for any mutator that leaves `outcome` alone. Every other field, including terminal + * `finishedAt` and terminal `startedAt`, now has a discriminating test somewhere in this suite. + * `startedAt` clears here even though it could not in the terminal table above, since + * IN_FLIGHT's `finishedAt` is always null and imposes no constraint on it. `outcome` has no + * clearing entry, the other genuine exemption: its declared type carries no `?`, so the + * language itself admits no null value to construct it with. `ExtractionRun.contextId` is the + * one header field with the same shape of exemption: `key()` derives `ExtractionRunKey` from + * `contextId` directly, and the store's map is keyed by that same `ExtractionRunKey`, so a + * `stored` run retrieved via `runs[run.key()]` always shares `run`'s `contextId` by + * construction — there is no way to look up a `stored` whose `contextId` differs from the + * `run` used to find it. */ + private val inFlightInvocationDeltas: List ExtractionInvocationRecord>> = listOf( + "outcome" to { r -> r.copy(outcome = ExtractionInvocationOutcome.SUCCEEDED) }, + "configuredService" to { r -> r.copy(configuredService = "service-changed") }, + "configuredService-cleared" to { r -> r.copy(configuredService = null) }, + "startedAt" to { r -> r.copy(startedAt = startedAt.plusSeconds(1)) }, + "startedAt-cleared" to { r -> r.copy(startedAt = null) }, + "usage" to { r -> r.copy(usage = ExtractionModelUsage(inputTokens = 99, outputTokens = 1)) }, + "usage-cleared" to { r -> r.copy(usage = null) }, + "providerResponse" to { r -> r.copy(providerResponse = ExtractionProviderResponseFacts(responseModel = "model-changed")) }, + "providerResponse-cleared" to { r -> r.copy(providerResponse = null) }, + ) + + @Test + fun `a terminal record rejects a recordInvocation resend that differs in exactly one field`() { + terminalInvocationDeltas.forEach { (fieldName, mutate) -> + val store = store() + val run = running("contract-terminal-delta-record-$fieldName") + store.save(run) + val id = ExtractionInvocationId.planned(0) + val stored = richTerminalInvocation(id) + store.recordInvocation(run.key(), stored) + + assertThrows( + ExtractionRunConflictException::class.java, + { store.recordInvocation(run.key(), mutate(stored)) }, + "a resend differing only in $fieldName must be rejected through recordInvocation", + ) + assertEquals( + listOf(stored), + store.invocationsOf(run.key()), + "the rejected $fieldName-only resend must not have changed storage", + ) + } + } + + @Test + fun `a terminal record with absent timing rejects a recordInvocation resend that clears its start time`() { + // ExtractionInvocationRecord.kt:219 permits a terminal record with no timing at all — a + // SUCCEEDED attempt with no startedAt is constructible, because timing here is only ever + // an observation. The shared terminal-delta table above cannot reach this case: its + // fixture's finishedAt is set, and finishedAt requires startedAt whenever finishedAt is + // non-null, so nulling startedAt there would build a record the type itself refuses to + // construct. A record whose finishedAt is already absent carries no such requirement, and + // clearing its startedAt is exactly the resend an incoming.startedAt ?: stored.startedAt + // implementation reads as identical. + val store = store() + val run = running("contract-terminal-startedAt-clear-record") + store.save(run) + val id = ExtractionInvocationId.planned(0) + val stored = ExtractionInvocationRecord( + id = id, + outcome = ExtractionInvocationOutcome.SUCCEEDED, + startedAt = startedAt, + ) + store.recordInvocation(run.key(), stored) + + assertThrows(ExtractionRunConflictException::class.java) { + store.recordInvocation(run.key(), stored.copy(startedAt = null)) + } + assertEquals(listOf(stored), store.invocationsOf(run.key())) + } + + @Test + fun `an IN_FLIGHT record accepts a recordInvocation update that differs in exactly one field`() { + inFlightInvocationDeltas.forEach { (fieldName, mutate) -> + val store = store() + val run = running("contract-inflight-delta-record-$fieldName") + store.save(run) + val id = ExtractionInvocationId.planned(0) + val stored = richInFlightInvocation(id) + store.recordInvocation(run.key(), stored) + + val updated = mutate(stored) + store.recordInvocation(run.key(), updated) + + assertEquals( + listOf(updated), + store.invocationsOf(run.key()), + "a $fieldName-only update must fully replace the stored record through recordInvocation", + ) + } + } + + /** Builds a variant of [base] with exactly the named header fields overridden, carrying every + * other owned field, `invocations`, and identity/lifecycle field straight through — the same + * shape as [InMemoryExtractionRunStore]'s own `rebuild`, since [ExtractionRun] publishes no + * `copy`. */ + private fun headerVariant( + base: ExtractionRun, + profile: ExtractionContentProfileRef? = base.profile, + sourceRevisions: List = base.sourceRevisions, + fingerprints: ExtractionRunFingerprints = base.fingerprints, + runtime: ExtractionRuntimeIdentity = base.runtime, + requestedModel: ExtractionRequestedModelConfig? = base.requestedModel, + subjectRefs: ExtractionRunSubjectRefs = base.subjectRefs, + experimentRef: ExtractionExperimentRef? = base.experimentRef, + cohortRef: ExtractionCohortRef? = base.cohortRef, + replayFidelity: ExtractionReplayFidelity = base.replayFidelity, + counts: ExtractionRunCounts = base.counts, + failures: List = base.failures, + ): ExtractionRun = ExtractionRun( + contextId = base.contextId, + lineage = base.lineage, + status = base.status, + startedAt = base.startedAt, + profile = profile, + sourceRevisions = sourceRevisions, + fingerprints = fingerprints, + runtime = runtime, + requestedModel = requestedModel, + subjectRefs = subjectRefs, + experimentRef = experimentRef, + cohortRef = cohortRef, + replayFidelity = replayFidelity, + counts = counts, + invocations = base.invocations, + failures = failures, + version = base.version, + ) + + private fun richHeader(runId: String): ExtractionRun = ExtractionRun( + contextId = tenant, + lineage = ExtractionRunLineage.root(ExtractionRunRef(runId)), + status = ExtractionRunStatus.RUNNING, + startedAt = startedAt, + profile = ExtractionContentProfileRef("profile-base", "v1"), + sourceRevisions = listOf(SourceRevisionRef("source-base", "rev-1")), + fingerprints = ExtractionRunFingerprints(promptTemplateFingerprint = "prompt-base"), + runtime = ExtractionRuntimeIdentity(extractor = "extractor-base"), + requestedModel = ExtractionRequestedModelConfig(requestedModel = "model-base"), + subjectRefs = ExtractionRunSubjectRefs(actor = ExtractionActorRef("actor-base")), + experimentRef = ExtractionExperimentRef("exp-base"), + cohortRef = ExtractionCohortRef("cohort-base"), + replayFidelity = ExtractionReplayFidelity.APPROXIMATE, + counts = ExtractionRunCounts(propositionsPersisted = 5), + failures = listOf(ExtractionFailure.of(ExtractionFailureCode.MODEL_TIMEOUT, ExtractionFailureStage.MODEL_CALL)), + ) + + /** Every field an accepted save owns, one at a time. `lineage`, `startedAt` and `version` sit + * outside save's ownership, each already has its own dedicated mismatch test, and a one-field + * delta on any of them throws a conflict — that case belongs with the terminal-conflict tests + * above this matrix, which covers accepted replacements. */ + private val headerFieldDeltas: List ExtractionRun>> = listOf( + "profile" to { r -> headerVariant(r, profile = ExtractionContentProfileRef("profile-changed", "v2")) }, + "sourceRevisions" to { r -> headerVariant(r, sourceRevisions = listOf(SourceRevisionRef("source-changed", "rev-1"))) }, + "fingerprints" to { r -> headerVariant(r, fingerprints = ExtractionRunFingerprints(promptTemplateFingerprint = "prompt-changed")) }, + "runtime" to { r -> headerVariant(r, runtime = ExtractionRuntimeIdentity(extractor = "extractor-changed")) }, + "requestedModel" to { r -> headerVariant(r, requestedModel = ExtractionRequestedModelConfig(requestedModel = "model-changed")) }, + "subjectRefs" to { r -> headerVariant(r, subjectRefs = ExtractionRunSubjectRefs(actor = ExtractionActorRef("actor-changed"))) }, + "experimentRef" to { r -> headerVariant(r, experimentRef = ExtractionExperimentRef("exp-changed")) }, + "cohortRef" to { r -> headerVariant(r, cohortRef = ExtractionCohortRef("cohort-changed")) }, + "replayFidelity" to { r -> headerVariant(r, replayFidelity = ExtractionReplayFidelity.NONE) }, + "counts" to { r -> headerVariant(r, counts = ExtractionRunCounts(propositionsPersisted = 6)) }, + "failures" to { r -> headerVariant(r, failures = listOf(ExtractionFailure.of(ExtractionFailureCode.MODEL_TIMEOUT, ExtractionFailureStage.RESPONSE_DECODE))) }, + ) + + @Test + fun `a save changing exactly one owned header field bumps the version as a genuinely accepted write`() { + // The bundled header-replace test above changes every owned field at once, so a backend + // whose no-op comparison quietly skips one particular field still looks correct there — + // every other field's difference is enough to trigger the version bump regardless of that + // one field. Isolating a single field is what exercises its own contribution to the no-op + // decision. + headerFieldDeltas.forEach { (fieldName, mutate) -> + val store = store() + val stored = store.save(richHeader("contract-header-delta-$fieldName")) + val changed = mutate(stored) + + val result = store.save(changed) + + assertEquals( + stored.version + 1, + result.version, + "a $fieldName-only change must bump the version as a genuinely accepted write", + ) + val persisted = store.findRun(stored.key())!! + assertEquals(result, persisted, "the $fieldName-only change must be exactly what is persisted") + } + } + + // ---- closed value sets: a field being present with one value is not the same as a test + // discriminating on every value it can hold. replayFidelity and the failure code are both + // carried through the store without it ever branching on which value they hold, and that + // makes an unexercised value more exposed to a drop-or-substitute bug, since nothing else in + // the store would notice either. Every declared value gets its own row below, through every + // door that can write it. ---- + + @Test + fun `every replay fidelity value comes back from save unchanged and outlives a terminal write`() { + // replayFidelity has one entry door: ExtractionRunTransition.applyTo's own KDoc says + // transition() always carries forward whatever value the run already holds. + ExtractionReplayFidelity.entries.forEach { fidelity -> + val store = store() + val run = running("contract-replay-fidelity-${fidelity.name}").let { + ExtractionRun( + contextId = it.contextId, + lineage = it.lineage, + status = it.status, + startedAt = it.startedAt, + replayFidelity = fidelity, + ) + } + val saved = store.save(run) + assertEquals(fidelity, saved.replayFidelity, "$fidelity must come back from save unchanged") + assertEquals( + fidelity, + store.findRun(run.key())?.replayFidelity, + "$fidelity must read back unchanged through findRun", + ) + + val terminal = store.transition(run.key(), ExtractionRunTransition.completed(finishedAt)).run + assertEquals(fidelity, terminal.replayFidelity, "$fidelity must survive the terminal write") + assertEquals( + fidelity, + store.findRun(run.key())?.replayFidelity, + "$fidelity must still read back after the terminal write", + ) + } + } + + @Test + fun `every failure code comes back from save unchanged and outlives a terminal write that keeps it`() { + ExtractionFailureCode.entries.forEach { code -> + val store = store() + val run = running("contract-failure-code-save-${code.name}").let { + ExtractionRun( + contextId = it.contextId, + lineage = it.lineage, + status = it.status, + startedAt = it.startedAt, + failures = listOf(ExtractionFailure.of(code)), + ) + } + val saved = store.save(run) + assertEquals( + listOf(code), + saved.failures.map { it.code }, + "$code must come back from save unchanged", + ) + assertEquals( + listOf(code), + store.findRun(run.key())?.failures?.map { it.code }, + "$code must read back unchanged through findRun", + ) + + val terminal = store.transition( + run.key(), + ExtractionRunTransition.completed(finishedAt, counts = null, failures = null), + ).run + assertEquals( + listOf(code), + terminal.failures.map { it.code }, + "$code must survive a terminal write that keeps the run's failures", + ) + assertEquals( + listOf(code), + store.findRun(run.key())?.failures?.map { it.code }, + "$code must still read back after the terminal write", + ) + } + } + + @Test + fun `every failure code comes back from transition unchanged and survives its own replay`() { + // The transition door's own write is the terminal write, so there is no later terminal + // write left to check the value against; a replay of the same transition is the store's + // own definition of "untouched" for a run that has already ended. + ExtractionFailureCode.entries.forEach { code -> + val store = store() + val run = running("contract-failure-code-transition-${code.name}") + store.save(run) + val transition = ExtractionRunTransition.completed( + finishedAt, + failures = listOf(ExtractionFailure.of(code)), + ) + + val applied = store.transition(run.key(), transition).run + assertEquals( + listOf(code), + applied.failures.map { it.code }, + "$code must come back from transition unchanged", + ) + assertEquals( + listOf(code), + store.findRun(run.key())?.failures?.map { it.code }, + "$code must read back unchanged through findRun", + ) + + val replayed = store.transition(run.key(), transition).run + assertEquals( + listOf(code), + replayed.failures.map { it.code }, + "$code must survive the transition's own replay", + ) + assertEquals( + listOf(code), + store.findRun(run.key())?.failures?.map { it.code }, + "$code must still read back after the replay", + ) + } + } + + // ---- bounded, scoped reads ---- + + @Test + fun `a page scopes before it limits`() { + val store = store() + (1..5).forEach { + store.save(running("contract-neighbour-$it", neighbour, startedAt.plusSeconds(100L + it))) + } + (1..3).forEach { store.save(running("contract-mine-$it", tenant, startedAt.plusSeconds(it.toLong()))) } + + val page = store.runsInContext(tenant, limit = 2, since = null) + + assertEquals(listOf("contract-mine-3", "contract-mine-2"), page.map { it.ref.runId }) + } + + @Test + fun `a page is newest first and tie-broken by run id`() { + val store = store() + listOf("contract-c", "contract-a", "contract-b").forEach { store.save(running(it)) } + + assertEquals( + listOf("contract-a", "contract-b", "contract-c"), + store.runsInContext(tenant, limit = 10, since = null).map { it.ref.runId }, + ) + } + + @Test + fun `since bounds the window at or after the instant given`() { + val store = store() + (0..3).forEach { store.save(running("contract-since-$it", startedAt = startedAt.plusSeconds(it * 10L))) } + + val windowed = store.runsInContext(tenant, limit = 10, since = startedAt.plusSeconds(20)) + + assertEquals( + listOf("contract-since-3", "contract-since-2"), + windowed.map { it.ref.runId }, + ) + } + + @Test + fun `every page rejects a limit that is not positive`() { + val store = store() + val root = ExtractionRunRef("contract-root") + + listOf(0, -1).forEach { limit -> + assertThrows(IllegalArgumentException::class.java) { + store.runsInContext(tenant, limit, null) + } + assertThrows(IllegalArgumentException::class.java) { + store.childrenOf(tenant, root, limit) + } + assertThrows(IllegalArgumentException::class.java) { + store.runsOfRoot(tenant, root, limit, null) + } + assertThrows(IllegalArgumentException::class.java) { + store.ancestorsOf(key("contract-root"), limit) + } + } + } + + // ---- lineage ---- + + @Test + fun `children are one hop down the parent axis`() { + val store = store() + val parent = ExtractionRunLineage.root(ExtractionRunRef("contract-parent")) + val child = ExtractionRunLineage.childOf(ExtractionRunRef("contract-child"), parent) + val grandchild = ExtractionRunLineage.childOf(ExtractionRunRef("contract-grandchild"), child) + listOf(parent, child, grandchild).forEach { + store.save(running(it.runRef.runId, lineage = it)) + } + + assertEquals( + listOf("contract-child"), + store.childrenOf(tenant, parent.runRef, limit = 10).map { it.ref.runId }, + ) + } + + @Test + fun `a whole lineage comes back from its root reference`() { + val store = store() + val root = ExtractionRunLineage.root(ExtractionRunRef("contract-root")) + val one = ExtractionRunLineage.childOf(ExtractionRunRef("contract-pass-1"), root) + val two = ExtractionRunLineage.childOf(ExtractionRunRef("contract-pass-2"), one) + val unrelated = ExtractionRunLineage.root(ExtractionRunRef("contract-unrelated")) + listOf(root, one, two, unrelated).forEachIndexed { index, lineage -> + store.save(running(lineage.runRef.runId, startedAt = startedAt.plusSeconds(index.toLong()), lineage = lineage)) + } + + assertEquals( + listOf("contract-pass-2", "contract-pass-1", "contract-root"), + store.runsOfRoot(tenant, root.runRef, limit = 10, since = null).map { it.ref.runId }, + ) + } + + @Test + fun `the chain walk is bounded, excludes the run, and stops at an unresolvable parent`() { + val store = store() + val root = ExtractionRunLineage.root(ExtractionRunRef("contract-root")) + val one = ExtractionRunLineage.childOf(ExtractionRunRef("contract-1"), root) + val two = ExtractionRunLineage.childOf(ExtractionRunRef("contract-2"), one) + listOf(root, one, two).forEach { store.save(running(it.runRef.runId, lineage = it)) } + + assertEquals( + listOf("contract-1", "contract-root"), + store.ancestorsOf(key("contract-2"), limit = 10).map { it.ref.runId }, + ) + assertEquals( + listOf("contract-1"), + store.ancestorsOf(key("contract-2"), limit = 1).map { it.ref.runId }, + ) + assertTrue(store.ancestorsOf(key("contract-root"), limit = 10).isEmpty()) + + val orphan = ExtractionRunLineage.childOf( + ExtractionRunRef("contract-orphan"), + ExtractionRunLineage.root(ExtractionRunRef("contract-missing")), + ) + store.save(running("contract-orphan", lineage = orphan)) + assertTrue(store.ancestorsOf(key("contract-orphan"), limit = 10).isEmpty()) + } + + @Test + fun `the chain walk terminates on a cycle`() { + val store = store() + val root = ExtractionRunRef("contract-cycle-root") + val a = ExtractionRunLineage.fromStoredFields( + runRef = ExtractionRunRef("contract-a"), + rootRunRef = root, + parentRunRef = ExtractionRunRef("contract-b"), + ) + val b = ExtractionRunLineage.fromStoredFields( + runRef = ExtractionRunRef("contract-b"), + rootRunRef = root, + parentRunRef = ExtractionRunRef("contract-a"), + ) + store.save(running("contract-a", lineage = a)) + store.save(running("contract-b", lineage = b)) + + assertEquals( + listOf("contract-b"), + store.ancestorsOf(key("contract-a"), limit = 1_000).map { it.ref.runId }, + ) + } + + // ---- tenants ---- + + @Test + fun `two tenants holding the same run id never collide`() { + val store = store() + val mine = running("contract-shared", tenant, startedAt) + val theirs = running("contract-shared", neighbour, startedAt.plusSeconds(60)) + store.save(mine) + store.save(theirs) + + assertEquals(startedAt, store.findRun(mine.key())?.startedAt) + assertEquals(startedAt.plusSeconds(60), store.findRun(theirs.key())?.startedAt) + + store.transition(mine.key(), ExtractionRunTransition.completed(finishedAt)) + + assertEquals(ExtractionRunStatus.COMPLETED, store.findRun(mine.key())?.status) + assertEquals(ExtractionRunStatus.RUNNING, store.findRun(theirs.key())?.status) + } + + @Test + fun `every read fails closed across tenants`() { + val store = store() + val root = ExtractionRunLineage.root(ExtractionRunRef("contract-root")) + val child = ExtractionRunLineage.childOf(ExtractionRunRef("contract-child"), root) + listOf(root, child).forEach { + store.save(running(it.runRef.runId, neighbour, lineage = it)) + } + + assertNull(store.findRun(key("contract-child", tenant))) + assertTrue(store.invocationsOf(key("contract-child", tenant)).isEmpty()) + assertTrue(store.runsInContext(tenant, 10, null).isEmpty()) + assertTrue(store.childrenOf(tenant, root.runRef, 10).isEmpty()) + assertTrue(store.runsOfRoot(tenant, root.runRef, 10, null).isEmpty()) + assertTrue(store.ancestorsOf(key("contract-child", tenant), 10).isEmpty()) + } + + @Test + fun `a chain walk stops rather than crossing into another tenant`() { + val store = store() + val parent = ExtractionRunLineage.root(ExtractionRunRef("contract-parent")) + val child = ExtractionRunLineage.childOf(ExtractionRunRef("contract-child"), parent) + store.save(running("contract-parent", neighbour, lineage = parent)) + store.save(running("contract-child", tenant, lineage = child)) + + assertTrue(store.ancestorsOf(key("contract-child", tenant), 10).isEmpty()) + } +} diff --git a/dice-storage/src/test/kotlin/com/embabel/dice/storage/InMemoryExtractionRunStoreContractTest.kt b/dice-storage/src/test/kotlin/com/embabel/dice/storage/InMemoryExtractionRunStoreContractTest.kt new file mode 100644 index 00000000..ad459914 --- /dev/null +++ b/dice-storage/src/test/kotlin/com/embabel/dice/storage/InMemoryExtractionRunStoreContractTest.kt @@ -0,0 +1,28 @@ +/* + * Copyright 2024-2026 Embabel Pty Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.embabel.dice.storage + +import com.embabel.dice.proposition.extraction.ExtractionRunStore +import com.embabel.dice.proposition.extraction.InMemoryExtractionRunStore + +/** + * Runs the [AbstractExtractionRunStoreContractTest] suite against the in-memory backend. No Docker, + * so it runs in the normal test phase — the always-on half of the cross-backend parity check the + * Drivine run store completes when it lands. + */ +class InMemoryExtractionRunStoreContractTest : AbstractExtractionRunStoreContractTest() { + override fun store(): ExtractionRunStore = InMemoryExtractionRunStore() +} diff --git a/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionInvocationRecord.kt b/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionInvocationRecord.kt index 979f5c07..951f59b1 100644 --- a/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionInvocationRecord.kt +++ b/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionInvocationRecord.kt @@ -83,6 +83,11 @@ enum class ExtractionInvocationOutcome { /** The attempt was stopped before it produced anything. */ CANCELLED, + ; + + /** True for the three outcomes an attempt does not leave, same rule as [ExtractionRunStatus]. */ + val isTerminal: Boolean + get() = this != IN_FLIGHT } /** diff --git a/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRun.kt b/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRun.kt index 009bcebe..81b2e8f9 100644 --- a/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRun.kt +++ b/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRun.kt @@ -41,6 +41,20 @@ data class ExtractionRunKey( /** The tenant id as a plain string, for Java callers, since `ContextId` is a value class. */ fun getContextIdValue(): String = contextId.value + + companion object { + + /** + * Java-friendly factory taking both halves as plain strings. + * + * `ContextId` is a Kotlin value class, so the constructor has a mangled JVM name. Every + * read on `ExtractionRunStore` keys on this type, so without a factory the store would be + * unreachable from Java. + */ + @JvmStatic + fun of(contextIdValue: String, runId: String): ExtractionRunKey = + ExtractionRunKey(ContextId(contextIdValue), ExtractionRunRef(runId)) + } } /** @@ -81,7 +95,7 @@ data class ExtractionRunKey( * This is a plain class rather than a data class on purpose. A data class has to declare its * collection parameters as properties, which means the field is the caller's list and there is * nowhere to copy it; its generated `copy` and `componentN` methods would also pin an ABI across - * seventeen fields while #67 is still moving. Equality and hash are written out over every + * eighteen fields while #67 is still moving. Equality and hash are written out over every * component instead. * * EXPERIMENTAL. The shape may still change while extraction runs (DICE #67) land. @@ -103,6 +117,15 @@ data class ExtractionRunKey( * @property counts How much the run got through * @property invocations One record per attempt at each planned model call * @property failures Bounded record of what went wrong, in the failure vocabulary + * @property version The compare-and-set generation [ExtractionRunStore.save] checks this header + * against. A run that has never been saved, and the run its first accepted save produces, both + * carry 0 — the first save inserts the row, and there is no earlier generation for it to raise + * past. A store rejects a first save naming any other value. Every later save [save] accepts that + * actually changes the header raises it by one; a save whose content already matches what is + * stored is accepted too, as a no-op replay, and leaves the generation exactly where it stood. + * [ExtractionRunStore.recordInvocation] never changes it, since an invocation record writes a + * child row of its own — and [ExtractionRunTransition.applyTo] carries whatever value the run + * already has, because a terminal run takes no more saves and nothing compares its version again */ @ApiStatus.Experimental class ExtractionRun @JvmOverloads constructor( @@ -123,6 +146,7 @@ class ExtractionRun @JvmOverloads constructor( val counts: ExtractionRunCounts = ExtractionRunCounts(), invocations: List = emptyList(), failures: List = emptyList(), + val version: Long = 0, ) { /** Which revisions of which sources this run read, in order. */ @@ -138,6 +162,8 @@ class ExtractionRun @JvmOverloads constructor( Collections.unmodifiableList(ArrayList(failures)) init { + require(version >= 0) { "version must not be negative, was $version" } + require(finishedAt == null || !finishedAt.isBefore(startedAt)) { "finishedAt must not be before startedAt" } @@ -233,7 +259,8 @@ class ExtractionRun @JvmOverloads constructor( replayFidelity == other.replayFidelity && counts == other.counts && invocations == other.invocations && - failures == other.failures + failures == other.failures && + version == other.version } override fun hashCode(): Int { @@ -254,6 +281,7 @@ class ExtractionRun @JvmOverloads constructor( result = 31 * result + counts.hashCode() result = 31 * result + invocations.hashCode() result = 31 * result + failures.hashCode() + result = 31 * result + version.hashCode() return result } @@ -267,7 +295,8 @@ class ExtractionRun @JvmOverloads constructor( "ExtractionRun(contextId=${contextId.value}, runId=${ref.runId}, rootRunId=${rootRef.runId}, " + "parentRunId=${parentRef?.runId}, pass=${lineage.passIndex}, status=$status, " + "startedAt=$startedAt, finishedAt=$finishedAt, sourceRevisions=${sourceRevisions.size}, " + - "invocations=${invocations.size}, failures=${failures.size}, replayFidelity=$replayFidelity)" + "invocations=${invocations.size}, failures=${failures.size}, replayFidelity=$replayFidelity, " + + "version=$version)" companion object { @@ -297,6 +326,7 @@ class ExtractionRun @JvmOverloads constructor( counts: ExtractionRunCounts = ExtractionRunCounts(), invocations: List = emptyList(), failures: List = emptyList(), + version: Long = 0, ): ExtractionRun = ExtractionRun( contextId = ContextId(contextIdValue), lineage = lineage, @@ -315,6 +345,7 @@ class ExtractionRun @JvmOverloads constructor( counts = counts, invocations = invocations, failures = failures, + version = version, ) } } diff --git a/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunFingerprint.kt b/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunFingerprint.kt new file mode 100644 index 00000000..ba48038c --- /dev/null +++ b/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunFingerprint.kt @@ -0,0 +1,175 @@ +/* + * Copyright 2024-2026 Embabel Pty Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.embabel.dice.proposition.extraction + +import org.jetbrains.annotations.ApiStatus +import java.security.MessageDigest +import java.time.Instant + +/** + * The digest a store compares two terminal writes by. + * + * A run store keeps the fingerprint of the write that terminalized a run. A second write under the + * same key either matches it — a retry, replayed as success — or does not, and is rejected. The + * whole mechanism rests on two payloads that mean the same thing producing the same bytes, so the + * encoding is specified here rather than left to whatever a serializer happens to emit. + * + * ## Why not JSON + * + * RFC 8785 exists because naive JSON serialization is not byte-stable. Three failure modes, all of + * which would surface here as a correct retry being rejected: + * + * - **Key order.** Most serializers emit fields in declaration or reflection order, and neither is + * guaranteed stable across versions of the code or the library. + * - **Number rendering.** The same value can serialize as `1`, `1.0`, `1e0` or `1.0E+0` depending + * on the writer, and floating-point round-tripping differs between implementations. + * - **Insignificant text.** Whitespace, escaping choices and Unicode normalization all move the + * bytes without moving the meaning. + * + * This encoding is the one DICE already uses for `MetamodelVersion.contentHash`, applied to a + * different payload: length-prefixed tokens, count-prefixed collections, sorted where order carries + * no meaning, SHA-256, lowercase hex. + * + * ## The rules + * + * 1. **Every token is length-prefixed**, `:`. A delimiter-joined encoding lets + * `["a;b"]` and `["a", "b"]` hash the same, which hides a real difference. Length prefixes make + * that collision unreachable whatever characters a token happens to carry. + * 2. **Every collection is preceded by its element count**, so a shorter list can never be a prefix + * of a longer one. + * 3. **Fields are emitted as `(name, value)` pairs sorted by name**, so the bytes do not depend on + * the order the fields happen to be declared in. + * 4. **A collection whose order carries no meaning is sorted.** The failure list is the case that + * matters: two coordinators that recorded the same failures in a different order made the same + * terminal write. The order is Kotlin's natural `String` order, which compares UTF-16 code units + * — not UTF-8 byte order, and not a locale collation. Any total order would do for correctness; + * naming this one matters because a backend re-implementing the sort in a query would pick a + * different one and produce a different digest. It should not re-implement it at all: the store + * records the string the transition already computed. + * 5. **Instants render as `.`.** Fixed width, and independent of + * `java.time`'s own formatting. `Instant.toString()` varies its precision with the value — + * `…:47Z` for a whole second, `…:47.500Z` for half of one — so the encoded length moves with the + * data and a persisted digest would depend on a formatting rule DICE does not own. Number + * rendering is the equivalent hazard in JSON and is most of what RFC 8785 is about. + * 6. **Absent is its own marker.** A null renders as `-`, never as an empty string, so "the caller + * said nothing" and "the caller said empty" stay distinguishable. + * 7. **The digest is SHA-256, rendered lowercase hex**, and the encoded input carries a version + * tag. A reader meeting a version it does not know matches nothing rather than guessing. + * + * ## This is a persisted format + * + * The digest is stored beside the run. Changing the encoding makes every recorded fingerprint + * unmatchable, so a correct retry against an old run would be rejected as an incompatible rewrite. + * `ExtractionRunFingerprintTest` pins the digest of a fixed payload with a literal assertion: + * changing the encoding means changing that literal deliberately. + * + * EXPERIMENTAL. The shape may still change while extraction runs (DICE #67) land. + */ +@ApiStatus.Experimental +object ExtractionRunFingerprint { + + /** Version tag on the terminal-write encoding. */ + const val TERMINAL_VERSION: String = "xrun-terminal:v1" + + /** What an absent value encodes as, kept distinct from any value a caller could supply. */ + private const val ABSENT = "-" + + /** + * The digest of one terminal write: the status it asserts, when the run finished, the counts it + * records, and the failures it records. + * + * The run itself is deliberately not part of this. A coordinator that records another invocation + * between a failed terminal write and its retry made the same terminal write both times, and + * folding the run's invocation list in would turn that correct retry into a rejected conflict. + */ + @JvmStatic + fun ofTerminal( + status: ExtractionRunStatus, + finishedAt: Instant, + counts: ExtractionRunCounts?, + failures: List?, + ): String { + val payload = fields( + "counts" to (counts?.let(::encodeCounts) ?: ABSENT), + "failures" to (failures?.let(::encodeFailures) ?: ABSENT), + "finishedAt" to encodeInstant(finishedAt), + "status" to status.name, + ) + return digest(TERMINAL_VERSION + "|" + payload) + } + + private fun encodeCounts(counts: ExtractionRunCounts): String = fields( + "chunksProcessed" to counts.chunksProcessed.toString(), + "entitiesResolved" to counts.entitiesResolved.toString(), + "propositionsExtracted" to counts.propositionsExtracted.toString(), + "propositionsPersisted" to counts.propositionsPersisted.toString(), + "propositionsRejected" to counts.propositionsRejected.toString(), + "sourcesRead" to counts.sourcesRead.toString(), + ) + + private fun encodeFailures(failures: List): String { + val encoded = failures.map { failure -> + fields( + "at" to encodeInstant(failure.at), + "code" to failure.code.name, + "invocation" to ( + failure.invocation + ?.let { "${it.invocationIndex}/${it.attempt}" } + ?: ABSENT + ), + "measure" to ( + failure.measure + ?.let { "${it.quantity.name}=${it.value}" } + ?: ABSENT + ), + "providerStatus" to (failure.providerStatus?.toString() ?: ABSENT), + "stage" to (failure.stage?.name ?: ABSENT), + ) + }.sorted() + return buildString { + append(encoded.size).append('|') + encoded.forEach { appendSized(it) } + } + } + + /** + * Fixed-width rendering: seconds since the epoch, a dot, then nanoseconds padded to nine + * digits. Two instants that compare equal always render identically, and no instant renders + * two ways. + */ + private fun encodeInstant(instant: Instant): String = + "${instant.epochSecond}.${instant.nano.toString().padStart(9, '0')}" + + /** Emit `(name, value)` pairs sorted by name, each half length-prefixed. */ + private fun fields(vararg pairs: Pair): String = buildString { + append(pairs.size).append('|') + pairs.sortedBy { it.first }.forEach { (name, value) -> + appendSized(name) + appendSized(value) + } + } + + private fun StringBuilder.appendSized(token: String) { + append(token.length).append(':').append(token) + } + + private fun digest(input: String): String = + MessageDigest.getInstance("SHA-256") + .digest(input.toByteArray(Charsets.UTF_8)) + .joinToString("") { byte -> HEX[(byte.toInt() shr 4) and 0xF].toString() + HEX[byte.toInt() and 0xF] } + + private const val HEX = "0123456789abcdef" +} diff --git a/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunStore.kt b/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunStore.kt new file mode 100644 index 00000000..e6773fc1 --- /dev/null +++ b/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunStore.kt @@ -0,0 +1,444 @@ +/* + * Copyright 2024-2026 Embabel Pty Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.embabel.dice.proposition.extraction + +import com.embabel.agent.core.ContextId +import org.jetbrains.annotations.ApiStatus +import java.time.Instant + +/** + * Durable store of extraction runs, and the lifecycle state machine that governs them. + * + * ## The state machine + * + * A run starts `RUNNING` and ends in one of `COMPLETED`, `FAILED` or `CANCELLED`. There are no + * other edges: a terminal run never re-opens, and a run never moves from one terminal state to + * another. + * + * ``` + * RUNNING ──▶ COMPLETED + * ──▶ FAILED + * ──▶ CANCELLED + * ``` + * + * The two write methods split along that line. [save] records a run and rejects anything that is + * not `RUNNING`, so a terminal status cannot enter the store through the door that also accepts + * new keys. [transition] is the only writer of a terminal status, and it is compare-and-set: it + * moves a run out of `RUNNING` or it does nothing. + * + * ## Three writes, three kinds of state, one owner each + * + * [save] owns the run header: the profile, the source revisions, the digests, the requested model, + * the counts, the failures — everything on [ExtractionRun] except its invocation list. It is + * compare-and-set on [ExtractionRun.version]. [recordInvocation] owns invocation rows: it is the + * only method that creates, updates or locks one, keyed by [ExtractionInvocationRecord.id], and it + * is insert-or-compare on that key alone — a write to one attempt's row never touches another's, and + * never touches the header's version. [transition] owns the terminal write. + * + * **A header save ignores whatever invocation snapshot it is handed.** [save] never creates, + * updates or deletes an invocation row, however non-empty [ExtractionRun.invocations] is on the run + * it is given. This was not always true — see [save]'s own KDoc for the shared-generation defect + * that a merged write produced, and why the fix gives invocation rows their own door and their + * own key, with compare-and-set scoped to that key alone. + * + * ## What `COMPLETED` asserts, and who may write it + * + * `COMPLETED` means every product the run's request called for is either durably persisted or + * terminally disposed. The store cannot check that — it holds run headers, not products — so it + * does the next best thing and makes the claim reachable through exactly one narrow call whose + * precondition is written down: + * + * - on the legacy path, the coordinator calls it once `persistAndProject` has returned; + * - on the DICE #68 commit path, the commit transaction calls it, and only the commit whose + * cumulative outcomes bring every requested product to persisted or terminally disposed. + * + * A run whose persistence never finished stays `RUNNING` and is retryable under compare-and-set. A + * commit that persists some products and leaves others outstanding leaves the run `RUNNING` too, so + * a terminal run never has re-committable products behind it. A run with zero products completes + * vacuously: there was nothing to persist, so the coverage claim holds. + * + * `FAILED` and `CANCELLED` carry no such precondition. A run that could not finish, or that was + * stopped, terminalizes independently of whether anything was persisted. + * + * ## Idempotency + * + * Every terminal write carries a fingerprint of its payload ([ExtractionRunTransition.fingerprint]). + * A store records the fingerprint of the write that terminalized a run, and a second write against + * that run is decided by comparison, never by overwrite: + * + * - same fingerprint — the same terminal write, retried. It replays as success + * ([ExtractionRunTransitionOutcome.REPLAYED]) and changes nothing. + * - different fingerprint — a second, incompatible claim about how the run ended. Rejected with + * [ExtractionRunConflictException]. + * + * That is insert-or-compare. DICE's existing `MERGE … SET` stores upsert by overwriting, which is + * safe for a record that is still being written and wrong for one that is finished: it would let a + * late or duplicated writer silently rewrite how a run ended, and the audit would carry the last + * write rather than the true one. No method here overwrites a terminal run. + * + * Two mechanisms from the idempotency prior art are deliberately not adopted. There is no + * epoch or writer generation of the Kafka kind: epochs fence a zombie writer across systems, and + * the concurrency this contract has to survive is two writers racing on one row, which the + * compare-and-set inside a single store transaction already decides. And there is no key expiry of + * the Stripe kind: a run header is a permanent audit row, and pruning idempotency records after a + * day would delete the evidence rather than the bookkeeping. + * + * ## Every read is tenant-scoped and bounded + * + * A run store grows once per extraction forever, so there is no unbounded read here. Every page + * takes a positive `limit`, and the reads that can span a long history also take an optional + * `since` window. + * + * **Scope is pushed down, never applied afterwards.** An implementation must restrict to the tenant + * inside the query and then limit. Fetching `limit` rows and filtering them by tenant afterwards + * would return fewer rows than asked for — or none — whenever a busy neighbouring tenant occupies + * the head of the index, and the caller cannot tell that from an empty tenant. This is why none of + * the scoped reads has a default body: a default that filtered in memory would be inherited + * silently by every backend that forgot to override it. + * + * The `ContextId`-typed overloads do have default bodies, and they are a different thing: they + * forward to the `String`-typed method that is the override point. They cannot return the wrong + * rows, because they do not filter. The split exists because `ContextId` is a Kotlin value class, + * so any method taking one compiles to a mangled JVM name that Java callers cannot reach. + * + * **Cross-tenant reads fail closed.** Every lookup, page, chain walk and aggregate is scoped to the + * tenant it was asked about. A run id that exists in two tenants is two runs, and a read against + * one never returns the other's. The chain walk stops rather than crossing: a parent reference that + * resolves only in another tenant is treated as unresolved. Slice 8 proves this against a real + * graph; here it is the contract every implementation is held to. + * + * ## Ordering + * + * Pages come back newest first by [ExtractionRun.startedAt], tie-broken by run id ascending. The + * tie-break is what makes a page repeatable: two runs started in the same millisecond would + * otherwise come back in whatever order the backend felt like, and a caller paging through would + * see one of them twice or neither. + * + * EXPERIMENTAL. The shape may still change while extraction runs (DICE #67) land. + */ +@ApiStatus.Experimental +interface ExtractionRunStore { + + // ---- writes ---- + + /** + * Records a running run's header, inserting it under [ExtractionRun.key] or updating the one + * already there. **This writes header fields only — it is not a door onto invocation state.** + * [recordInvocation] is the only method that creates, updates or locks an invocation row; + * whatever [ExtractionRun.invocations] holds on [run] is not written anywhere and does not + * affect what a save accepts, rejects or replays as a no-op. A caller building [run] from a + * previous read does not need to strip that field, but nothing is lost either way if it does. + * + * The lineage and the start time are fixed at insert. A later save that disagrees with either + * names a different run wearing the same id and is rejected. Everything else the header owns — + * the profile, the source revisions the run has read so far, the digests, the counts, the + * failures it has accumulated — may be filled in as the run proceeds, and an accepted update + * replaces the previous value for every one of those fields at once. + * + * **A header update is compare-and-set on [ExtractionRun.version].** Two callers can hold a run + * at once — one updating counts, one recording a source revision it just read — and whichever + * saves second must not silently put the header back the way it looked before the first save. + * `save` accepts an update only when [run]'s version names the version currently stored; a first + * save must name `0`, the version a run nobody has saved yet carries. When the named version does + * not match what is stored, the store has moved since [run] was read, and the write is rejected. + * A caller meeting that rejection reads the run again with [findRun] and rebuilds its update on + * the version that read returns. A save whose header content is already exactly what is stored + * replays as a no-op regardless of the version it names, so a retry that never learned its first + * attempt landed is never told it conflicted. + * + * This was tried first as a field-by-field merge that let two saves each keep the parts of the + * header the other did not touch. A review round found the merge could not be made both correct + * and simple: independent count contributions from two writers cannot be recovered by keeping + * the larger number, because each may have counted disjoint work the other did not see; a union + * of source revisions loses the order they were read in, which is the field's own documented + * meaning; and a merge that combines fields from two different writers' saves can produce a + * header no writer ever actually held — a fingerprint from one save paired with a replay + * fidelity from another that the fingerprint does not support. A rejected stale write, retried + * against fresh state by a caller who has the domain knowledge to combine its own new work with + * what it reads, avoids all three. + * + * **Why invocation state moved to its own door entirely.** An earlier version of this contract + * had `save` merge the invocation records [run] carried into the ones already stored, so a header + * update built on a run read before an attempt was recorded would not silently drop that + * attempt. That merge shared the header's version fence for its own conflict checks but not for + * its own staleness: recording an attempt never moved the header's version, so a save built on a + * header read well before a [recordInvocation] call landed could still name the current version + * and be accepted — carrying a stale, non-empty invocation snapshot in on the same write, silently + * replacing dispatch details that call had already filled in. Two independent writers each + * updating counts on a header read before the other's attempt was recorded could race the same + * way against each other. Versioning the header does not cover invocation rows, so the lost + * update survived the version check — which was the defect. Run state here follows the model + * lineage systems such as OpenLineage use: independent writers contribute rows, and no write + * rewrites a row another writer owns. `recordInvocation` insert-or-compares on the invocation's + * own key, so two attempts never contend on the header's version because neither is writing the + * header, and a header save can never be the stale write that erases one. + * + * @param run The run to record. Must be [ExtractionRunStatus.RUNNING] and carry no + * [ExtractionRun.finishedAt]. + * @return The stored run, including any invocation records already recorded against it through + * [recordInvocation]. Its [ExtractionRun.version] is one higher than [run]'s when the header + * changed, and unchanged when the save was a no-op. + * @throws IllegalArgumentException if [run] is not running, if it carries a finish time, or if + * it is a first save naming a version other than `0`. A terminal status reaches the store only + * through [transition], which is what keeps `COMPLETED` behind its precondition; a running run + * that has already finished is a record every page and audit meeting it has to guess about; + * [ExtractionRun] leaves status-and-timing pairing to this state machine, so this is where + * both checks live, alongside the version a run's first save must carry. + * @throws ExtractionRunConflictException if a run is already stored under the same key and has + * ended, if it disagrees on lineage or start time (the tenant cannot disagree — it is half of + * [ExtractionRun.key]), or if [run]'s version does not name the version currently stored. + */ + fun save(run: ExtractionRun): ExtractionRun + + /** + * Records one attempt at one model call against a running run. **This is the only door onto + * invocation state** — `save` writes header fields only and never creates, updates or deletes an + * invocation row, however non-empty the invocation list on the run it is handed. + * + * The record's [ExtractionInvocationRecord.id] is its key within the run, and every write here + * is insert-or-compare on that key alone: a row for an id not yet stored is inserted, and a + * write against a row another writer owns never overwrites it wholesale — see the terminal lock + * below for what "owns" means before an id's outcome settles. A caller does not have to hold the + * whole run header to add one, and two calls recording different ids never contend with each + * other or with a concurrent [save]. + * + * While the attempt is [ExtractionInvocationOutcome.IN_FLIGHT], a repeated write for the same id + * updates in place — that is how dispatch details and, eventually, the terminal outcome fill in + * as the attempt runs — and the next attempt lands on its own row. That in-place update replaces + * the whole record with whatever the latest write carries; it does not merge fields from the + * write it displaces. Two writers racing on the same id while it is still + * [ExtractionInvocationOutcome.IN_FLIGHT], each carrying disjoint dispatch facts the other does + * not have, leave only the facts the later write named — the earlier write's facts are gone, + * with no conflict raised, because neither write disagrees about the outcome and the lock in the + * next paragraph applies only once the record is terminal. A caller that needs every writer's + * facts preserved has to carry the full accumulated record on each write itself; the store does + * not accumulate one for it. + * + * **Once an attempt is terminal, its record is locked.** A write for an id already stored as + * [ExtractionInvocationOutcome.SUCCEEDED], [ExtractionInvocationOutcome.FAILED] or + * [ExtractionInvocationOutcome.CANCELLED] is accepted only when it equals the stored record + * exactly — an identical retry replays as a no-op — and every other write for that id is + * rejected, whether it claims a different outcome or the same outcome with different timing, + * usage or provider facts. The case that motivates the rule is a dispatcher's own retry timer + * firing late and delivering an [ExtractionInvocationOutcome.IN_FLIGHT] write for an attempt + * that had already succeeded or failed, which would otherwise put the attempt back to + * outstanding and erase the record of how it actually ended. Locking the whole record closes a + * narrower version of the same problem too — a delayed write that repeats the correct outcome + * and omits the timing, usage or provider facts the terminal write actually carried. A rejection + * here matches [transition]'s own choice for the run as a whole: a caller finding out that its + * message arrived too late is safer than a caller that cannot tell whether it did. Because `save` + * never touches this state, a stale header snapshot — however old, however different its + * invocation list — cannot be the write that puts a terminal record back to outstanding or + * erases the facts it carries; only another call here can. + * + * @param key The run to record against. + * @param record The attempt. + * @return The run, with the record in place. + * @throws ExtractionRunNotFoundException if no run is stored under [key]. + * @throws ExtractionRunConflictException if the run has already ended (a finished run's + * invocation list is part of how it finished), or if [record] differs from an attempt the store + * already holds as terminal. + */ + fun recordInvocation(key: ExtractionRunKey, record: ExtractionInvocationRecord): ExtractionRun + + /** + * Ends a run: compare-and-set from `RUNNING` to the transition's terminal status. + * + * **A replay needs the identical payload, finish time included.** A coordinator retrying after a + * crash it never saw the answer to must reuse the transition it built the first time, or read + * the run back with [findRun] and stop if it has already ended. Minting a fresh `finishedAt` on + * the retry produces a different fingerprint, which is an incompatible rewrite and is rejected — + * safe, and the opposite of what a caller expecting idempotency would predict. It is the + * transition-side twin of the rule [save] states for start times. + * + * @param key The run to end. + * @param transition What the run ended as. + * @return The terminal run, and whether this call ended it or replayed a write already + * recorded. + * @throws ExtractionRunNotFoundException if no run is stored under [key]. A terminal write + * against a run nobody started is a bug in the caller, not a run to invent. + * @throws ExtractionRunConflictException if the run has already ended under a terminal write + * with a different fingerprint. + */ + fun transition( + key: ExtractionRunKey, + transition: ExtractionRunTransition, + ): ExtractionRunTransitionResult + + // ---- reads ---- + + /** + * The run stored under [key], or null. + * + * @param key The tenant-qualified run identity. + * @return The run, or null if this tenant has no run under that id. + */ + fun findRun(key: ExtractionRunKey): ExtractionRun? + + /** + * Every attempt recorded against the run, in plan order: call 0 before call 1, and within a + * call, first attempt before second. + * + * @param key The tenant-qualified run identity. + * @return The records, or empty if the run has none or does not exist. + */ + fun invocationsOf(key: ExtractionRunKey): List + + /** + * One tenant's runs, newest first. + * + * @param contextIdValue The tenant. + * @param limit The most runs to return. Must be positive. + * @param since When non-null, only runs started at or after this instant. + * @return At most [limit] runs, newest first by start time and then by run id. + * @throws IllegalArgumentException if [limit] is not positive. + */ + fun runsInContext(contextIdValue: String, limit: Int, since: Instant?): List + + /** + * [runsInContext] for Kotlin callers holding a typed tenant. + * + * @param contextId The tenant. + * @param limit The most runs to return. Must be positive. + * @param since When non-null, only runs started at or after this instant. + * @return At most [limit] runs, newest first. + */ + fun runsInContext(contextId: ContextId, limit: Int, since: Instant?): List = + runsInContext(contextId.value, limit, since) + + /** + * The runs whose immediate parent is [parentRunId] — one hop down the parent axis, in one + * tenant. + * + * Supersession is a separate axis and is not walked here. A run that replaces another without + * continuing it is not its child. + * + * @param contextIdValue The tenant. + * @param parentRunId The parent run's id. + * @param limit The most runs to return. Must be positive. + * @return At most [limit] children, newest first. + * @throws IllegalArgumentException if [limit] is not positive. + */ + fun childrenOf(contextIdValue: String, parentRunId: String, limit: Int): List + + /** + * [childrenOf] for Kotlin callers holding typed references. + * + * @param contextId The tenant. + * @param parent The parent run. + * @param limit The most runs to return. Must be positive. + * @return At most [limit] children, newest first. + */ + fun childrenOf(contextId: ContextId, parent: ExtractionRunRef, limit: Int): List = + childrenOf(contextId.value, parent.runId, limit) + + /** + * Every run in one lineage: those whose [ExtractionRunLineage.rootRunRef] is [rootRunId], + * including the root itself. + * + * This is the read the denormalized root reference exists for. The root is fixed when a lineage + * is minted and can never drift, so a whole lineage is one indexed read on one property instead + * of a chain walk a hop at a time. + * + * @param contextIdValue The tenant. + * @param rootRunId The root run's id. + * @param limit The most runs to return. Must be positive. + * @param since When non-null, only runs started at or after this instant. + * @return At most [limit] runs of that lineage, newest first. + * @throws IllegalArgumentException if [limit] is not positive. + */ + fun runsOfRoot( + contextIdValue: String, + rootRunId: String, + limit: Int, + since: Instant?, + ): List + + /** + * [runsOfRoot] for Kotlin callers holding typed references. + * + * @param contextId The tenant. + * @param root The root run. + * @param limit The most runs to return. Must be positive. + * @param since When non-null, only runs started at or after this instant. + * @return At most [limit] runs of that lineage, newest first. + */ + fun runsOfRoot( + contextId: ContextId, + root: ExtractionRunRef, + limit: Int, + since: Instant?, + ): List = runsOfRoot(contextId.value, root.runId, limit, since) + + /** + * Walks the parent chain up from the run at [key], nearest ancestor first. The run itself is + * not in the result. + * + * The walk is bounded and cycle-safe, and it needs to be both. [limit] stops it in a lineage + * deeper than the caller wants to read. A run already visited stops it outright: a value type + * can reject a run that is its own parent, but a two-hop cycle needs the other runs to see, so + * detecting one is the store's job. A store holding a cycle is corrupt; a store that hangs on + * one is worse. + * + * The walk also stops at the tenant boundary. A parent reference that resolves only in another + * tenant resolves to nothing here, so a chain read can never leak a neighbour's run. + * + * @param key The run to walk up from. + * @param limit The most ancestors to return. Must be positive. + * @return At most [limit] ancestors, parent first, ending early at an unresolvable parent or a + * run already seen. Empty if the run is a root, or is not stored. + * @throws IllegalArgumentException if [limit] is not positive. + */ + fun ancestorsOf(key: ExtractionRunKey, limit: Int): List +} + +/** + * A write named a run the store does not hold. + * + * EXPERIMENTAL. The shape may still change while extraction runs (DICE #67) land. + * + * @property key The run that was not found + */ +@ApiStatus.Experimental +class ExtractionRunNotFoundException(val key: ExtractionRunKey) : RuntimeException( + "no run ${key.runRef.runId} in context ${key.contextId.value}", +) + +/** + * A write disagreed with what the store already holds. + * + * Five cases, one per door: a terminal write whose fingerprint differs from the one that ended the + * run; a write, through [ExtractionRunStore.save] or [ExtractionRunStore.recordInvocation], against + * a run that has already ended; a save that disagrees with the stored run's lineage or start time + * (tenant cannot disagree — it is half of [ExtractionRunKey], so a different tenant always + * addresses an entirely separate run); a save whose [ExtractionRun.version] does not name the + * version currently stored; and an invocation record, through [ExtractionRunStore.recordInvocation] + * — the only door onto that state — that differs from an attempt the store already holds as + * terminal. `save` cannot raise that last case: it does not read or write invocation rows, so + * nothing it carries there is ever compared against one. + * + * The message never quotes a fingerprint in full or any part of a payload. It says which run and + * which rule, which is what an operator needs, and leaves the values to a deliberate read. + * + * EXPERIMENTAL. The shape may still change while extraction runs (DICE #67) land. + * + * @property key The run the write was against + */ +@ApiStatus.Experimental +class ExtractionRunConflictException( + val key: ExtractionRunKey, + message: String, +) : RuntimeException("run ${key.runRef.runId} in context ${key.contextId.value}: $message") diff --git a/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunTransition.kt b/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunTransition.kt new file mode 100644 index 00000000..14f1d9fa --- /dev/null +++ b/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunTransition.kt @@ -0,0 +1,238 @@ +/* + * Copyright 2024-2026 Embabel Pty Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.embabel.dice.proposition.extraction + +import org.jetbrains.annotations.ApiStatus +import java.time.Instant +import java.util.Collections + +/** + * One terminal write against a run: the state it ends in, when it ended, and the counts and + * failures that go with it. + * + * This type is how a run becomes a different run. [ExtractionRun] publishes no `withStatus`, no + * `finished()` and no `copy`, so [applyTo] is the only place a terminal run is derived. A store + * applies it under compare-and-set; nothing else may. + * + * **Null means keep, a value means replace.** [counts] and [failures] are both nullable and both + * follow the same rule, so a coordinator that only knows the run stopped can say that and nothing + * more. An empty [failures] list is a value: it replaces whatever the run accumulated with none. + * + * **A `FAILED` transition need not carry a failure.** A run can stop on something the coordinator + * classifies at the run level with no per-attempt detail, and requiring the pairing would make an + * honest "we know it failed and not why" unrecordable. A `COMPLETED` transition may carry failures + * for the same reason from the other side: a run that retried past a failed attempt and finished + * still happened. + * + * [fingerprint] is what a store compares a retry against; see [ExtractionRunFingerprint] for the + * encoding and why it is specified rather than left to a serializer. + * + * EXPERIMENTAL. The shape may still change while extraction runs (DICE #67) land. + * + * @property status The terminal state the run ends in + * @property finishedAt When the run reached it + * @property counts The run's final counts, or null to keep what the run already recorded + * @property failures The run's final failures, or null to keep what the run already recorded + */ +@ApiStatus.Experimental +class ExtractionRunTransition @JvmOverloads constructor( + val status: ExtractionRunStatus, + val finishedAt: Instant, + val counts: ExtractionRunCounts? = null, + failures: List? = null, +) { + + /** The run's final failures, or null to keep what the run already recorded. */ + val failures: List? = + failures?.let { Collections.unmodifiableList(ArrayList(it)) } + + init { + require(status.isTerminal) { + "a transition ends a run, so its status must be terminal; $status is not" + } + val declared = this.failures + if (declared != null) { + require(declared.size <= ExtractionRunLimits.MAX_FAILURES) { + "a run may record at most ${ExtractionRunLimits.MAX_FAILURES} failures, " + + "was ${declared.size}" + } + } + } + + /** + * The digest a store compares a repeated terminal write against, computed once at construction. + */ + val fingerprint: String = + ExtractionRunFingerprint.ofTerminal(status, finishedAt, counts, this.failures) + + /** + * Derives the terminal run this transition produces from the running one it is applied to. + * + * Everything the transition does not own is carried across unchanged: lineage, tenant, start + * time, profile, source revisions, digests, runtime identity, requested configuration, subject + * references, experiment and cohort labels, replay fidelity, the invocation records the run + * accumulated, and its header version. + * + * @param run The run to terminalize. Must be [ExtractionRunStatus.RUNNING]. + * @return The same run in its terminal state. + * @throws IllegalArgumentException if [run] is already terminal, or if [finishedAt] precedes + * its start, or if the resulting run breaks one of [ExtractionRun]'s own invariants — a + * failure naming an invocation the run has no record of, most often. + */ + fun applyTo(run: ExtractionRun): ExtractionRun { + require(run.status == ExtractionRunStatus.RUNNING) { + "a run in ${run.status} has already ended and cannot be transitioned to $status" + } + require(!finishedAt.isBefore(run.startedAt)) { + "finishedAt must not be before the run's startedAt" + } + return ExtractionRun( + contextId = run.contextId, + lineage = run.lineage, + status = status, + startedAt = run.startedAt, + finishedAt = finishedAt, + profile = run.profile, + sourceRevisions = run.sourceRevisions, + fingerprints = run.fingerprints, + runtime = run.runtime, + requestedModel = run.requestedModel, + subjectRefs = run.subjectRefs, + experimentRef = run.experimentRef, + cohortRef = run.cohortRef, + replayFidelity = run.replayFidelity, + counts = counts ?: run.counts, + invocations = run.invocations, + failures = failures ?: run.failures, + version = run.version, + ) + } + + override fun equals(other: Any?): Boolean { + if (this === other) return true + if (other !is ExtractionRunTransition) return false + return status == other.status && + finishedAt == other.finishedAt && + counts == other.counts && + failures == other.failures + } + + override fun hashCode(): Int { + var result = status.hashCode() + result = 31 * result + finishedAt.hashCode() + result = 31 * result + (counts?.hashCode() ?: 0) + result = 31 * result + (failures?.hashCode() ?: 0) + return result + } + + /** Identity and sizes. Failure details stay out of it, as they do on the run. */ + override fun toString(): String = + "ExtractionRunTransition(status=$status, finishedAt=$finishedAt, " + + "counts=${if (counts == null) "kept" else "replaced"}, " + + "failures=${failures?.size ?: "kept"}, fingerprint=${fingerprint.take(12)}…)" + + companion object { + + /** + * The run finished everything it was asked for: every product its request called for is + * durably persisted or terminally disposed. + * + * Only call this after persistence has returned. On the legacy path that is the coordinator + * once `persistAndProject` returns; on the #68 path it is inside the commit transaction. A + * run whose persistence never finished stays running and retryable, which is the whole + * reason the state exists. + * + * A run with zero products takes this too: there was nothing to persist, so the coverage + * claim holds vacuously. + */ + @JvmStatic + @JvmOverloads + fun completed( + finishedAt: Instant, + counts: ExtractionRunCounts? = null, + failures: List? = null, + ): ExtractionRunTransition = + ExtractionRunTransition(ExtractionRunStatus.COMPLETED, finishedAt, counts, failures) + + /** The run stopped on an error it could not get past. */ + @JvmStatic + @JvmOverloads + fun failed( + finishedAt: Instant, + counts: ExtractionRunCounts? = null, + failures: List? = null, + ): ExtractionRunTransition = + ExtractionRunTransition(ExtractionRunStatus.FAILED, finishedAt, counts, failures) + + /** + * The run stopped before finishing, by request or by external termination. + * + * This is also the abandonment path for a partially successful run nobody intends to + * finish. Its outstanding products stay outstanding behind it and recovery goes through a + * new run linked by parent or superseded reference. + */ + @JvmStatic + @JvmOverloads + fun cancelled( + finishedAt: Instant, + counts: ExtractionRunCounts? = null, + failures: List? = null, + ): ExtractionRunTransition = + ExtractionRunTransition(ExtractionRunStatus.CANCELLED, finishedAt, counts, failures) + } +} + +/** + * Whether a terminal write did the work or found it already done. + * + * Both are success. A caller retrying after a timeout it never saw the answer to gets + * [REPLAYED] and can carry on; the distinction is there for metrics and for a coordinator that + * wants to log the difference. + * + * EXPERIMENTAL. The shape may still change while extraction runs (DICE #67) land. + */ +@ApiStatus.Experimental +enum class ExtractionRunTransitionOutcome { + + /** The run was running, and this call ended it. */ + APPLIED, + + /** + * The run had already ended under a terminal write with the same fingerprint, and this call + * changed nothing. + */ + REPLAYED, +} + +/** + * What a terminal write returned: the run in its terminal state, and whether this call is what put + * it there. + * + * EXPERIMENTAL. The shape may still change while extraction runs (DICE #67) land. + * + * @property run The run, terminal + * @property outcome Whether this call applied the transition or replayed one already recorded + */ +@ApiStatus.Experimental +data class ExtractionRunTransitionResult( + val run: ExtractionRun, + val outcome: ExtractionRunTransitionOutcome, +) { + + /** True when this call ended the run rather than finding it already ended. */ + val isApplied: Boolean + get() = outcome == ExtractionRunTransitionOutcome.APPLIED +} diff --git a/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/InMemoryExtractionRunStore.kt b/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/InMemoryExtractionRunStore.kt new file mode 100644 index 00000000..0472e8db --- /dev/null +++ b/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/InMemoryExtractionRunStore.kt @@ -0,0 +1,331 @@ +/* + * Copyright 2024-2026 Embabel Pty Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.embabel.dice.proposition.extraction + +import org.jetbrains.annotations.ApiStatus +import java.time.Instant + +/** + * Reference [ExtractionRunStore] that keeps runs in a map. + * + * It is the executable statement of what the contract means, so a durable backend can be held to + * the same suite of tests. It also lets a host record and read runs before it has a database, which + * is most of what the first tier of run lineage is for. + * + * **Compare-and-set is real here, not simulated.** Every write and every read runs inside one + * monitor, so the read of a run's status and the write that changes it cannot interleave with + * another thread's. A durable store gets the same guarantee from its transaction. A reference + * implementation that read and then wrote without holding a lock would pass every single-threaded + * test and lie about the property the contract is named for. + * + * **A header save is itself compare-and-set, on [ExtractionRun.version] — and it owns header + * fields only.** Two writers can hold the same running header at once, and whichever saves second + * must not silently put the other's write back the way it looked before. `save` accepts a write + * only when the version it names matches what is stored; a stale writer is told so, in an + * [ExtractionRunConflictException], and has to read the run again and rebuild its save on what it + * holds now. See `save`'s own KDoc for why this replaced an earlier, field-by-field merge of the + * running header. Whatever [run]'s `invocations` field carries plays no part in that comparison or + * in what gets stored — see `save`'s own KDoc for why. + * + * **`recordInvocation` is the only door onto invocation rows, and each row keeps its own + * concurrency control.** Two attempts never contend on the header's version, because they are not + * writing the header; each is decided against the row already stored under its own + * `(invocationIndex, attempt)` key. + * + * **Scope is applied before the limit.** Each page filters to the tenant, then orders, then takes + * the limit. That order is the whole point of the contract's rule, so the reference implementation + * does it in the order a query would rather than filtering a truncated list. + * + * **There is no unscoped read, not even for tests.** One instance holds every tenant's runs, so a + * public "everything in the store" method would be a cross-tenant, unbounded read on a store whose + * contract is neither — and a host running the shipped in-memory backend would have one. The tests + * read through the contract like any other caller. + * + * Nothing here survives the JVM, and two instances know nothing about each other. + * + * EXPERIMENTAL. The shape may still change while extraction runs (DICE #67) land. + */ +@ApiStatus.Experimental +class InMemoryExtractionRunStore : ExtractionRunStore { + + private val lock = Any() + + private val runs = LinkedHashMap() + + /** The fingerprint of the terminal write that ended each run, for comparing a retry against. */ + private val terminalWrites = HashMap() + + override fun save(run: ExtractionRun): ExtractionRun { + require(run.status == ExtractionRunStatus.RUNNING) { + "save records a running run; ${run.status} is terminal and belongs to transition()" + } + // ExtractionRun leaves status-and-timing pairing to the state machine, which is here. A + // running run with a finish time is a record that reads as running and as finished at once, + // and every page and audit that meets it has to guess which. + require(run.finishedAt == null) { + "a running run has not finished, so it carries no finishedAt" + } + val key = run.key() + synchronized(lock) { + val stored = runs[key] + if (stored == null) { + require(run.version == 0L) { + "a run's first save must name version 0, the version a run nobody has saved " + + "yet carries; this one names ${run.version}" + } + // The insert is header-only, the same as every later save: whatever `run.invocations` + // carries is not this door's to write. A run has no invocation rows before it exists, + // so there is nothing to preserve here, but the rule reads the same either way — this + // door never originates a row. + val inserted = rebuild(run, invocations = emptyList(), version = 0L) + runs[key] = inserted + return inserted + } + if (stored.status.isTerminal) { + throw ExtractionRunConflictException( + key, + "already ended as ${stored.status} and cannot be re-opened by a save", + ) + } + if (stored.lineage != run.lineage) { + throw ExtractionRunConflictException( + key, + "stored lineage differs from the one being saved; lineage is fixed at insert", + ) + } + if (stored.startedAt != run.startedAt) { + throw ExtractionRunConflictException( + key, + "stored startedAt differs from the one being saved; a run starts once", + ) + } + // The stored invocation rows are carried through untouched, regardless of what + // `run.invocations` holds — recordInvocation is the only door onto that state. + // Building the no-op candidate at the stored invocations and the stored version turns a + // byte-identical header resend into a check against itself, so a retry that never + // learned its first attempt landed does not get an avoidable conflict. + val candidate = rebuild(run, invocations = stored.invocations, version = stored.version) + if (candidate == stored) { + return stored + } + if (run.version != stored.version) { + throw ExtractionRunConflictException( + key, + "the header was read at version ${run.version}; the store is now at " + + "${stored.version}. Read the run again with findRun and rebuild this save " + + "on what it holds now", + ) + } + val updated = rebuild(run, invocations = stored.invocations, version = stored.version + 1) + runs[key] = updated + return updated + } + } + + override fun recordInvocation( + key: ExtractionRunKey, + record: ExtractionInvocationRecord, + ): ExtractionRun { + synchronized(lock) { + val stored = runs[key] ?: throw ExtractionRunNotFoundException(key) + if (stored.status.isTerminal) { + throw ExtractionRunConflictException( + key, + "already ended as ${stored.status}; its invocation records are part of how it ended", + ) + } + val invocations = applyInvocationWrite(key, stored.invocations, record) + // The version tracks header writes exclusively, so recording an attempt leaves it + // exactly where it stood: a save built on the header this call started from still names + // the current version and is accepted, and it carries the recorded attempt forward + // because save no longer touches invocation rows at all. + val updated = rebuild(stored, invocations = invocations, version = stored.version) + runs[key] = updated + return updated + } + } + + override fun transition( + key: ExtractionRunKey, + transition: ExtractionRunTransition, + ): ExtractionRunTransitionResult { + synchronized(lock) { + val stored = runs[key] ?: throw ExtractionRunNotFoundException(key) + if (stored.status.isTerminal) { + val recorded = terminalWrites[key] + if (recorded == transition.fingerprint) { + return ExtractionRunTransitionResult( + stored, + ExtractionRunTransitionOutcome.REPLAYED, + ) + } + throw ExtractionRunConflictException( + key, + "already ended as ${stored.status} under a different terminal write; " + + "this one claims ${transition.status}", + ) + } + val terminal = transition.applyTo(stored) + runs[key] = terminal + terminalWrites[key] = transition.fingerprint + return ExtractionRunTransitionResult(terminal, ExtractionRunTransitionOutcome.APPLIED) + } + } + + override fun findRun(key: ExtractionRunKey): ExtractionRun? = synchronized(lock) { runs[key] } + + override fun invocationsOf(key: ExtractionRunKey): List = + synchronized(lock) { runs[key]?.invocationsInPlanOrder().orEmpty() } + + override fun runsInContext( + contextIdValue: String, + limit: Int, + since: Instant?, + ): List = page(limit) { run -> + run.contextId.value == contextIdValue && startedAtOrAfter(run, since) + } + + override fun childrenOf( + contextIdValue: String, + parentRunId: String, + limit: Int, + ): List = page(limit) { run -> + run.contextId.value == contextIdValue && run.parentRef?.runId == parentRunId + } + + override fun runsOfRoot( + contextIdValue: String, + rootRunId: String, + limit: Int, + since: Instant?, + ): List = page(limit) { run -> + run.contextId.value == contextIdValue && + run.rootRef.runId == rootRunId && + startedAtOrAfter(run, since) + } + + override fun ancestorsOf(key: ExtractionRunKey, limit: Int): List { + requirePositiveLimit(limit) + synchronized(lock) { + val start = runs[key] ?: return emptyList() + val walked = mutableListOf() + val seen = mutableSetOf(start.ref) + var parentRef = start.parentRef + while (parentRef != null && walked.size < limit && seen.add(parentRef)) { + // Scoped to the starting run's tenant, so a parent id that exists only in another + // tenant resolves to nothing and the walk stops here. + val parent = runs[ExtractionRunKey(key.contextId, parentRef)] ?: break + walked += parent + parentRef = parent.parentRef + } + return walked + } + } + + /** + * Filter, then order, then limit — in that order, because a page that limited first would drop + * a tenant's runs behind a busier neighbour's and report the shortfall as an empty tenant. + */ + private fun page(limit: Int, matches: (ExtractionRun) -> Boolean): List { + requirePositiveLimit(limit) + return synchronized(lock) { + runs.values + .filter(matches) + .sortedWith(NEWEST_FIRST) + .take(limit) + } + } + + private fun startedAtOrAfter(run: ExtractionRun, since: Instant?): Boolean = + since == null || !run.startedAt.isBefore(since) + + private fun requirePositiveLimit(limit: Int) { + require(limit > 0) { "limit must be positive, was $limit" } + } + + /** Re-lists the run with its invocations and version replaced, since [ExtractionRun] publishes + * no `copy`. */ + private fun rebuild( + run: ExtractionRun, + invocations: List, + version: Long, + ): ExtractionRun = ExtractionRun( + contextId = run.contextId, + lineage = run.lineage, + status = run.status, + startedAt = run.startedAt, + finishedAt = run.finishedAt, + profile = run.profile, + sourceRevisions = run.sourceRevisions, + fingerprints = run.fingerprints, + runtime = run.runtime, + requestedModel = run.requestedModel, + subjectRefs = run.subjectRefs, + experimentRef = run.experimentRef, + cohortRef = run.cohortRef, + replayFidelity = run.replayFidelity, + counts = run.counts, + invocations = invocations, + failures = run.failures, + version = version, + ) + + /** + * Inserts or compares one invocation record against the rows already stored — the whole of + * [recordInvocation]'s write, on the row's own `(invocationIndex, attempt)` key. + * + * A record for an id not yet stored is inserted. A record for an id stored with an + * [ExtractionInvocationOutcome.IN_FLIGHT] outcome replaces it in place: that is how a call's + * observed facts fill in while it runs. Once an id's stored record is terminal it is locked — an + * incoming record for that id is accepted only when it equals the stored one exactly, an + * idempotent replay, and every other write for that id is rejected: a different outcome, or the + * same outcome carrying different timing, usage or provider facts, both count as rewriting how + * the attempt ended. + * + * **An existing id is replaced in place, at its stored position; only a genuinely new id is + * appended.** [ExtractionRun.equals] compares this list by position, so a resend of a record + * already stored has to land back where it already was. An identical replay landing anywhere + * else would read as a change: it could move the header's version on what should be a no-op, or + * turn a stale but otherwise identical resend into a rejection, breaking the no-op the store + * promises everywhere else. `save` no longer calls this — it does not touch invocation rows at + * all — so each call inserts or compares exactly one row, always a single record. + */ + private fun applyInvocationWrite( + key: ExtractionRunKey, + stored: List, + record: ExtractionInvocationRecord, + ): List { + val existing = stored.find { it.id == record.id } + if (existing == null) { + return stored + record + } + if (existing.outcome.isTerminal && record != existing) { + throw ExtractionRunConflictException( + key, + "${existing.id} already ended as ${existing.outcome}; once an attempt is " + + "terminal only an identical write replays, and this one differs", + ) + } + return stored.map { if (it.id == record.id) record else it } + } + + private companion object { + + /** Newest start first, then run id ascending so a page is repeatable. */ + private val NEWEST_FIRST: Comparator = + compareByDescending { it.startedAt }.thenBy { it.ref.runId } + } +} diff --git a/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunContractTest.kt b/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunContractTest.kt index ee5bcd8c..57295657 100644 --- a/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunContractTest.kt +++ b/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunContractTest.kt @@ -345,9 +345,10 @@ class ExtractionRunContractTest { // failure whose invocation it has no record of. "invocations" to base.copyWith(invocations = base.invocations.drop(1)), "failures" to base.copyWith(failures = emptyList()), + "version" to base.copyWith(version = base.version + 1), ) - assertThat(variants).hasSize(17) + assertThat(variants).hasSize(18) variants.forEach { (component, variant) -> assertThat(variant) .describedAs("a run differing only in %s", component) @@ -421,6 +422,7 @@ private fun ExtractionRun.copyWith( counts: ExtractionRunCounts = this.counts, invocations: List = this.invocations, failures: List = this.failures, + version: Long = this.version, ): ExtractionRun = ExtractionRun( contextId = contextId, lineage = lineage, @@ -439,4 +441,5 @@ private fun ExtractionRun.copyWith( counts = counts, invocations = invocations, failures = failures, + version = version, ) diff --git a/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunFingerprintTest.kt b/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunFingerprintTest.kt new file mode 100644 index 00000000..be1420c5 --- /dev/null +++ b/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunFingerprintTest.kt @@ -0,0 +1,307 @@ +/* + * Copyright 2024-2026 Embabel Pty Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.embabel.dice.proposition.extraction + +import com.embabel.dice.proposition.extraction.ExtractionRunFixtures.FINISHED_AT +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test +import java.time.Instant + +/** + * The canonical encoding behind terminal-write idempotency. + * + * Two things have to hold. Two payloads that mean the same must produce the same digest, or a + * correct retry is rejected as an incompatible rewrite. Two payloads that differ must produce + * different digests, or an incompatible rewrite is accepted as a retry — which is the failure that + * loses audit evidence. + */ +class ExtractionRunFingerprintTest { + + private fun failure( + code: ExtractionFailureCode, + stage: ExtractionFailureStage? = null, + at: Instant = FINISHED_AT, + invocation: ExtractionInvocationId? = null, + providerStatus: Int? = null, + measure: ExtractionFailureMeasure? = null, + ) = ExtractionFailure( + code = code, + stage = stage, + providerStatus = providerStatus, + measure = measure, + at = at, + invocation = invocation, + ) + + // ---- order independence ---- + + @Test + fun `the failure list's order does not move the fingerprint`() { + val one = failure(ExtractionFailureCode.RATE_LIMITED, ExtractionFailureStage.MODEL_CALL) + val two = failure(ExtractionFailureCode.DECODE_FAILED, ExtractionFailureStage.RESPONSE_DECODE) + val three = failure(ExtractionFailureCode.MODEL_TIMEOUT, invocation = ExtractionInvocationId(1, 2)) + + val forwards = ExtractionRunTransition.failed(FINISHED_AT, failures = listOf(one, two, three)) + val backwards = ExtractionRunTransition.failed(FINISHED_AT, failures = listOf(three, two, one)) + val shuffled = ExtractionRunTransition.failed(FINISHED_AT, failures = listOf(two, three, one)) + + assertThat(forwards.fingerprint).isEqualTo(backwards.fingerprint) + assertThat(forwards.fingerprint).isEqualTo(shuffled.fingerprint) + } + + @Test + fun `two failures that differ only in one field still differ`() { + val base = failure( + ExtractionFailureCode.MODEL_TIMEOUT, + ExtractionFailureStage.MODEL_CALL, + FINISHED_AT, + ExtractionInvocationId(1, 2), + ) + val variants = listOf( + "code" to failure( + ExtractionFailureCode.RATE_LIMITED, + ExtractionFailureStage.MODEL_CALL, + FINISHED_AT, + ExtractionInvocationId(1, 2), + ), + "stage" to failure( + ExtractionFailureCode.MODEL_TIMEOUT, + ExtractionFailureStage.RESPONSE_DECODE, + FINISHED_AT, + ExtractionInvocationId(1, 2), + ), + "no stage" to failure( + ExtractionFailureCode.MODEL_TIMEOUT, + null, + FINISHED_AT, + ExtractionInvocationId(1, 2), + ), + "providerStatus" to failure( + ExtractionFailureCode.MODEL_TIMEOUT, + ExtractionFailureStage.MODEL_CALL, + FINISHED_AT, + ExtractionInvocationId(1, 2), + providerStatus = 504, + ), + "measure" to failure( + ExtractionFailureCode.MODEL_TIMEOUT, + ExtractionFailureStage.MODEL_CALL, + FINISHED_AT, + ExtractionInvocationId(1, 2), + measure = ExtractionFailureMeasure(ExtractionFailureQuantity.ELAPSED_MILLIS, 30_000), + ), + "at" to failure( + ExtractionFailureCode.MODEL_TIMEOUT, + ExtractionFailureStage.MODEL_CALL, + FINISHED_AT.plusNanos(1), + ExtractionInvocationId(1, 2), + ), + "invocation index" to failure( + ExtractionFailureCode.MODEL_TIMEOUT, + ExtractionFailureStage.MODEL_CALL, + FINISHED_AT, + ExtractionInvocationId(2, 2), + ), + "attempt" to failure( + ExtractionFailureCode.MODEL_TIMEOUT, + ExtractionFailureStage.MODEL_CALL, + FINISHED_AT, + ExtractionInvocationId(1, 3), + ), + "no invocation" to failure( + ExtractionFailureCode.MODEL_TIMEOUT, + ExtractionFailureStage.MODEL_CALL, + FINISHED_AT, + null, + ), + ) + val baseline = ExtractionRunTransition.failed(FINISHED_AT, failures = listOf(base)).fingerprint + + variants.forEach { (name, variant) -> + assertThat(ExtractionRunTransition.failed(FINISHED_AT, failures = listOf(variant)).fingerprint) + .describedAs("a failure differing in %s", name) + .isNotEqualTo(baseline) + } + } + + // ---- the whole payload ---- + + @Test + fun `changing any component of a terminal write changes its fingerprint`() { + val baseline = ExtractionRunTransition.completed( + finishedAt = FINISHED_AT, + counts = ExtractionRunCounts(propositionsExtracted = 3, propositionsPersisted = 3), + failures = listOf(failure(ExtractionFailureCode.RATE_LIMITED, ExtractionFailureStage.MODEL_CALL)), + ) + val variants = mapOf( + "status" to ExtractionRunTransition.cancelled( + finishedAt = FINISHED_AT, + counts = baseline.counts, + failures = baseline.failures, + ), + "finishedAt" to ExtractionRunTransition.completed( + finishedAt = FINISHED_AT.plusNanos(1), + counts = baseline.counts, + failures = baseline.failures, + ), + "counts" to ExtractionRunTransition.completed( + finishedAt = FINISHED_AT, + counts = ExtractionRunCounts(propositionsExtracted = 3, propositionsPersisted = 2), + failures = baseline.failures, + ), + "failures" to ExtractionRunTransition.completed( + finishedAt = FINISHED_AT, + counts = baseline.counts, + failures = emptyList(), + ), + ) + + variants.forEach { (name, variant) -> + assertThat(variant.fingerprint) + .describedAs("a terminal write differing in %s", name) + .isNotEqualTo(baseline.fingerprint) + } + } + + @Test + fun `absent and empty are different claims`() { + // Null means keep what the run recorded and a value means replace it, so the encoding has + // to tell them apart or a store would replay one as the other. + val keptCounts = ExtractionRunTransition.completed(FINISHED_AT, counts = null) + val zeroCounts = ExtractionRunTransition.completed(FINISHED_AT, counts = ExtractionRunCounts()) + assertThat(keptCounts.fingerprint).isNotEqualTo(zeroCounts.fingerprint) + + val keptFailures = ExtractionRunTransition.completed(FINISHED_AT, failures = null) + val noFailures = ExtractionRunTransition.completed(FINISHED_AT, failures = emptyList()) + assertThat(keptFailures.fingerprint).isNotEqualTo(noFailures.fingerprint) + } + + @Test + fun `one failure never collides with two that carry the same field values between them`() { + // Length-prefixing and the count prefix are what make this hold. A delimiter-joined + // encoding would render ["a|b"] and ["a", "b"] the same way, and a list with no count + // in front of it lets a shorter list be a prefix of a longer one. + val single = ExtractionRunTransition.failed( + FINISHED_AT, + failures = listOf( + failure(ExtractionFailureCode.INTERNAL, ExtractionFailureStage.CHUNKING, providerStatus = 500), + ), + ) + val split = ExtractionRunTransition.failed( + FINISHED_AT, + failures = listOf( + failure(ExtractionFailureCode.INTERNAL, ExtractionFailureStage.CHUNKING), + failure(ExtractionFailureCode.INTERNAL, providerStatus = 500), + ), + ) + + assertThat(single.fingerprint).isNotEqualTo(split.fingerprint) + } + + @Test + fun `an instant is encoded at full precision whichever way it was built`() { + // Sub-second precision survives: a run that finished 1ns after another is a different + // terminal write, and an encoding that rendered to seconds would replay one as the other. + val whole = Instant.ofEpochSecond(1_800_000_000L) + val precisions = listOf( + whole, + whole.plusMillis(500), + whole.plusNanos(1_000), + whole.plusNanos(1), + ) + + val digests = precisions.map { ExtractionRunTransition.completed(it).fingerprint } + assertThat(digests).doesNotHaveDuplicates() + + // The digest is a function of the instant's value, not of how the caller built it. + assertThat(ExtractionRunTransition.completed(Instant.parse("2027-01-15T08:00:00Z")).fingerprint) + .isEqualTo(ExtractionRunTransition.completed(Instant.ofEpochMilli(1_800_000_000_000L)).fingerprint) + + // Nothing here pins the rendering itself. `the digest of a fixed terminal write is pinned` + // is what catches a change to it, which is the same discipline MetamodelVersion's content + // hash uses: one golden literal over one fixed payload. + } + + // ---- the persisted format ---- + + @Test + fun `the digest of a fixed terminal write is pinned`() { + // This is a persisted format: a store keeps the digest beside the run and compares a retry + // against it. Changing the encoding makes every recorded fingerprint unmatchable, so a + // correct retry against an existing run would be rejected as an incompatible rewrite. + // Moving this literal is how that decision gets made deliberately. + val pinned = ExtractionRunTransition.completed( + finishedAt = Instant.parse("2026-08-31T10:15:47Z"), + counts = ExtractionRunCounts( + sourcesRead = 2, + chunksProcessed = 6, + propositionsExtracted = 14, + propositionsPersisted = 11, + propositionsRejected = 3, + entitiesResolved = 9, + ), + failures = listOf( + ExtractionFailure( + code = ExtractionFailureCode.DECODE_FAILED, + stage = ExtractionFailureStage.RESPONSE_DECODE, + providerStatus = 502, + measure = ExtractionFailureMeasure(ExtractionFailureQuantity.CHARACTER_COUNT, 4_128), + at = Instant.parse("2026-08-31T10:15:47Z"), + invocation = ExtractionInvocationId(1, 2), + ), + ), + ) + + assertThat(pinned.fingerprint) + .isEqualTo("66e8ef75aeb2c1f12b241aebd9f943dc26c654fb18cb4181ed8cecd4994c4309") + } + + @Test + fun `a fingerprint is a lowercase sha-256 hex digest`() { + val fingerprint = ExtractionRunTransition.completed(FINISHED_AT).fingerprint + + assertThat(fingerprint).hasSize(64) + assertThat(fingerprint).matches("[0-9a-f]{64}") + } + + @Test + fun `the encoding carries a version tag`() { + // A reader meeting a version it does not know matches nothing rather than guessing. + assertThat(ExtractionRunFingerprint.TERMINAL_VERSION).isEqualTo("xrun-terminal:v1") + } + + @Test + fun `the fingerprint is a function of the payload and nothing else`() { + val transition = ExtractionRunTransition.completed(FINISHED_AT) + + assertThat(transition.fingerprint).isEqualTo( + ExtractionRunFingerprint.ofTerminal( + status = ExtractionRunStatus.COMPLETED, + finishedAt = FINISHED_AT, + counts = null, + failures = null, + ), + ) + } + + @Test + fun `a fingerprint never appears in full in a transition's toString`() { + val transition = ExtractionRunTransition.completed(FINISHED_AT) + + assertThat(transition.toString()).doesNotContain(transition.fingerprint) + assertThat(transition.toString()).contains(transition.fingerprint.take(12)) + } +} diff --git a/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunFixtures.kt b/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunFixtures.kt index 4c7a70d4..b3f6fe5a 100644 --- a/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunFixtures.kt +++ b/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunFixtures.kt @@ -74,6 +74,40 @@ internal object ExtractionRunFixtures { startedAt = STARTED_AT, ) + /** + * A running run with the two things the store's pages and chains key on — the lineage and the + * start time — under the caller's control. + */ + fun runningRun( + contextId: ContextId = CONTEXT, + lineage: ExtractionRunLineage, + startedAt: Instant = STARTED_AT, + counts: ExtractionRunCounts = ExtractionRunCounts(), + invocations: List = emptyList(), + ): ExtractionRun = ExtractionRun( + contextId = contextId, + lineage = lineage, + status = ExtractionRunStatus.RUNNING, + startedAt = startedAt, + counts = counts, + invocations = invocations, + ) + + /** A root-lineage running run named by its run id, which is all most store tests need. */ + fun runningRun( + runId: String, + contextId: ContextId = CONTEXT, + startedAt: Instant = STARTED_AT, + ): ExtractionRun = runningRun( + contextId = contextId, + lineage = ExtractionRunLineage.root(ExtractionRunRef(runId)), + startedAt = startedAt, + ) + + /** The key of a run in the default tenant. */ + fun keyOf(runId: String, contextId: ContextId = CONTEXT): ExtractionRunKey = + ExtractionRunKey(contextId, ExtractionRunRef(runId)) + /** * A run with every field populated, which is what the privacy assertions dump. * diff --git a/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunLifecycleTest.kt b/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunLifecycleTest.kt new file mode 100644 index 00000000..aff17bdb --- /dev/null +++ b/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunLifecycleTest.kt @@ -0,0 +1,599 @@ +/* + * Copyright 2024-2026 Embabel Pty Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.embabel.dice.proposition.extraction + +import com.embabel.dice.proposition.extraction.ExtractionRunFixtures.CONTEXT +import com.embabel.dice.proposition.extraction.ExtractionRunFixtures.FINISHED_AT +import com.embabel.dice.proposition.extraction.ExtractionRunFixtures.OTHER_CONTEXT +import com.embabel.dice.proposition.extraction.ExtractionRunFixtures.STARTED_AT +import org.assertj.core.api.Assertions.assertThat +import org.assertj.core.api.Assertions.assertThatIllegalArgumentException +import org.assertj.core.api.Assertions.assertThatThrownBy +import org.junit.jupiter.api.Test + +/** + * Every edge of the run lifecycle, and the two rules that make the terminal ones safe: only + * compare-and-set writes them, and a repeat of one is decided by comparing fingerprints rather than + * by overwriting. + */ +class ExtractionRunLifecycleTest { + + private val store = InMemoryExtractionRunStore() + + private fun started(runId: String = "run-1") = ExtractionRunFixtures.runningRun(runId) + + // ---- the three legal edges ---- + + @Test + fun `a running run may end completed, failed or cancelled`() { + val edges = listOf( + ExtractionRunStatus.COMPLETED to ExtractionRunTransition.completed(FINISHED_AT), + ExtractionRunStatus.FAILED to ExtractionRunTransition.failed(FINISHED_AT), + ExtractionRunStatus.CANCELLED to ExtractionRunTransition.cancelled(FINISHED_AT), + ) + + edges.forEachIndexed { index, (expected, transition) -> + val run = started("run-edge-$index") + store.save(run) + + val result = store.transition(run.key(), transition) + + assertThat(result.outcome) + .describedAs("RUNNING to %s", expected) + .isEqualTo(ExtractionRunTransitionOutcome.APPLIED) + assertThat(result.isApplied).isTrue() + assertThat(result.run.status).isEqualTo(expected) + assertThat(result.run.finishedAt).isEqualTo(FINISHED_AT) + assertThat(store.findRun(run.key())?.status).isEqualTo(expected) + } + } + + @Test + fun `a transition must be terminal`() { + // RUNNING to RUNNING is not an edge, and the transition type will not hold the value. + assertThatIllegalArgumentException() + .isThrownBy { ExtractionRunTransition(ExtractionRunStatus.RUNNING, FINISHED_AT) } + .withMessageContaining("must be terminal") + } + + // ---- no edge leaves a terminal state ---- + + @Test + fun `a run that has ended cannot end again as something else`() { + val terminals = listOf( + ExtractionRunTransition.completed(FINISHED_AT), + ExtractionRunTransition.failed(FINISHED_AT), + ExtractionRunTransition.cancelled(FINISHED_AT), + ) + + terminals.forEachIndexed { index, first -> + val run = started("run-terminal-$index") + store.save(run) + store.transition(run.key(), first) + + terminals.filter { it.status != first.status }.forEach { second -> + assertThatThrownBy { store.transition(run.key(), second) } + .describedAs("%s then %s", first.status, second.status) + .isInstanceOf(ExtractionRunConflictException::class.java) + .hasMessageContaining("already ended as ${first.status}") + } + + assertThat(store.findRun(run.key())?.status).isEqualTo(first.status) + } + } + + @Test + fun `a terminal run cannot be re-opened by a save`() { + val run = started() + store.save(run) + store.transition(run.key(), ExtractionRunTransition.completed(FINISHED_AT)) + + // The same running header that was legal a moment ago is now a rewrite of a finished + // record, which is the write the design forbids MERGE from doing quietly. + assertThatThrownBy { store.save(run) } + .isInstanceOf(ExtractionRunConflictException::class.java) + .hasMessageContaining("cannot be re-opened") + + assertThat(store.findRun(run.key())?.status).isEqualTo(ExtractionRunStatus.COMPLETED) + } + + @Test + fun `a terminal run takes no more invocation records`() { + val run = started() + store.save(run) + store.transition(run.key(), ExtractionRunTransition.failed(FINISHED_AT)) + + assertThatThrownBy { + store.recordInvocation(run.key(), ExtractionInvocationRecord.planned(0)) + } + .isInstanceOf(ExtractionRunConflictException::class.java) + .hasMessageContaining("part of how it ended") + } + + // ---- idempotent replay ---- + + @Test + fun `the same terminal write replays as success and changes nothing`() { + val run = started() + store.save(run) + val transition = ExtractionRunTransition.completed( + finishedAt = FINISHED_AT, + counts = ExtractionRunCounts(propositionsPersisted = 11), + ) + + val first = store.transition(run.key(), transition) + val second = store.transition(run.key(), transition) + // A retry that rebuilt the payload rather than keeping the object still replays: the + // comparison is on the fingerprint, not on identity. + val third = store.transition( + run.key(), + ExtractionRunTransition.completed( + finishedAt = FINISHED_AT, + counts = ExtractionRunCounts(propositionsPersisted = 11), + ), + ) + + assertThat(first.outcome).isEqualTo(ExtractionRunTransitionOutcome.APPLIED) + assertThat(second.outcome).isEqualTo(ExtractionRunTransitionOutcome.REPLAYED) + assertThat(third.outcome).isEqualTo(ExtractionRunTransitionOutcome.REPLAYED) + assertThat(second.run).isEqualTo(first.run) + assertThat(third.run).isEqualTo(first.run) + } + + @Test + fun `a replay after another invocation was recorded is still a replay`() { + val run = started() + store.save(run) + store.recordInvocation(run.key(), ExtractionInvocationRecord.planned(0)) + val transition = ExtractionRunTransition.completed(FINISHED_AT) + store.transition(run.key(), transition) + + // The fingerprint covers the terminal write, not the run. A coordinator that recorded + // another attempt between a terminal write it never saw the answer to and its retry made + // the same terminal write both times. + assertThat(store.transition(run.key(), transition).outcome) + .isEqualTo(ExtractionRunTransitionOutcome.REPLAYED) + } + + // ---- incompatible rewrite ---- + + @Test + fun `an incompatible terminal rewrite is rejected rather than overwriting`() { + val incompatible = listOf( + "a different status" to ExtractionRunTransition.failed(FINISHED_AT), + "a different finish time" to ExtractionRunTransition.completed(FINISHED_AT.plusSeconds(1)), + "different counts" to ExtractionRunTransition.completed( + finishedAt = FINISHED_AT, + counts = ExtractionRunCounts(propositionsPersisted = 12), + ), + "a different failure list" to ExtractionRunTransition.completed( + finishedAt = FINISHED_AT, + failures = listOf(ExtractionFailure(ExtractionFailureCode.RATE_LIMITED)), + ), + ) + val recorded = ExtractionRunTransition.completed( + finishedAt = FINISHED_AT, + counts = ExtractionRunCounts(propositionsPersisted = 11), + ) + + incompatible.forEachIndexed { index, (name, rewrite) -> + val run = started("run-rewrite-$index") + store.save(run) + store.transition(run.key(), recorded) + + assertThatThrownBy { store.transition(run.key(), rewrite) } + .describedAs(name) + .isInstanceOf(ExtractionRunConflictException::class.java) + + assertThat(store.findRun(run.key())) + .describedAs("%s left the recorded terminal write in place", name) + .isEqualTo(recorded.applyTo(started("run-rewrite-$index"))) + } + } + + @Test + fun `keeping counts and replacing them with the same values are the same terminal write`() { + val run = ExtractionRunFixtures.runningRun( + lineage = ExtractionRunLineage.root(ExtractionRunRef("run-counts")), + counts = ExtractionRunCounts(propositionsPersisted = 4), + ) + store.save(run) + store.transition(run.key(), ExtractionRunTransition.completed(FINISHED_AT, counts = null)) + + // Null means keep and a value means replace, so they are different claims even when they + // land on the same numbers. Rejecting the second is the honest answer: the store cannot + // tell a caller who meant "leave them" from one who meant "these are final". + assertThatThrownBy { + store.transition( + run.key(), + ExtractionRunTransition.completed( + FINISHED_AT, + counts = ExtractionRunCounts(propositionsPersisted = 4), + ), + ) + }.isInstanceOf(ExtractionRunConflictException::class.java) + } + + // ---- nothing reaches COMPLETED except through the transition ---- + + @Test + fun `a terminal run cannot be saved`() { + ExtractionRunStatus.entries.filter { it.isTerminal }.forEach { status -> + val terminal = ExtractionRun( + contextId = CONTEXT, + lineage = ExtractionRunLineage.root(ExtractionRunRef("run-direct-$status")), + status = status, + startedAt = STARTED_AT, + finishedAt = FINISHED_AT, + ) + + assertThatIllegalArgumentException() + .describedAs("saving a %s run", status) + .isThrownBy { store.save(terminal) } + .withMessageContaining("belongs to transition()") + } + } + + @Test + fun `nothing can complete a run the store has never seen`() { + // COMPLETED asserts every requested product is persisted or disposed. A run nobody started + // has no request behind it, so the claim has nothing to be true of. + assertThatThrownBy { + store.transition( + ExtractionRunFixtures.keyOf("run-never-started"), + ExtractionRunTransition.completed(FINISHED_AT), + ) + }.isInstanceOf(ExtractionRunNotFoundException::class.java) + + // Read back through the contract, which is the only way in: the store publishes no + // unscoped "everything" method, because one instance holds every tenant's runs. + assertThat(store.findRun(ExtractionRunFixtures.keyOf("run-never-started"))).isNull() + assertThat(store.runsInContext(CONTEXT, 10, null)).isEmpty() + assertThat(store.runsInContext(OTHER_CONTEXT, 10, null)).isEmpty() + } + + @Test + fun `the store publishes no unscoped read`() { + // One instance holds every tenant's runs, so an "everything in the store" method would be a + // cross-tenant unbounded read on a contract that is neither — and a host running the + // shipped in-memory backend would have one. + val unscoped = InMemoryExtractionRunStore::class.java.methods + .filter { java.lang.reflect.Modifier.isPublic(it.modifiers) } + .filter { List::class.java.isAssignableFrom(it.returnType) } + .filter { method -> method.parameterCount == 0 } + .map { it.name } + + assertThat(unscoped).isEmpty() + } + + @Test + fun `the only writer of a terminal status is the transition`() { + // Structural, not narrative: save rejects every terminal status, so the sole path from + // RUNNING to COMPLETED is transition(), whose precondition is written on the contract. + val writers = ExtractionRunStore::class.java.methods + .filter { it.name == "save" || it.name == "recordInvocation" || it.name == "transition" } + .map { it.name } + .toSet() + + assertThat(writers).containsExactlyInAnyOrder("save", "recordInvocation", "transition") + + val run = started("run-sole-writer") + store.save(run) + assertThat(store.findRun(run.key())?.status).isEqualTo(ExtractionRunStatus.RUNNING) + store.recordInvocation(run.key(), ExtractionInvocationRecord.planned(0)) + assertThat(store.findRun(run.key())?.status).isEqualTo(ExtractionRunStatus.RUNNING) + store.transition(run.key(), ExtractionRunTransition.completed(FINISHED_AT)) + assertThat(store.findRun(run.key())?.status).isEqualTo(ExtractionRunStatus.COMPLETED) + } + + // ---- the empty run ---- + + @Test + fun `an empty run completes vacuously`() { + val run = started("run-empty") + store.save(run) + + val result = store.transition(run.key(), ExtractionRunTransition.completed(FINISHED_AT)) + + assertThat(result.run.status).isEqualTo(ExtractionRunStatus.COMPLETED) + assertThat(result.run.invocations).isEmpty() + assertThat(result.run.failures).isEmpty() + assertThat(result.run.counts).isEqualTo(ExtractionRunCounts()) + assertThat(store.invocationsOf(run.key())).isEmpty() + } + + // ---- the running state stays retryable ---- + + @Test + fun `a run stays running and re-savable until something ends it`() { + val run = started("run-partial") + store.save(run) + + // Partial success: products persisted, more outstanding. The header is updated and the run + // is still RUNNING, so the commit that completes coverage can still terminalize it. + val progressed = ExtractionRunFixtures.runningRun( + lineage = run.lineage, + counts = ExtractionRunCounts(propositionsExtracted = 9, propositionsPersisted = 4), + ) + store.save(progressed) + + assertThat(store.findRun(run.key())?.status).isEqualTo(ExtractionRunStatus.RUNNING) + assertThat(store.findRun(run.key())?.counts?.propositionsPersisted).isEqualTo(4) + + val result = store.transition( + run.key(), + ExtractionRunTransition.completed( + FINISHED_AT, + counts = ExtractionRunCounts(propositionsExtracted = 9, propositionsPersisted = 9), + ), + ) + assertThat(result.outcome).isEqualTo(ExtractionRunTransitionOutcome.APPLIED) + assertThat(result.run.counts.propositionsPersisted).isEqualTo(9) + } + + @Test + fun `a save cannot change what fixes a run's identity`() { + val run = started("run-identity") + store.save(run) + + val differentStart = ExtractionRunFixtures.runningRun( + lineage = run.lineage, + startedAt = STARTED_AT.plusSeconds(30), + ) + assertThatThrownBy { store.save(differentStart) } + .isInstanceOf(ExtractionRunConflictException::class.java) + .hasMessageContaining("a run starts once") + + val differentLineage = ExtractionRunFixtures.runningRun( + lineage = ExtractionRunLineage.root( + runRef = ExtractionRunRef("run-identity"), + supersedesRunRef = ExtractionRunRef("run-something-else"), + ), + ) + assertThatThrownBy { store.save(differentLineage) } + .isInstanceOf(ExtractionRunConflictException::class.java) + .hasMessageContaining("lineage is fixed at insert") + + assertThat(store.findRun(run.key())).isEqualTo(run) + } + + @Test + fun `re-saving the same running run is idempotent`() { + val run = started("run-resave") + store.save(run) + store.save(run) + + assertThat(store.runsInContext(CONTEXT, 10, null)).containsExactly(run) + } + + @Test + fun `a running run carrying a finish time is rejected`() { + // ExtractionRun leaves status-and-timing pairing to the state machine, which is here. A + // record that reads as running and as finished at once makes every page and audit meeting + // it guess which. + val finishedWhileRunning = ExtractionRun( + contextId = CONTEXT, + lineage = ExtractionRunLineage.root(ExtractionRunRef("run-finished-running")), + status = ExtractionRunStatus.RUNNING, + startedAt = STARTED_AT, + finishedAt = FINISHED_AT, + ) + + assertThatIllegalArgumentException() + .isThrownBy { store.save(finishedWhileRunning) } + .withMessageContaining("carries no finishedAt") + + assertThat(store.findRun(finishedWhileRunning.key())).isNull() + } + + @Test + fun `a header save never deletes a recorded invocation`() { + val run = started("run-child-rows") + store.save(run) + // A caller loads the run, an attempt is recorded, and the caller then saves its own copy + // with updated counts. The attempt it never saw must survive — save owns header fields + // only and never reaches into invocation rows at all. + store.recordInvocation(run.key(), ExtractionInvocationRecord.planned(0)) + val staleWithCounts = ExtractionRunFixtures.runningRun( + lineage = run.lineage, + counts = ExtractionRunCounts(propositionsExtracted = 5), + ) + + val saved = store.save(staleWithCounts) + + assertThat(saved.counts.propositionsExtracted).isEqualTo(5) + assertThat(store.invocationsOf(run.key()).map { it.id }) + .containsExactly(ExtractionInvocationId(0, 1)) + } + + @Test + fun `a header save embedding a record for an identity already stored never touches it`() { + // recordInvocation is the only door onto invocation rows. A save's own invocations field is + // not written anywhere, whether it names an id already stored, a brand-new one, or both — + // the row recordInvocation wrote stays exactly as recordInvocation left it. + val run = started("run-child-save-ignored") + store.save(run) + store.recordInvocation( + run.key(), + ExtractionInvocationRecord( + id = ExtractionInvocationId.planned(0), + configuredService = "service-alpha", + ), + ) + + store.save( + ExtractionRunFixtures.runningRun( + lineage = run.lineage, + invocations = listOf( + ExtractionInvocationRecord( + id = ExtractionInvocationId.planned(0), + outcome = ExtractionInvocationOutcome.SUCCEEDED, + configuredService = "service-beta", + ), + ExtractionInvocationRecord.planned(1), + ), + ), + ) + + val stored = store.invocationsOf(run.key()) + assertThat(stored.map { it.id }).containsExactly(ExtractionInvocationId(0, 1)) + assertThat(stored.first().configuredService).isEqualTo("service-alpha") + } + + // ---- what the transition derives, and what it carries across ---- + + @Test + fun `a transition carries every component it does not own across unchanged`() { + val populated = ExtractionRunFixtures.populatedRun() + // populatedRun() is FAILED; rebuild it as the running run it was a moment before. + val running = ExtractionRun( + contextId = populated.contextId, + lineage = populated.lineage, + status = ExtractionRunStatus.RUNNING, + startedAt = populated.startedAt, + profile = populated.profile, + sourceRevisions = populated.sourceRevisions, + fingerprints = populated.fingerprints, + runtime = populated.runtime, + requestedModel = populated.requestedModel, + subjectRefs = populated.subjectRefs, + experimentRef = populated.experimentRef, + cohortRef = populated.cohortRef, + replayFidelity = populated.replayFidelity, + counts = populated.counts, + invocations = populated.invocations, + failures = populated.failures, + version = 7, + ) + + val terminal = ExtractionRunTransition.cancelled(FINISHED_AT).applyTo(running) + + assertThat(terminal.status).isEqualTo(ExtractionRunStatus.CANCELLED) + assertThat(terminal.finishedAt).isEqualTo(FINISHED_AT) + // Everything else is the run it was applied to, component by component. + assertThat(terminal.contextId).isEqualTo(running.contextId) + assertThat(terminal.lineage).isEqualTo(running.lineage) + assertThat(terminal.startedAt).isEqualTo(running.startedAt) + assertThat(terminal.profile).isEqualTo(running.profile) + assertThat(terminal.sourceRevisions).isEqualTo(running.sourceRevisions) + assertThat(terminal.fingerprints).isEqualTo(running.fingerprints) + assertThat(terminal.runtime).isEqualTo(running.runtime) + assertThat(terminal.requestedModel).isEqualTo(running.requestedModel) + assertThat(terminal.subjectRefs).isEqualTo(running.subjectRefs) + assertThat(terminal.experimentRef).isEqualTo(running.experimentRef) + assertThat(terminal.cohortRef).isEqualTo(running.cohortRef) + assertThat(terminal.replayFidelity).isEqualTo(running.replayFidelity) + assertThat(terminal.counts).isEqualTo(running.counts) + assertThat(terminal.invocations).isEqualTo(running.invocations) + assertThat(terminal.failures).isEqualTo(running.failures) + assertThat(terminal.version).isEqualTo(running.version) + } + + @Test + fun `null keeps what the run recorded and a value replaces it`() { + val accumulated = listOf( + ExtractionFailure(ExtractionFailureCode.RATE_LIMITED, ExtractionFailureStage.MODEL_CALL), + ) + val running = ExtractionRun( + contextId = CONTEXT, + lineage = ExtractionRunLineage.root(ExtractionRunRef("run-keep")), + status = ExtractionRunStatus.RUNNING, + startedAt = STARTED_AT, + counts = ExtractionRunCounts(propositionsExtracted = 3), + failures = accumulated, + ) + + val kept = ExtractionRunTransition.completed(FINISHED_AT).applyTo(running) + assertThat(kept.counts.propositionsExtracted).isEqualTo(3) + assertThat(kept.failures).isEqualTo(accumulated) + + val replaced = ExtractionRunTransition.completed( + finishedAt = FINISHED_AT, + counts = ExtractionRunCounts(propositionsExtracted = 7), + failures = emptyList(), + ).applyTo(running) + assertThat(replaced.counts.propositionsExtracted).isEqualTo(7) + assertThat(replaced.failures).isEmpty() + } + + @Test + fun `a transition refuses a run that has already ended and a finish before its start`() { + val running = ExtractionRunFixtures.startedRun() + + assertThatIllegalArgumentException() + .isThrownBy { + ExtractionRunTransition.completed(STARTED_AT.minusSeconds(1)).applyTo(running) + } + .withMessageContaining("must not be before") + + val terminal = ExtractionRunTransition.failed(FINISHED_AT).applyTo(running) + assertThatIllegalArgumentException() + .isThrownBy { ExtractionRunTransition.cancelled(FINISHED_AT).applyTo(terminal) } + .withMessageContaining("has already ended") + } + + @Test + fun `a transition is bounded by the same failure cap as the run`() { + val over = (0..ExtractionRunLimits.MAX_FAILURES) + .map { ExtractionFailure(ExtractionFailureCode.INTERNAL) } + + assertThatIllegalArgumentException() + .isThrownBy { ExtractionRunTransition.failed(FINISHED_AT, failures = over) } + .withMessageContaining("at most ${ExtractionRunLimits.MAX_FAILURES} failures") + } + + @Test + fun `a failed run need not say why, and a completed run may still carry failures`() { + val silent = started("run-silent") + store.save(silent) + val silentResult = store.transition(silent.key(), ExtractionRunTransition.failed(FINISHED_AT)) + assertThat(silentResult.run.failures).isEmpty() + + val recovered = started("run-recovered") + store.save(recovered) + val recoveredResult = store.transition( + recovered.key(), + ExtractionRunTransition.completed( + finishedAt = FINISHED_AT, + failures = listOf( + ExtractionFailure(ExtractionFailureCode.RATE_LIMITED, ExtractionFailureStage.MODEL_CALL), + ), + ), + ) + assertThat(recoveredResult.run.status).isEqualTo(ExtractionRunStatus.COMPLETED) + assertThat(recoveredResult.run.failures).hasSize(1) + } + + // ---- tenants ---- + + @Test + fun `two tenants running the same run id transition independently`() { + val mine = ExtractionRunFixtures.runningRun("run-shared", CONTEXT) + val theirs = ExtractionRunFixtures.runningRun("run-shared", OTHER_CONTEXT) + store.save(mine) + store.save(theirs) + + store.transition(mine.key(), ExtractionRunTransition.completed(FINISHED_AT)) + + assertThat(store.findRun(mine.key())?.status).isEqualTo(ExtractionRunStatus.COMPLETED) + assertThat(store.findRun(theirs.key())?.status).isEqualTo(ExtractionRunStatus.RUNNING) + + // The neighbour's run is still running, so its own terminal write applies rather than + // colliding with the one already recorded under the same run id. + assertThat(store.transition(theirs.key(), ExtractionRunTransition.failed(FINISHED_AT)).outcome) + .isEqualTo(ExtractionRunTransitionOutcome.APPLIED) + } +} diff --git a/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunStoreReadsTest.kt b/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunStoreReadsTest.kt new file mode 100644 index 00000000..74c7cce0 --- /dev/null +++ b/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunStoreReadsTest.kt @@ -0,0 +1,372 @@ +/* + * Copyright 2024-2026 Embabel Pty Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.embabel.dice.proposition.extraction + +import com.embabel.dice.proposition.extraction.ExtractionRunFixtures.CONTEXT +import com.embabel.dice.proposition.extraction.ExtractionRunFixtures.OTHER_CONTEXT +import com.embabel.dice.proposition.extraction.ExtractionRunFixtures.STARTED_AT +import org.assertj.core.api.Assertions.assertThat +import org.assertj.core.api.Assertions.assertThatIllegalArgumentException +import org.assertj.core.api.Assertions.assertThatThrownBy +import org.junit.jupiter.api.Test +import java.time.Instant + +/** + * The read side: bounded tenant-scoped pages, the two lineage lookups and the chain walk. + * + * The scope-before-limit cases are the ones worth reading twice. A page that limited first and + * scoped afterwards passes every single-tenant test and returns an empty page in production the + * first time a busy neighbour occupies the head of the index. + */ +class ExtractionRunStoreReadsTest { + + private val store = InMemoryExtractionRunStore() + + private fun run(runId: String, contextId: com.embabel.agent.core.ContextId, atSeconds: Long) = + ExtractionRunFixtures.runningRun(runId, contextId, STARTED_AT.plusSeconds(atSeconds)) + + // ---- scope before limit ---- + + @Test + fun `a page scopes to the tenant before it limits`() { + // The neighbour holds the six newest runs in the store. A page that took the newest six + // and then filtered by tenant would return nothing for CONTEXT, which reads exactly like a + // tenant with no runs. + (1..6).forEach { store.save(run("neighbour-$it", OTHER_CONTEXT, 100L + it)) } + (1..4).forEach { store.save(run("mine-$it", CONTEXT, it.toLong())) } + + val page = store.runsInContext(CONTEXT, limit = 3, since = null) + + assertThat(page).hasSize(3) + assertThat(page.map { it.ref.runId }).containsExactly("mine-4", "mine-3", "mine-2") + assertThat(page).allSatisfy { assertThat(it.contextId).isEqualTo(CONTEXT) } + } + + @Test + fun `the lineage reads scope before they limit too`() { + val root = ExtractionRunRef("root-1") + (1..6).forEach { + store.save( + ExtractionRunFixtures.runningRun( + contextId = OTHER_CONTEXT, + lineage = ExtractionRunLineage.childOf( + runRef = ExtractionRunRef("neighbour-child-$it"), + parent = ExtractionRunLineage.root(root), + ), + startedAt = STARTED_AT.plusSeconds(100L + it), + ), + ) + } + (1..3).forEach { + store.save( + ExtractionRunFixtures.runningRun( + contextId = CONTEXT, + lineage = ExtractionRunLineage.childOf( + runRef = ExtractionRunRef("my-child-$it"), + parent = ExtractionRunLineage.root(root), + ), + startedAt = STARTED_AT.plusSeconds(it.toLong()), + ), + ) + } + + assertThat(store.childrenOf(CONTEXT, root, limit = 2).map { it.ref.runId }) + .containsExactly("my-child-3", "my-child-2") + assertThat(store.runsOfRoot(CONTEXT, root, limit = 2, since = null).map { it.ref.runId }) + .containsExactly("my-child-3", "my-child-2") + } + + // ---- bounds and windows ---- + + @Test + fun `every page takes a positive limit`() { + val root = ExtractionRunRef("root-limit") + listOf(0, -1).forEach { limit -> + assertThatIllegalArgumentException() + .describedAs("limit %s", limit) + .isThrownBy { store.runsInContext(CONTEXT, limit, null) } + assertThatIllegalArgumentException() + .isThrownBy { store.childrenOf(CONTEXT, root, limit) } + assertThatIllegalArgumentException() + .isThrownBy { store.runsOfRoot(CONTEXT, root, limit, null) } + assertThatIllegalArgumentException() + .isThrownBy { store.ancestorsOf(ExtractionRunFixtures.keyOf("run-1"), limit) } + } + } + + @Test + fun `since bounds the window at or after the instant given`() { + (0..4).forEach { store.save(run("run-$it", CONTEXT, it * 10L)) } + val cutoff = STARTED_AT.plusSeconds(20) + + val windowed = store.runsInContext(CONTEXT, limit = 10, since = cutoff) + + assertThat(windowed.map { it.ref.runId }).containsExactly("run-4", "run-3", "run-2") + assertThat(store.runsInContext(CONTEXT, limit = 10, since = null)).hasSize(5) + } + + @Test + fun `a page is repeatable when two runs share a start instant`() { + listOf("run-c", "run-a", "run-b").forEach { store.save(run(it, CONTEXT, 0L)) } + + // Ordering by start alone would leave these three in whatever order the backend liked, and + // a caller paging through would see one twice or none. + repeat(3) { + assertThat(store.runsInContext(CONTEXT, limit = 2, since = null).map { r -> r.ref.runId }) + .containsExactly("run-a", "run-b") + } + } + + // ---- lineage ---- + + @Test + fun `children are one hop down the parent axis and supersession is not walked`() { + val parent = ExtractionRunLineage.root(ExtractionRunRef("run-parent")) + val child = ExtractionRunLineage.childOf(ExtractionRunRef("run-child"), parent) + val grandchild = ExtractionRunLineage.childOf(ExtractionRunRef("run-grandchild"), child) + val replacement = ExtractionRunLineage.root( + runRef = ExtractionRunRef("run-replacement"), + supersedesRunRef = parent.runRef, + ) + listOf(parent, child, grandchild, replacement).forEach { + store.save(ExtractionRunFixtures.runningRun(lineage = it)) + } + + val children = store.childrenOf(CONTEXT, parent.runRef, limit = 10) + + assertThat(children.map { it.ref.runId }).containsExactly("run-child") + } + + @Test + fun `the whole lineage comes back from the root reference in one read`() { + val root = ExtractionRunLineage.root(ExtractionRunRef("run-root")) + val pass1 = ExtractionRunLineage.childOf(ExtractionRunRef("run-pass-1"), root) + val pass2 = ExtractionRunLineage.childOf(ExtractionRunRef("run-pass-2"), pass1) + val pass3 = ExtractionRunLineage.childOf(ExtractionRunRef("run-pass-3"), pass2) + val unrelated = ExtractionRunLineage.root(ExtractionRunRef("run-unrelated")) + listOf(root, pass1, pass2, pass3, unrelated).forEachIndexed { index, lineage -> + store.save( + ExtractionRunFixtures.runningRun( + lineage = lineage, + startedAt = STARTED_AT.plusSeconds(index.toLong()), + ), + ) + } + + val lineage = store.runsOfRoot(CONTEXT, root.runRef, limit = 10, since = null) + + // Four hops deep, one read, and the root itself is in it. + assertThat(lineage.map { it.ref.runId }) + .containsExactly("run-pass-3", "run-pass-2", "run-pass-1", "run-root") + } + + @Test + fun `the chain walk climbs to the root, excludes the run itself, and stops at the limit`() { + val root = ExtractionRunLineage.root(ExtractionRunRef("run-root")) + val one = ExtractionRunLineage.childOf(ExtractionRunRef("run-1"), root) + val two = ExtractionRunLineage.childOf(ExtractionRunRef("run-2"), one) + val three = ExtractionRunLineage.childOf(ExtractionRunRef("run-3"), two) + listOf(root, one, two, three).forEach { + store.save(ExtractionRunFixtures.runningRun(lineage = it)) + } + + val all = store.ancestorsOf(ExtractionRunFixtures.keyOf("run-3"), limit = 10) + assertThat(all.map { it.ref.runId }).containsExactly("run-2", "run-1", "run-root") + + val bounded = store.ancestorsOf(ExtractionRunFixtures.keyOf("run-3"), limit = 2) + assertThat(bounded.map { it.ref.runId }).containsExactly("run-2", "run-1") + + assertThat(store.ancestorsOf(ExtractionRunFixtures.keyOf("run-root"), limit = 10)).isEmpty() + assertThat(store.ancestorsOf(ExtractionRunFixtures.keyOf("run-absent"), limit = 10)).isEmpty() + } + + @Test + fun `the chain walk stops at a parent the store does not hold`() { + val missing = ExtractionRunLineage.root(ExtractionRunRef("run-missing")) + val orphan = ExtractionRunLineage.childOf(ExtractionRunRef("run-orphan"), missing) + store.save(ExtractionRunFixtures.runningRun(lineage = orphan)) + + assertThat(store.ancestorsOf(ExtractionRunFixtures.keyOf("run-orphan"), limit = 10)).isEmpty() + } + + @Test + fun `the chain walk terminates on a cycle`() { + // A value type rejects a run that is its own parent; a two-hop cycle needs the other run to + // see, so only the store can catch it. A corrupt store is a real possibility. A store that + // hangs on one is worse than a corrupt store. + // root() and childOf() cannot mint a cycle. fromStoredFields can, which is the point of + // this test: a cycle is something a store reads back, never something an application mints. + val root = ExtractionRunRef("run-cycle-root") + val a = ExtractionRunLineage.fromStoredFields( + runRef = ExtractionRunRef("run-a"), + rootRunRef = root, + parentRunRef = ExtractionRunRef("run-b"), + ) + val b = ExtractionRunLineage.fromStoredFields( + runRef = ExtractionRunRef("run-b"), + rootRunRef = root, + parentRunRef = ExtractionRunRef("run-a"), + ) + store.save(ExtractionRunFixtures.runningRun(lineage = a)) + store.save(ExtractionRunFixtures.runningRun(lineage = b)) + + val walked = store.ancestorsOf(ExtractionRunFixtures.keyOf("run-a"), limit = 1_000) + + assertThat(walked.map { it.ref.runId }).containsExactly("run-b") + } + + // ---- tenants ---- + + @Test + fun `every read fails closed across tenants`() { + val root = ExtractionRunLineage.root(ExtractionRunRef("run-root")) + val child = ExtractionRunLineage.childOf(ExtractionRunRef("run-child"), root) + listOf(root, child).forEach { + store.save(ExtractionRunFixtures.runningRun(contextId = OTHER_CONTEXT, lineage = it)) + } + + assertThat(store.findRun(ExtractionRunFixtures.keyOf("run-child", CONTEXT))).isNull() + assertThat(store.findRun(ExtractionRunFixtures.keyOf("run-child", OTHER_CONTEXT))).isNotNull() + assertThat(store.invocationsOf(ExtractionRunFixtures.keyOf("run-child", CONTEXT))).isEmpty() + assertThat(store.runsInContext(CONTEXT, 10, null)).isEmpty() + assertThat(store.childrenOf(CONTEXT, root.runRef, 10)).isEmpty() + assertThat(store.runsOfRoot(CONTEXT, root.runRef, 10, null)).isEmpty() + assertThat(store.ancestorsOf(ExtractionRunFixtures.keyOf("run-child", CONTEXT), 10)).isEmpty() + } + + @Test + fun `a chain walk stops rather than crossing into another tenant`() { + val parent = ExtractionRunLineage.root(ExtractionRunRef("run-parent")) + val child = ExtractionRunLineage.childOf(ExtractionRunRef("run-child"), parent) + // The parent exists, in the neighbour's tenant. The child is mine. + store.save(ExtractionRunFixtures.runningRun(contextId = OTHER_CONTEXT, lineage = parent)) + store.save(ExtractionRunFixtures.runningRun(contextId = CONTEXT, lineage = child)) + + assertThat(store.ancestorsOf(ExtractionRunFixtures.keyOf("run-child", CONTEXT), 10)).isEmpty() + assertThat(store.ancestorsOf(ExtractionRunFixtures.keyOf("run-child", OTHER_CONTEXT), 10)).isEmpty() + } + + @Test + fun `two tenants may hold the same run id without colliding`() { + val mine = ExtractionRunFixtures.runningRun("run-shared", CONTEXT, STARTED_AT) + val theirs = ExtractionRunFixtures.runningRun("run-shared", OTHER_CONTEXT, STARTED_AT.plusSeconds(60)) + store.save(mine) + store.save(theirs) + + assertThat(store.findRun(mine.key())?.startedAt).isEqualTo(STARTED_AT) + assertThat(store.findRun(theirs.key())?.startedAt).isEqualTo(STARTED_AT.plusSeconds(60)) + assertThat(store.runsInContext(CONTEXT, 10, null)).containsExactly(mine) + assertThat(store.runsInContext(OTHER_CONTEXT, 10, null)).containsExactly(theirs) + } + + // ---- invocation records ---- + + @Test + fun `a run records zero, one or many invocations`() { + val zero = ExtractionRunFixtures.runningRun("run-zero") + store.save(zero) + assertThat(store.invocationsOf(zero.key())).isEmpty() + + val one = ExtractionRunFixtures.runningRun("run-one") + store.save(one) + store.recordInvocation(one.key(), ExtractionInvocationRecord.planned(0)) + assertThat(store.invocationsOf(one.key())).hasSize(1) + + val many = ExtractionRunFixtures.runningRun("run-many") + store.save(many) + ExtractionInvocationRecord.plan(4).forEach { store.recordInvocation(many.key(), it) } + assertThat(store.invocationsOf(many.key()).map { it.invocationIndex }) + .containsExactly(0, 1, 2, 3) + } + + @Test + fun `records come back in plan order however they arrived`() { + val run = ExtractionRunFixtures.runningRun("run-order") + store.save(run) + // Completion order, which is not plan order. + listOf(2, 0, 3, 1).forEach { + store.recordInvocation( + run.key(), + ExtractionInvocationRecord( + id = ExtractionInvocationId.planned(it), + outcome = ExtractionInvocationOutcome.SUCCEEDED, + ), + ) + } + + assertThat(store.invocationsOf(run.key()).map { it.invocationIndex }) + .containsExactly(0, 1, 2, 3) + } + + @Test + fun `an in-flight record updates in place, and a terminal one accepts only an identical replay`() { + val run = ExtractionRunFixtures.runningRun("run-retry") + store.save(run) + val id = ExtractionInvocationId.planned(0) + + // While the attempt is in flight, dispatch details fill in as they're known — same + // identity, more known about it, one record updated. + store.recordInvocation(run.key(), ExtractionInvocationRecord(id = id, configuredService = "service-alpha")) + store.recordInvocation(run.key(), ExtractionInvocationRecord(id = id, configuredService = "service-beta")) + assertThat(store.invocationsOf(run.key())).hasSize(1) + assertThat(store.invocationsOf(run.key()).single().configuredService).isEqualTo("service-beta") + + val terminal = ExtractionInvocationRecord( + id = id, + outcome = ExtractionInvocationOutcome.FAILED, + configuredService = "service-beta", + ) + store.recordInvocation(run.key(), terminal) + + store.recordInvocation(run.key(), terminal) + assertThat(store.invocationsOf(run.key())).containsExactly(terminal) + + assertThatThrownBy { + store.recordInvocation(run.key(), terminal.copy(configuredService = "service-gamma")) + }.isInstanceOf(ExtractionRunConflictException::class.java) + assertThat(store.invocationsOf(run.key())).containsExactly(terminal) + + // The next attempt at the same call: a second record, not a replacement. + store.recordInvocation(run.key(), ExtractionInvocationRecord(id = id.nextAttempt())) + assertThat(store.invocationsOf(run.key()).map { it.id }) + .containsExactly(ExtractionInvocationId(0, 1), ExtractionInvocationId(0, 2)) + } + + @Test + fun `recording against a run nobody started is rejected`() { + assertThatThrownBy { + store.recordInvocation( + ExtractionRunFixtures.keyOf("run-absent"), + ExtractionInvocationRecord.planned(0), + ) + }.isInstanceOf(ExtractionRunNotFoundException::class.java) + } + + // ---- Java reachability ---- + + @Test + fun `the tenant-scoped reads are reachable without a value-class argument`() { + val run = ExtractionRunFixtures.runningRun("run-java") + store.save(run) + + // ContextId is a Kotlin value class, so a method taking one has a mangled JVM name. The + // String form is the override point and is what a Java caller reaches. + assertThat(store.runsInContext(CONTEXT.value, 10, null)).containsExactly(run) + assertThat(store.findRun(ExtractionRunKey.of(CONTEXT.value, "run-java"))).isEqualTo(run) + assertThat(store.childrenOf(CONTEXT.value, "run-java", 10)).isEmpty() + assertThat(store.runsOfRoot(CONTEXT.value, "run-java", 10, null)).containsExactly(run) + } +} diff --git a/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunValueTypesTest.kt b/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunValueTypesTest.kt index 5ee7c838..27e88a99 100644 --- a/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunValueTypesTest.kt +++ b/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunValueTypesTest.kt @@ -284,6 +284,14 @@ class ExtractionRunValueTypesTest { ExtractionDeploymentRef::class.java, ExtractionExperimentRef::class.java, ExtractionCohortRef::class.java, + ExtractionRunStore::class.java, + InMemoryExtractionRunStore::class.java, + ExtractionRunTransition::class.java, + ExtractionRunTransitionOutcome::class.java, + ExtractionRunTransitionResult::class.java, + ExtractionRunFingerprint::class.java, + ExtractionRunNotFoundException::class.java, + ExtractionRunConflictException::class.java, ).forEach { type -> assertThat(isMarkedExperimental(type)) .describedAs("%s is marked experimental", type.simpleName) diff --git a/docs/design/extraction-runs.md b/docs/design/extraction-runs.md index f2defb3b..e03d1747 100644 --- a/docs/design/extraction-runs.md +++ b/docs/design/extraction-runs.md @@ -12,10 +12,10 @@ what ran, with what, and what came out. DICE does not model the host's episode; it can know about and leaves `profile` and the run lineage as the join points a host uses to fold runs into its own audit. -This note covers DICE #67's value model — the types in `com.embabel.dice.proposition.extraction` -that later slices store, key, and expose. The lifecycle state machine, the store contract, the -Drivine implementation, the proposition-to-run relation and the wiring are separate slices; where -this note says "the store contract", that is what it means. +This note covers DICE #67's value model and its store contract — the types in +`com.embabel.dice.proposition.extraction`, the lifecycle state machine that governs a run's status, +and the reads a store owes. The Drivine implementation, the proposition-to-run relation and the +wiring are separate slices. ## What a run holds @@ -289,18 +289,323 @@ batching and floating-point nondeterminism. The field says what the *record* sup recorded, identities and fingerprints only, or those plus the requested configuration — and makes no promise about the provider. Host replay policy stays the host's. -## Four lifecycle states +## The lifecycle state machine -`ExtractionRunStatus` is `RUNNING`, `COMPLETED`, `FAILED`, `CANCELLED`. The values only: the -transitions, the compare-and-set rules and idempotent terminal rewrites belong to the store -contract, so this type does not half-encode them. It checks that a finish does not precede a start -and stops there — a terminal status with no finish time is constructible here and is the state -machine's to reject. +`ExtractionRunStatus` is `RUNNING`, `COMPLETED`, `FAILED`, `CANCELLED`. A run starts running and +ends in one of the other three. There are no other edges: a terminal run never re-opens, and it +never moves from one terminal state to another. -`COMPLETED`'s meaning is pinned where the value is declared, because it is not obvious and it is -load-bearing: every product the run's request called for is either durably persisted or terminally -disposed. It is written after persistence, never before, so a run whose persistence never finished -stays `RUNNING` and retryable. A run with zero products terminalizes `COMPLETED` vacuously. +```mermaid +stateDiagram-v2 + [*] --> RUNNING: save() + RUNNING --> RUNNING: save() — header fields + RUNNING --> RUNNING: recordInvocation() — invocation rows + RUNNING --> COMPLETED: transition(completed) — after persistence + RUNNING --> FAILED: transition(failed) + RUNNING --> CANCELLED: transition(cancelled) + COMPLETED --> COMPLETED: replay, same fingerprint + FAILED --> FAILED: replay, same fingerprint + CANCELLED --> CANCELLED: replay, same fingerprint + COMPLETED --> [*] + FAILED --> [*] + CANCELLED --> [*] +``` + +The two write methods split along that line, and the split is what makes the `COMPLETED` rule +enforceable rather than advisory. `ExtractionRunStore.save` rejects any status other than `RUNNING`, +so a terminal status cannot enter through the door that also accepts new keys. +`ExtractionRunStore.transition` is the only writer of a terminal status, and it is compare-and-set: +it moves a run out of `RUNNING` or it does nothing. + +`ExtractionRun` itself still checks only that a finish does not precede a start. The value type does +not half-encode the machine; a terminal status with no finish time is constructible there and is +rejected here, where the rule is defined once. + +### What `COMPLETED` asserts, and who may write it + +Every product the run's request called for is either durably persisted or terminally disposed. + +The store cannot check that — it holds run headers, not products. So it does the next best thing and +makes the claim reachable through one narrow call whose precondition is written down: on the legacy +path the coordinator calls it once `persistAndProject` has returned, and on the #68 commit path the +commit transaction calls it, and only the commit whose cumulative outcomes bring every requested +product to persisted or terminally disposed. + +Three consequences follow, and each is a test: + +- a run whose persistence never finished stays `RUNNING` and is retryable under compare-and-set; +- a commit that persists some products and leaves others outstanding leaves the run `RUNNING`, so a + terminal run never has re-committable products behind it; +- a run with zero products completes vacuously — there was nothing to persist, so the coverage claim + holds. + +`FAILED` and `CANCELLED` carry no such precondition. A run that could not finish, or that was +stopped, terminalizes whether or not anything was persisted. `CANCELLED` is also the abandonment +path for a partially successful run nobody intends to finish: its outstanding products stay +outstanding behind it, and recovery goes through a new run linked by parent or superseded reference. + +A `FAILED` transition need not carry a failure. A run can stop on something the coordinator +classifies at the run level with no per-attempt detail, and requiring the pairing would make an +honest "we know it failed and not why" unrecordable. A `COMPLETED` transition may carry failures for +the same reason from the other side: a run that retried past a failed attempt and finished still +happened. + +### Deriving the terminal run + +`ExtractionRun` publishes no `withStatus`, no `finished()` and no `copy`. +`ExtractionRunTransition.applyTo` is the only place a terminal run is derived. Other code re-lists +the run's eighteen constructor arguments — `InMemoryExtractionRunStore` does it to add a child +record or move the header version — but nothing else produces a run in a terminal state. + +A transition carries exactly the fields the lifecycle owns: the terminal status, the finish instant, +and optionally the final counts and failures. Everything else is carried across unchanged, including +the header version, and a test asserts that component by component. The alternative — public +mutators on the run — would put the state machine in two places and let anything in the codebase +manufacture a `COMPLETED` run without going near a store. `ExtractionRunLifecycleTest` pins +`applyTo` directly, in `dice`; the cross-backend suite pins the same promise through a store's +`transition()`, which is what a backend that maps rows in and out of its own storage actually has to +get right — `a terminal write through the store preserves every field it does not own, header +version included` and `null counts and failures on a transition keep what the stored run held, and +values replace them`. + +`counts` and `failures` are nullable and follow one rule: null keeps what the run recorded, a value +replaces it. An empty failure list is a value. That distinction reaches the fingerprint, so +"leave the counts alone" and "these counts are final" are two different terminal writes even when +they land on the same numbers. + +Invocation records do not travel on a transition. They arrive through `recordInvocation` while the +run is still running, keyed by `(invocationIndex, attempt)`, and a terminal run takes no more — +a finished run's invocation list is part of how it finished. + +### `recordInvocation` is the only door onto invocation state + +`save` writes header fields only. It never creates, updates or deletes an invocation row, whatever +`ExtractionRun.invocations` holds on the run it is handed — that field is not written anywhere by +`save`, and it plays no part in what `save` accepts, rejects, or replays as a no-op. Every +invocation write goes through `recordInvocation`, keyed by the record's own `(invocationIndex, +attempt)`, insert-or-compare on that key alone. + +This was not the original design. An earlier version had `save` merge the invocation records it was +handed into the ones already stored, by identity, so a header update built on a run read before an +attempt was recorded would not silently drop that attempt. See +["Why the store uses compare-and-set" below](#why-the-store-uses-compare-and-set) for why that +shared-generation merge produced exactly the defect it was meant to prevent, and why the fix gives +invocation rows their own door and their own key, with compare-and-set scoped to that key alone. + +### Invocation records are locked once terminal + +A record for an id already stored, and still `IN_FLIGHT`, updates in place — that is how dispatch +details and, eventually, the terminal outcome fill in as an attempt runs. Once the stored record is +terminal it is locked: an incoming record for that id is accepted only when it equals the stored +one exactly (an identical retry replays as a no-op), and every other write for that id is rejected +with `ExtractionRunConflictException`. A different outcome is the +case the review comment named — a delayed `IN_FLIGHT` message arriving after the attempt already +succeeded or failed, from a dispatcher's own retry timer firing late or two writers racing on the +same attempt, which would otherwise put a finished attempt back to outstanding and erase the record +of how it ended. The same-outcome case is narrower and easy to miss: a delayed write that repeats +the correct terminal outcome but carries different timing, usage or provider facts than the write +that actually landed first — accepting it as an in-place update would erase those facts just as +surely, under an outcome that never changed. Locking the whole record closes both cases; a lock +scoped to the outcome field alone would still miss the second. This is the same category of harm +`transition` refuses at the run level: a late or duplicated writer silently rewriting how something +ended. Because `save` never reaches this state, a stale header snapshot cannot be the write that +puts a terminal record back to outstanding or erases its facts — only another `recordInvocation` +call can, and the lock decides it the same way regardless of how old the caller's own copy of the +run is. + +A record accepted as an identical replay lands back at the position the stored record already held. `ExtractionRun.equals` compares `invocations` by position, so an +identical resend that moved to the end would read as a change even though nothing about the run +actually differs, which would either bump a header save's version on a no-op or turn a stale but +otherwise identical resend into a rejection the store promises everywhere else not to raise. Only a +genuinely new id is appended. + +In the cross-backend suite: `a delayed IN_FLIGHT write does not replace a terminal record for the +same attempt`, `a same-outcome write that differs from a terminal record is rejected too`, +`repeating the same terminal invocation write is idempotent`, and `a terminal record's identical +replay through recordInvocation lands back at its stored position` pin the lock and the +position-preserving replace. `an in-flight record updates in place, and a terminal one accepts only +an identical replay`, in `dice`'s own `ExtractionRunStoreReadsTest`, pins the +retry-lands-on-its-own-record half alongside the lock. The in-place half of the rule — an +identical replay is one shape of accepted write, and a genuinely changed `IN_FLIGHT` write is +another, and a reader sees its new content afterward — is pinned by `a changed IN_FLIGHT record +updates in place through recordInvocation, clearing an omitted fact`, which names a fact on the +first write that the second write omits and reads the result back through `invocationsOf`, a read +against the store's own storage, independent of the object the write call happens to return. A +field-merging backend keeps the omitted fact; only whole-record replacement clears it, and only +that independent read can tell the two apart. + +`a header save embedding a brand-new invocation never creates the row`, `a header save embedding a +changed invocation never updates the stored row` and `a header save embedding an empty invocation +list never deletes a stored row` pin the three shapes of "`save` does not touch this state" — a save +cannot originate a row, cannot update one, and cannot remove one, however its own `invocations` +field is populated. `a stale header save carrying an old invocation snapshot leaves the newer +stored invocation intact` is the case the finding named directly: a caller's header save, built on +a run read before a later `recordInvocation` call landed, still names the version currently stored +— `recordInvocation` never moves it — and is accepted as a genuine header change, carrying a stale +invocation snapshot along for a ride the store no longer takes. `two concurrent IN_FLIGHT writers +on different attempts both land, losing neither` is the concurrent form of the same guarantee: two +attempts contend for nothing, because each is decided on its own key and neither touches a shared +header generation. + +### Idempotency: insert-or-compare, never overwrite + +Every terminal write carries a fingerprint of its payload. A store records the fingerprint of the +write that terminalized a run, and a second write against that run is decided by comparison: + +| Second write | Result | +| --- | --- | +| same fingerprint | replays as success, `REPLAYED`, changes nothing | +| different fingerprint | `ExtractionRunConflictException` | +| any `save` | `ExtractionRunConflictException` — a terminal run is not re-openable | + +DICE's existing `MERGE … SET` stores upsert by overwriting. That is safe for a record still being +written and wrong for one that is finished: it would let a late or duplicated writer silently +rewrite how a run ended, and the audit would carry the last write rather than the true one. No +method on this contract overwrites a terminal run. + +The fingerprint covers the terminal write and not the run. A coordinator that recorded another +attempt between a terminal write it never saw the answer to and its retry made the same terminal +write both times, and folding the run's invocation list in would turn that correct retry into a +rejected conflict. + +`save` is the one method that updates in place, and only on a run that is still running. Even there +it is fenced four ways: + +- it rejects any status but `RUNNING`, so a terminal status cannot enter through it; +- it rejects a `RUNNING` run carrying a `finishedAt`. `ExtractionRun` deliberately leaves + status-and-timing pairing to this state machine, and a record that reads as running and as + finished at once makes every page and audit meeting it guess which; +- it rejects a save disagreeing with the stored lineage or start time — a different run wearing the + same id. The tenant cannot disagree, since it is half of the key; +- it compares `ExtractionRun.version` before it accepts anything else about the header. A save + names the version it was read at; the store accepts the write only when that matches the version + currently stored, and rejects it with `ExtractionRunConflictException` otherwise, naming both + versions so the caller knows to read the run again and rebuild its update. A save whose header + content is identical to what is already stored replays as a no-op regardless of the version it + names — `a save whose content already matches what is stored replays as a no-op at a stale + version` pins that — the same idempotent-retry courtesy `transition` gives a terminal write. The + version names the CAS generation the header is currently at: a run that has never been saved, and + the run its first accepted save produces, both carry `0`, because + that first save inserts the row and there is no earlier generation for it to raise past, and a + first save naming any other value is rejected before it reaches the store — `a first save must + name version 0` pins the rejection and confirms nothing was written. Each later save that actually + changes the header raises the generation by one; a no-op replay is accepted too, and leaves the + generation exactly where it stood — `a save whose content already matches what is stored replays + as a no-op at a stale version` pins that half specifically. `recordInvocation` never moves it, so a + save built on the header as it stood before an attempt was recorded still names the current + version and is accepted; the attempt survives because save does not touch invocation rows at all. + `recording an invocation does not move the header version` pins that in the cross-backend suite. + +`save` leaves invocation records alone entirely: it does not read or write invocation rows, so it +cannot delete one, put one back to `IN_FLIGHT`, create one, or update one. Records live under +`recordInvocation`'s own key, with their own lifecycle, entirely independent of the header's +version; see ["`recordInvocation` is the only door onto invocation +state"](#recordinvocation-is-the-only-door-onto-invocation-state) above for what that closes. + +#### Why the store uses compare-and-set + +The first version of this rule tried the other approach: keep the version field off `ExtractionRun` +entirely and merge two writers' headers field by field instead, letting each save keep whatever +part of the header the other did not touch. It looked smaller, and it was wrong in three +independent ways that a review round found: + +- **Counts cannot be merged by taking the larger reading.** Two writers can each account for + disjoint work — one processed five items, the other seven, and the run really got through twelve + — and there is no way for the store to tell that case apart from one writer's stale re-report of + work the other writer's higher count already covers. Taking the larger number is correct in the + second case and silently drops five items' worth of work in the first. Rejecting the stale write + and letting the caller re-read and add its own new count to what is now stored gets the right + total in both cases, because the caller — not the store — knows which one it is in. +- **A union of source revisions loses the order they were read in**, which is the field's own + documented meaning. Appending a slower writer's unseen revisions after the ones already stored can + put a source that was read first after one that was read later, if the first writer saved last. +- **A merge assembled from two different writers' saves is a header no writer ever held.** A + fingerprint from one save could sit beside a replay fidelity from another that fingerprint does + not support, misstating how much of the run could be set up again from what it recorded. + +Compare-and-set does not have any of these problems, because it never combines two writers' data — +either a save is building on the freshest header, and it replaces the whole thing, or it is stale +and is rejected outright. `a stale header save is rejected, and the header it read is left in +place` and `an accepted header save replaces the whole header at once` are in the cross-backend +suite. + +**A second version of this rule kept the version field but still had `save` merge invocation +records by identity, and a review round found that merge reintroduced the same defect one layer +down.** Versioning the header protects the header; it does nothing for a row `save` folds in from +the run it was handed, because `recordInvocation` never advances the header's version. A header +save built well before a `recordInvocation` call landed could still name the version currently +stored — nothing about that version had moved — and be accepted as a genuine header change, carrying +a stale invocation snapshot in on the same write. That snapshot could silently replace `IN_FLIGHT` +dispatch details, or a terminal outcome, the later `recordInvocation` call had already settled, with +no conflict raised on either side: the header's CAS check saw a real change and let it through, and +the merge's own terminal lock only compared the stale snapshot against what was stored at write +time, which was already the newer value. Two independent header writers, each merging an attempt +recorded before the other's read, could lose each other's facts the same way. The header's +generation cannot fence state that recording an attempt does not move it for — which is exactly +what the finding on PR #98 named. The fix removes the merge and gives invocation rows their own +key and their own compare-and-set, entirely off the header's generation. Run state is accumulative +the way lineage systems such as OpenLineage model a run: independent writers contribute rows, and +no write rewrites a row another writer owns. + +**Replay needs the identical payload, finish time included.** A coordinator retrying after a crash +it never saw the answer to must reuse the transition it built the first time, or read the run back +with `findRun` and stop if it has already ended. Minting a fresh `finishedAt` on the retry produces +a different fingerprint, which is an incompatible rewrite and is rejected. That is safe and it is +the opposite of what a caller expecting idempotency would predict, so it is a constraint the wiring +slice designs its retry path around, the same way it has to around start times on `save`. + +#### The canonical encoding, and the RFC 8785 failure modes + +RFC 8785 exists because naive JSON serialization is not byte-stable. Three failure modes, each of +which would surface here as a correct retry being rejected as an incompatible rewrite: **key order** +(most serializers emit fields in declaration or reflection order, neither guaranteed stable), +**number rendering** (the same value serializing as `1`, `1.0` or `1e0`, and floating-point +round-tripping differing between implementations), and **insignificant text** (whitespace, escaping, +Unicode normalization). + +The encoding is the one DICE already uses for `MetamodelVersion.contentHash`, applied to a different +payload: + +- every token is length-prefixed, `:`, because a delimiter-joined encoding lets + `["a;b"]` and `["a", "b"]` hash the same, and a length prefix keeps two tokens apart whatever + characters either one carries; +- every collection is preceded by its element count, so a shorter list cannot be a prefix of a + longer one; +- fields are emitted as `(name, value)` pairs sorted by name; +- a collection whose order carries no meaning is sorted, which is what makes two coordinators + recording the same failures in a different order the same terminal write — the order is Kotlin's + natural `String` order, comparing UTF-16 code units, not UTF-8 byte order and not a locale + collation, and it is named because a backend re-implementing the sort in a query would pick a + different one and get a different digest; +- instants render as `.` — fixed width, and independent of + `java.time`'s own formatting, whose precision varies with the value; +- absent is its own marker, so null and empty stay distinguishable; +- SHA-256, lowercase hex, with a version tag on the input so a reader meeting a version it does not + know matches nothing rather than guessing. + +This is a persisted format. `ExtractionRunFingerprintTest` pins the digest of a fixed payload with a +literal assertion, so changing the encoding means changing that literal deliberately. + +**A store records the string the transition computed and never re-derives it.** That is what makes +the encoding's finer points — which sort order, how an instant renders — unable to cause a +cross-backend divergence: only one implementation ever runs. A backend that re-derived the digest +from the stored run in Cypher would also break the payload-only rule above, and +`a replay after an interleaved invocation record is still a replay` in the cross-backend suite is +the case that catches it. + +#### Two mechanisms deliberately not adopted + +**No epoch or writer generation.** Kafka's transactional producer bumps a monotonic epoch on every +`initTransactions()` and fences zombie writers with it, because a bare identity key cannot tell the +same logical writer retrying from an old instance that should be shut out. DICE's concurrency is two +writers racing on one row, which the compare-and-set inside a single store transaction already +decides. Epochs solve fencing across systems; this contract does not span systems. + +**No idempotency-key expiry.** Stripe prunes idempotency records after roughly 24 hours, and a +pruned key starts a fresh request with no comparison. A run header is a permanent audit row, so the +equivalent here would delete the evidence rather than the bookkeeping. + +### Four states, not five MLflow's run status has five values — `RUNNING`, `SCHEDULED`, `FINISHED`, `FAILED`, `KILLED`. The two DICE does not have are deliberate: @@ -311,6 +616,73 @@ The two DICE does not have are deliberate: stopped short of its products, and that is the same fact whichever side pressed stop. `CANCELLED` is also the abandonment path for a partially successful run nobody intends to finish. +## The store contract: every read tenant-scoped and bounded + +A run store grows once per extraction forever, so there is no unbounded read on +`ExtractionRunStore`. Every page takes a positive `limit`, and the reads that can span a long +history also take a `since` window. + +| Read | Answers | +| --- | --- | +| `findRun(key)` | one run, by tenant-qualified identity | +| `invocationsOf(key)` | that run's attempts, in plan order | +| `runsInContext(tenant, limit, since)` | one tenant's runs, newest first | +| `childrenOf(tenant, parent, limit)` | one hop down the parent axis | +| `runsOfRoot(tenant, root, limit, since)` | a whole lineage, in one read | +| `ancestorsOf(key, limit)` | the parent chain, walked upward | + +**Scope is pushed down, never applied afterwards.** An implementation restricts to the tenant inside +the query and then limits. Fetching `limit` rows and filtering by tenant afterwards returns fewer +rows than asked for — or none — whenever a busy neighbouring tenant occupies the head of the index, +and the caller cannot tell that from a tenant with no runs. This is the drift-report store's rule +carried over, and it is why none of the scoped reads has a default body: a default that filtered in +memory would be inherited silently by every backend that forgot to override it. + +The `ContextId`-typed overloads do have default bodies and are a different thing — they forward to +the `String`-typed method that is the override point, and cannot return the wrong rows because they +do not filter. The split exists because `ContextId` is a Kotlin value class, so a method taking one +compiles to a mangled JVM name Java cannot reach. `ExtractionRunKey.of(contextIdValue, runId)` +exists for the same reason. + +**Pages are ordered newest first by start time, tie-broken by run id ascending.** The tie-break is +what makes a page repeatable: two runs started in the same millisecond would otherwise come back in +whatever order the backend felt like, and a caller paging through would see one twice or neither. + +**Cross-tenant reads fail closed.** A run id that exists in two tenants is two runs, and a read +against one never returns the other's. The chain walk stops rather than crossing: a parent reference +that resolves only in another tenant is treated as unresolved. Slice 8 proves this against a real +graph; here it is what every implementation is held to. + +**The chain walk is bounded and cycle-safe, and needs to be both.** `limit` stops it in a lineage +deeper than the caller wants to read. A run already visited stops it outright: a value type can +reject a run that is its own parent, but a two-hop cycle needs the other runs to see, so detecting +one is the store's job. A store holding a cycle is corrupt, and a store that hangs on one is worse. + +`runsOfRoot` is the read the denormalized root reference exists for. The root is fixed when a +lineage is minted and cannot drift, so a whole lineage is one indexed read on one property rather +than a chain walk a hop at a time. + +`InMemoryExtractionRunStore` is the reference implementation, and it is in main sources rather than +test sources for the same reason `InMemoryCollectorTraceStore` is: a host can record and read runs +before it has a database. It therefore has no unscoped read at all, not even a test helper: one +instance holds every tenant's runs, so an "everything in the store" method would hand a host running +the shipped backend a cross-tenant unbounded read on a contract that is neither. The tests read +through the contract like any other caller. + +Compare-and-set is real there, not simulated — every write and read runs inside one monitor, so the +read of a run's status and the write that changes it cannot interleave. A durable store gets the +same guarantee from its transaction, and the cross-backend suite races two threads to end one run +and asserts exactly one `APPLIED` and one `REPLAYED`, so the claim is inherited rather than +remembered. Removing the monitor from `transition` fails that test and nothing else. + +`AbstractExtractionRunStoreContractTest` in `dice-storage` is the cross-backend suite. Each backend +supplies a store and inherits the whole thing, so the Drivine store is held to the in-memory +reference's semantics at authoring time. The cases there are the ones a durable backend gets wrong +in a way a single-backend test would miss: a `MERGE … SET` upsert passes "a terminal write is +recorded" and fails "an incompatible terminal rewrite is rejected", a finder that filters in memory +passes every single-tenant read and fails "a page scopes before it limits", and a chain walk written +as a recursive Cypher pattern passes on a healthy graph and hangs on a cycle. + ## OpenTelemetry GenAI naming, not adopted OTel's GenAI semantic conventions cover the same ground — `gen_ai.request.*`, `gen_ai.response.*`, @@ -374,12 +746,12 @@ all. A failure that happened outside any model call names no invocation and is a Everything here is immutable and validated in `init`, with `@JvmStatic`/`@JvmOverloads` factories on the types that have optional parameters. -`ExtractionRun` itself is a plain class rather than a data class, for two reasons. A data class has +`ExtractionRun` is a plain class, for two reasons. A data class has to declare its collection parameters as properties, which means the field *is* the caller's list and there is nowhere to copy it. And a generated `copy`/`componentN` surface would pin an ABI -across seventeen fields while #67 is still moving. Equality and hash are written out over every -component, and a test varies each of the seventeen in turn so a component dropped from `equals` -fails rather than passing quietly. +across eighteen fields while #67 is still moving. Equality and hash are written out over every +component, and a test varies each of the eighteen in turn, so a component dropped from `equals` +makes that test fail. Collections are copied on the way in **unconditionally**, empty ones included. A copy skipped when the list is empty leaves the run aliasing a list the caller still holds, and the caller fills it @@ -396,22 +768,25 @@ As with #66, a Kotlin `@RequiresOptIn` marker would make the status enforceable rather than advisory. DICE defines none today, and inventing one is a decision about the whole public surface. -## What this slice does not do +## What is not here yet -- **No store.** Nothing persists a run. The store contract, the in-memory implementation and the - lifecycle state machine are the next slice; the Drivine implementation and its schema follow. -- **No lifecycle.** There are four status values and no transitions. Nothing here can move a run - from `RUNNING` to anything. -- **No coordinator.** Nothing constructs an `ExtractionRun` during extraction yet, so the - sanitization tests reproduce the leak path rather than driving a real extraction into a stored - run. +- **No durable store.** `InMemoryExtractionRunStore` is the only implementation. The Drivine store + — the tenant-qualified natural key, the deterministic child key for invocation records, the + uniqueness constraints, and the Cypher that scopes before it limits — is the next slice. +- **No coordinator.** Nothing constructs an `ExtractionRun` during extraction yet, and nothing calls + `save` or `transition` outside tests. Which means the `COMPLETED` precondition is documented and + structurally narrowed, not observed: the wiring slice is where "the coordinator really does wait + for `persistAndProject`" becomes a test rather than a contract clause. - **No proposition-to-run relation.** Attribution from a claim to the runs that produced or confirmed it is its own slice, on canonical saved ids, and run identity stays out of source-provenance equality. -- **No protected-content references.** Optional replay material represented by classified, - expiring references is part of #67 and is not in this slice; the model's current answer to - replay material is that there is none. +- **No protected-content reference type.** A first cut (`ProtectedContentRef`, + `ProtectedContentClassification`, `ProtectedContentHandle`) landed and was removed again: nothing + in DICE attached one to an `ExtractionRun`, read one, or enforced its retention, so it was a shape + with no runtime path exercising it. It returns with the first runtime path that needs it — a + writer, a reader, or retention behaviour. A value type with no consumer does not stay on the + branch. - **No per-invocation requested configuration.** The requested configuration is one record on the run header. A later slice that needs to vary settings per call adds a separate requested record keyed by invocation index rather than a field on the observed record, which would collapse the - distinction this slice exists to draw. + distinction this model exists to draw. From 34d3f33ca014847fae621f81bd6a0f325d881dbc Mon Sep 17 00:00:00 2001 From: James Dunnam <7660553+jimador@users.noreply.github.com> Date: Wed, 2 Sep 2026 01:32:22 -0400 Subject: [PATCH 2/7] Hash only transition identity and announce applied transitions The terminal fingerprint covered counts and failures, so any change to their shape would have changed a persisted format; it now hashes status and finishedAt alone under xrun-terminal:v2, and counts and failures ride as data. A typed product outcome in a later slice changes no stored byte. The store announces an applied transition through ExtractionRunTransitioned, emitted once where the transition is accepted: a replay announces nothing and a rejected write announces nothing, pinned by contract. The suite carries the reshaped failure vocabulary throughout. Signed-off-by: James Dunnam <7660553+jimador@users.noreply.github.com> --- CHANGELOG.md | 47 +++ .../AbstractExtractionRunStoreContractTest.kt | 281 ++++++++++++++++-- .../InMemoryExtractionRunStoreContractTest.kt | 4 +- .../com/embabel/dice/common/DiceEvent.kt | 26 ++ .../extraction/ExtractionRunFingerprint.kt | 100 ++----- .../extraction/ExtractionRunStore.kt | 22 +- .../extraction/ExtractionRunTransition.kt | 14 +- .../extraction/InMemoryExtractionRunStore.kt | 48 ++- .../ExtractionRunFingerprintTest.kt | 209 +++++-------- .../extraction/ExtractionRunLifecycleTest.kt | 83 ++++-- .../extraction/ExtractionRunValueTypesTest.kt | 1 + docs/design/extraction-runs.md | 77 ++++- 12 files changed, 615 insertions(+), 297 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ddc9a98..e4142903 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1555,3 +1555,50 @@ and the consumer PRs that deliver it). that update land through `save` and has to call `recordInvocation` directly instead; no shipped caller does this today, since nothing outside a test calls `save` with a non-empty invocation list yet. Design note: [docs/design/extraction-runs.md](docs/design/extraction-runs.md). + +- **EXPERIMENTAL, reworks the two entries above.** The terminal fingerprint now covers the + transition's identity alone, and a run that ends announces itself. **The digest narrowed, and + `ExtractionRunFingerprint.TERMINAL_VERSION` moved from `xrun-terminal:v1` to `xrun-terminal:v2` + with it.** `ofTerminal` takes the terminal status and the finish time; the counts and failures a + transition carries no longer reach the hashed bytes, and `ofTerminal`'s two payload parameters are + gone. Two writes that agree on status and finish time are the same terminal write, so the second + replays whatever numbers it names, and the run keeps what the first accepted terminal write + delivered — a run's outcome is written once. **Why.** Folding the outcome into the digest made + every difference in the payload an incompatible rewrite, which reads as safe and buys an audit + nothing: the first write had already landed and the second changed nothing either way, so the only + thing the wider digest decided was whether the caller heard "conflict" or "replay". What it cost + was the persisted format. The digest is stored beside a run and compared against every retry, and + under `v1` it moved whenever the outcome payload gained a field — so DICE #69's typed product + outcomes would have changed a format already written to disk. Under `v2` every field #69 adds + lands in the counts-and-failures half, reaches none of the hashed bytes, and no recorded digest + stops matching. Counts and failures still travel on the transition and still reach the terminal + run: `null` keeps what the run recorded and a value replaces it, exactly as before. What changed + is that the distinction decides the row a store writes, and the digest never sees it. **A run that ends emits a + `DiceEvent`, exactly once.** `ExtractionRunTransitioned` carries the run in its terminal state and + fires from the store for the call that ended it. A `REPLAYED` transition emits nothing, because a + coordinator retrying a terminal write whose answer it never saw would otherwise notify every + downstream consumer a second time for a run that ended once; a rejected write, a `save` and a + `recordInvocation` emit nothing either. The listener reaches the store as a constructor + collaborator defaulting to `DiceEventListener.DEV_NULL`, the shape + `EventEmittingPropositionRepository` already uses — nothing is wired automatically, and a host + with nothing listening constructs the store as it always did. The announcement is made after the + write has landed and outside the store's own lock, so a listener that blocks or reads the run back + holds up no other writer. **The contract suite carries both promises.** Its store factory now + takes a `DiceEventListener`, since a suite that could not observe the announcement could not hold + a backend to it, and the no-argument `store()` forwards to it with `DEV_NULL`. New cases: a retry + carrying different counts and failures replays and the first write's outcome stands; keeping + counts and replacing them are different claims on the run that lands, and the digest sees neither; + one announcement per applied transition and none for a replay; every terminal status announces the + run it ended; a rejected terminal write announces nothing; a `save` and a `recordInvocation` + announce nothing; and eight threads racing to end one run produce one announcement between them. + The cases that pinned counts and failures as part of the compared payload are gone, because the + digest no longer covers them. **Compatibility: breaking for two callers, neither of which exists + yet.** `ExtractionRunFingerprint.ofTerminal` loses its `counts` and `failures` parameters, so a + caller passing them stops compiling; nothing outside `dice`'s own tests calls it. Every recorded + `v1` digest becomes unmatchable, and nothing durable holds one — no store outside the in-memory + reference has written a run — so no migration follows. `InMemoryExtractionRunStore` gains an + optional first constructor parameter with `@JvmOverloads`, so its no-argument construction keeps + working from Kotlin and Java alike. `ExtractionRunStore`'s interface is unchanged. + `ExtractionRunTransitioned` is new and carries `@ApiStatus.Experimental` like every other type in + this train, added to the same class-file assertion. Design note: + [docs/design/extraction-runs.md](docs/design/extraction-runs.md). diff --git a/dice-storage/src/test/kotlin/com/embabel/dice/storage/AbstractExtractionRunStoreContractTest.kt b/dice-storage/src/test/kotlin/com/embabel/dice/storage/AbstractExtractionRunStoreContractTest.kt index 97a57556..68643135 100644 --- a/dice-storage/src/test/kotlin/com/embabel/dice/storage/AbstractExtractionRunStoreContractTest.kt +++ b/dice-storage/src/test/kotlin/com/embabel/dice/storage/AbstractExtractionRunStoreContractTest.kt @@ -16,6 +16,9 @@ package com.embabel.dice.storage import com.embabel.agent.core.ContextId +import com.embabel.dice.common.DiceEvent +import com.embabel.dice.common.DiceEventListener +import com.embabel.dice.common.ExtractionRunTransitioned import com.embabel.dice.proposition.extraction.ExtractionActorRef import com.embabel.dice.proposition.extraction.ExtractionCohortRef import com.embabel.dice.proposition.extraction.ExtractionContentProfileRef @@ -66,8 +69,34 @@ import java.time.Instant */ abstract class AbstractExtractionRunStoreContractTest { - /** A store holding nothing for the tenants below. */ - protected abstract fun store(): ExtractionRunStore + /** + * A store holding nothing for the tenants below, announcing what it does to [listener]. + * + * The listener is the one construction argument every backend has to accept, because "a run + * that ends announces itself once" is part of the contract and a suite that could not observe + * the announcement could not hold a backend to it. + */ + protected abstract fun store(listener: DiceEventListener): ExtractionRunStore + + /** A store holding nothing for the tenants below, with nothing listening. */ + protected fun store(): ExtractionRunStore = store(DiceEventListener.DEV_NULL) + + /** Keeps every event a store hands it, so a test can count them. */ + private class RecordingListener : DiceEventListener { + + private val received = mutableListOf() + + override fun onEvent(event: DiceEvent) { + synchronized(received) { received += event } + } + + /** Everything received so far, as a snapshot the caller can read at its leisure. */ + fun events(): List = synchronized(received) { received.toList() } + + /** The runs announced as ended, in the order they were announced. */ + fun transitioned(): List = + events().filterIsInstance().map { it.run } + } private val tenant = ContextId("contract-tenant") private val neighbour = ContextId("contract-neighbour") @@ -236,6 +265,9 @@ abstract class AbstractExtractionRunStoreContractTest { @Test fun `an incompatible terminal rewrite is rejected and changes nothing`() { + // Incompatible means a different transition identity: another terminal status, or another + // finish time. Both are checked here; the finish-time half gets its own per-status test + // below, because this one varies status while holding finish time fixed. val store = store() val run = running("contract-rewrite") store.save(run) @@ -251,15 +283,179 @@ abstract class AbstractExtractionRunStoreContractTest { assertThrows(ExtractionRunConflictException::class.java) { store.transition( run.key(), - ExtractionRunTransition.completed( - finishedAt = finishedAt, - counts = ExtractionRunCounts(propositionsPersisted = 8), - ), + ExtractionRunTransition.completed(finishedAt.plusSeconds(1)), ) } assertEquals(applied, store.findRun(run.key())) } + @Test + fun `a retry carrying different counts and failures replays, and the first write's outcome stands`() { + // The fingerprint covers the transition's identity — the terminal status and the finish + // time — so a second write agreeing on both is the same terminal write however different + // the numbers it carries. It replays, and the run keeps what the write that landed first + // delivered: a run's outcome is written once. A backend folding counts or failures into its + // own digest would reject this as an incompatible rewrite, and one that took the retry's + // payload would overwrite an outcome that had already been announced. + val store = store() + val run = running("contract-outcome-once") + store.save(run) + val landed = store.transition( + run.key(), + ExtractionRunTransition.completed( + finishedAt, + counts = ExtractionRunCounts(propositionsPersisted = 7), + failures = emptyList(), + ), + ) + + val retry = store.transition( + run.key(), + ExtractionRunTransition.completed( + finishedAt, + counts = ExtractionRunCounts(propositionsPersisted = 99, entitiesResolved = 4), + failures = listOf( + ExtractionFailure.of(ExtractionFailureCode.INTERNAL, ExtractionFailureStage.PERSISTENCE), + ), + ), + ) + + assertEquals(ExtractionRunTransitionOutcome.APPLIED, landed.outcome) + assertEquals(ExtractionRunTransitionOutcome.REPLAYED, retry.outcome) + assertEquals(landed.run, retry.run) + assertEquals(landed.run, store.findRun(run.key())) + assertEquals(7, store.findRun(run.key())?.counts?.propositionsPersisted) + assertEquals(emptyList(), store.findRun(run.key())?.failures) + } + + // ---- announcing a run that ended ---- + + @Test + fun `an applied transition announces the run once, and a replay announces nothing`() { + val listener = RecordingListener() + val store = store(listener) + val run = running("contract-announce-once") + store.save(run) + val transition = ExtractionRunTransition.completed( + finishedAt, + counts = ExtractionRunCounts(propositionsPersisted = 7), + ) + + val applied = store.transition(run.key(), transition) + assertEquals(ExtractionRunTransitionOutcome.APPLIED, applied.outcome) + assertEquals( + listOf(applied.run), + listener.transitioned(), + "the call that ended the run must announce it exactly once, carrying the terminal run", + ) + + store.transition(run.key(), transition) + store.transition(run.key(), transition) + assertEquals( + listOf(applied.run), + listener.transitioned(), + "a replay changed nothing and must announce nothing; a run that ended once is announced once", + ) + } + + @Test + fun `every terminal status announces the run it ended`() { + listOf( + ExtractionRunStatus.COMPLETED to ExtractionRunTransition.completed(finishedAt), + ExtractionRunStatus.FAILED to ExtractionRunTransition.failed(finishedAt), + ExtractionRunStatus.CANCELLED to ExtractionRunTransition.cancelled(finishedAt), + ).forEach { (expected, transition) -> + val listener = RecordingListener() + val store = store(listener) + val run = running("contract-announce-$expected") + store.save(run) + + store.transition(run.key(), transition) + + assertEquals( + listOf(expected), + listener.transitioned().map { it.status }, + "$expected must be announced like every other terminal status", + ) + assertEquals( + listOf(run.key()), + listener.transitioned().map { it.key() }, + "the announced run must be the one that ended, tenant included", + ) + } + } + + @Test + fun `writes that do not end a run announce nothing`() { + // A backend announcing on every write would pass the exactly-once test above and still + // notify a downstream consumer about a run that has not finished. + val listener = RecordingListener() + val store = store(listener) + val run = running("contract-announce-silence") + + store.save(run) + store.recordInvocation(run.key(), ExtractionInvocationRecord.planned(0)) + store.save(store.findRun(run.key())!!.let { stored -> headerVariant(stored, counts = ExtractionRunCounts(sourcesRead = 1)) }) + store.findRun(run.key()) + store.runsInContext(tenant, 10, null) + + assertEquals(emptyList(), listener.events()) + } + + @Test + fun `a rejected terminal write announces nothing`() { + val listener = RecordingListener() + val store = store(listener) + val run = running("contract-announce-rejected") + store.save(run) + val applied = store.transition(run.key(), ExtractionRunTransition.completed(finishedAt)).run + + assertThrows(ExtractionRunConflictException::class.java) { + store.transition(run.key(), ExtractionRunTransition.failed(finishedAt)) + } + + assertEquals( + listOf(applied), + listener.transitioned(), + "the rejected write announced nothing, so the applied one's announcement stands alone", + ) + } + + @Test + fun `concurrent terminal writers announce the run exactly once between them`() { + // The race that makes exactly-once worth pinning: many threads send the same terminal + // write, one applies it and the rest replay. A backend announcing outside its own + // compare-and-set, or on the replay path, notifies a consumer once per caller. + val listener = RecordingListener() + val store = store(listener) + val run = running("contract-announce-race") + store.save(run) + val transition = ExtractionRunTransition.completed(finishedAt) + val writers = 8 + val ready = java.util.concurrent.CountDownLatch(writers) + val go = java.util.concurrent.CountDownLatch(1) + val pool = java.util.concurrent.Executors.newFixedThreadPool(writers) + try { + val outcomes = (1..writers).map { + pool.submit { + ready.countDown() + go.await() + store.transition(run.key(), transition).outcome + } + } + ready.await() + go.countDown() + val results = outcomes.map { it.get() } + + assertEquals(1, results.count { it == ExtractionRunTransitionOutcome.APPLIED }) + assertEquals(writers - 1, results.count { it == ExtractionRunTransitionOutcome.REPLAYED }) + } finally { + pool.shutdownNow() + } + + assertEquals(1, listener.transitioned().size, "$writers writers, one run that ended, one announcement") + } + @Test fun `a retry differing only in finish time is an incompatible rewrite, for every terminal status`() { // docs/design/extraction-runs.md:432 promises finish time participates in the fingerprint @@ -336,43 +532,66 @@ abstract class AbstractExtractionRunStoreContractTest { } @Test - fun `keeping counts and replacing them with the same values are different terminal writes`() { - // Null means keep and a value means replace, so they are different claims even when they - // land on the same numbers. A backend that normalised null to the run's stored counts before - // fingerprinting would replay the second as the first. - val store = store() - val run = ExtractionRun( + fun `keeping counts and replacing them are different claims on the run that lands, and the digest sees neither`() { + // Null means keep and a value means replace, and the difference shows in what the terminal + // run holds. The digest sees neither: both writes name the same status and finish time, so + // whichever arrives second replays. A backend has to get both halves right — apply the + // distinction to the row it writes, and keep it out of the string it compares retries by. + val keepStore = store() + val keepRun = ExtractionRun( contextId = tenant, lineage = ExtractionRunLineage.root(ExtractionRunRef("contract-kept")), status = ExtractionRunStatus.RUNNING, startedAt = startedAt, counts = ExtractionRunCounts(propositionsPersisted = 4), ) - store.save(run) - val originalTransition = ExtractionRunTransition.completed(finishedAt, counts = null) - val kept = store.transition(run.key(), originalTransition).run + keepStore.save(keepRun) + val keptTerminal = keepStore.transition( + keepRun.key(), + ExtractionRunTransition.completed(finishedAt, counts = null), + ).run + assertEquals(4, keptTerminal.counts.propositionsPersisted) - assertThrows(ExtractionRunConflictException::class.java) { - store.transition( - run.key(), + val replaceStore = store() + val replaceRun = ExtractionRun( + contextId = tenant, + lineage = ExtractionRunLineage.root(ExtractionRunRef("contract-replaced")), + status = ExtractionRunStatus.RUNNING, + startedAt = startedAt, + counts = ExtractionRunCounts(propositionsPersisted = 4), + ) + replaceStore.save(replaceRun) + val replacedTerminal = replaceStore.transition( + replaceRun.key(), + ExtractionRunTransition.completed( + finishedAt, + counts = ExtractionRunCounts(entitiesResolved = 9), + ), + ).run + assertEquals(0, replacedTerminal.counts.propositionsPersisted) + assertEquals(9, replacedTerminal.counts.entitiesResolved) + assertEquals(replacedTerminal, replaceStore.findRun(replaceRun.key())) + + // The digest half: against the run that kept its counts, a write naming the same status and + // finish time replays, whether it repeats "keep" or asks to replace. + assertEquals( + ExtractionRunTransitionOutcome.REPLAYED, + keepStore.transition( + keepRun.key(), + ExtractionRunTransition.completed(finishedAt, counts = null), + ).outcome, + ) + assertEquals( + ExtractionRunTransitionOutcome.REPLAYED, + keepStore.transition( + keepRun.key(), ExtractionRunTransition.completed( finishedAt, counts = ExtractionRunCounts(propositionsPersisted = 4), ), - ) - } - // The rejected second write must not have touched storage, even though it names the same - // numbers the first write kept. - assertEquals(kept, store.findRun(run.key())) - // findRun alone cannot tell which fingerprint is actually stored: both transitions produce - // the same visible counts, so a backend that recorded the rejected write's fingerprint - // internally, while still throwing and leaving the visible run alone, would pass the - // assertion above. Replaying the original transition surfaces the stored fingerprint - // directly: it must still be recognised as the same write that already landed. - assertEquals( - ExtractionRunTransitionOutcome.REPLAYED, - store.transition(run.key(), originalTransition).outcome, + ).outcome, ) + assertEquals(keptTerminal, keepStore.findRun(keepRun.key())) } @Test diff --git a/dice-storage/src/test/kotlin/com/embabel/dice/storage/InMemoryExtractionRunStoreContractTest.kt b/dice-storage/src/test/kotlin/com/embabel/dice/storage/InMemoryExtractionRunStoreContractTest.kt index ad459914..fb6d7cb2 100644 --- a/dice-storage/src/test/kotlin/com/embabel/dice/storage/InMemoryExtractionRunStoreContractTest.kt +++ b/dice-storage/src/test/kotlin/com/embabel/dice/storage/InMemoryExtractionRunStoreContractTest.kt @@ -15,6 +15,7 @@ */ package com.embabel.dice.storage +import com.embabel.dice.common.DiceEventListener import com.embabel.dice.proposition.extraction.ExtractionRunStore import com.embabel.dice.proposition.extraction.InMemoryExtractionRunStore @@ -24,5 +25,6 @@ import com.embabel.dice.proposition.extraction.InMemoryExtractionRunStore * Drivine run store completes when it lands. */ class InMemoryExtractionRunStoreContractTest : AbstractExtractionRunStoreContractTest() { - override fun store(): ExtractionRunStore = InMemoryExtractionRunStore() + override fun store(listener: DiceEventListener): ExtractionRunStore = + InMemoryExtractionRunStore(listener) } diff --git a/dice/src/main/kotlin/com/embabel/dice/common/DiceEvent.kt b/dice/src/main/kotlin/com/embabel/dice/common/DiceEvent.kt index 0f3ac31b..1df01816 100644 --- a/dice/src/main/kotlin/com/embabel/dice/common/DiceEvent.kt +++ b/dice/src/main/kotlin/com/embabel/dice/common/DiceEvent.kt @@ -20,7 +20,9 @@ import com.embabel.common.core.types.Timestamped import com.embabel.dice.pipeline.PropositionExtractionStats import com.embabel.dice.proposition.Proposition import com.embabel.dice.proposition.PropositionStatus +import com.embabel.dice.proposition.extraction.ExtractionRun import com.fasterxml.jackson.annotation.JsonTypeInfo +import org.jetbrains.annotations.ApiStatus import java.time.Instant /** @@ -160,6 +162,30 @@ data class ExtractionBatchCompleted @JvmOverloads constructor( override val timestamp: Instant = Instant.now(), ) : DiceEvent +/** + * An extraction run ended, and this is the call that ended it. + * + * It fires once per run, from the store, after the terminal write has landed. A coordinator + * retrying a terminal write it never saw the answer to gets a replay, and a replay fires nothing — + * so a listener counting finished runs, or kicking off work behind one, sees each run once however + * many times its terminal write was sent. + * + * The run carries everything there is to know about how it ended: the tenant, the lineage, the + * terminal status, the finish time, the counts, and the failures in their closed vocabulary. There + * is no source text or provider message anywhere in it, so this event is safe to hand to a listener + * that logs or forwards what it receives. + * + * EXPERIMENTAL. The shape may still change while extraction runs (DICE #67) land. + * + * @property run The run in its terminal state. + * @property timestamp When the event was created. + */ +@ApiStatus.Experimental +data class ExtractionRunTransitioned @JvmOverloads constructor( + val run: ExtractionRun, + override val timestamp: Instant = Instant.now(), +) : DiceEvent + /** * A proposition moved to a different lifecycle [PropositionStatus] — for example going stale * during a decay sweep, or coming back to life when it's seen again. diff --git a/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunFingerprint.kt b/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunFingerprint.kt index ba48038c..492e218b 100644 --- a/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunFingerprint.kt +++ b/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunFingerprint.kt @@ -27,6 +27,17 @@ import java.time.Instant * whole mechanism rests on two payloads that mean the same thing producing the same bytes, so the * encoding is specified here rather than left to whatever a serializer happens to emit. * + * ## What the digest covers + * + * The transition's identity: the terminal status, and when the run finished. Two writes that agree + * on those two things are the same terminal write, and the second replays. + * + * The counts and failures a transition carries are data it delivers, and they stay outside the + * digest. A run's outcome is written once — the first accepted terminal write wins, and a retry + * carrying different numbers replays against what is already stored. Keeping them out of the digest + * is what lets the outcome payload grow: DICE #69 adds typed product outcomes to what a terminal + * write reports, and no field it adds can move a digest already recorded beside a run. + * * ## Why not JSON * * RFC 8785 exists because naive JSON serialization is not byte-stable. Three failure modes, all of @@ -40,39 +51,31 @@ import java.time.Instant * bytes without moving the meaning. * * This encoding is the one DICE already uses for `MetamodelVersion.contentHash`, applied to a - * different payload: length-prefixed tokens, count-prefixed collections, sorted where order carries - * no meaning, SHA-256, lowercase hex. + * different payload: length-prefixed tokens, count-prefixed field sets, SHA-256, lowercase hex. * * ## The rules * * 1. **Every token is length-prefixed**, `:`. A delimiter-joined encoding lets * `["a;b"]` and `["a", "b"]` hash the same, which hides a real difference. Length prefixes make * that collision unreachable whatever characters a token happens to carry. - * 2. **Every collection is preceded by its element count**, so a shorter list can never be a prefix - * of a longer one. + * 2. **A field set is preceded by how many fields it holds**, so a shorter one can never be a + * prefix of a longer one. * 3. **Fields are emitted as `(name, value)` pairs sorted by name**, so the bytes do not depend on * the order the fields happen to be declared in. - * 4. **A collection whose order carries no meaning is sorted.** The failure list is the case that - * matters: two coordinators that recorded the same failures in a different order made the same - * terminal write. The order is Kotlin's natural `String` order, which compares UTF-16 code units - * — not UTF-8 byte order, and not a locale collation. Any total order would do for correctness; - * naming this one matters because a backend re-implementing the sort in a query would pick a - * different one and produce a different digest. It should not re-implement it at all: the store - * records the string the transition already computed. - * 5. **Instants render as `.`.** Fixed width, and independent of + * 4. **Instants render as `.`.** Fixed width, and independent of * `java.time`'s own formatting. `Instant.toString()` varies its precision with the value — * `…:47Z` for a whole second, `…:47.500Z` for half of one — so the encoded length moves with the * data and a persisted digest would depend on a formatting rule DICE does not own. Number * rendering is the equivalent hazard in JSON and is most of what RFC 8785 is about. - * 6. **Absent is its own marker.** A null renders as `-`, never as an empty string, so "the caller - * said nothing" and "the caller said empty" stay distinguishable. - * 7. **The digest is SHA-256, rendered lowercase hex**, and the encoded input carries a version + * 5. **The digest is SHA-256, rendered lowercase hex**, and the encoded input carries a version * tag. A reader meeting a version it does not know matches nothing rather than guessing. * * ## This is a persisted format * * The digest is stored beside the run. Changing the encoding makes every recorded fingerprint * unmatchable, so a correct retry against an old run would be rejected as an incompatible rewrite. + * That is what [TERMINAL_VERSION] is for, and it is why the payload is the transition's identity + * alone: a field added to what a terminal write reports never reaches these bytes. * `ExtractionRunFingerprintTest` pins the digest of a fixed payload with a literal assertion: * changing the encoding means changing that literal deliberately. * @@ -81,70 +84,33 @@ import java.time.Instant @ApiStatus.Experimental object ExtractionRunFingerprint { - /** Version tag on the terminal-write encoding. */ - const val TERMINAL_VERSION: String = "xrun-terminal:v1" - - /** What an absent value encodes as, kept distinct from any value a caller could supply. */ - private const val ABSENT = "-" + /** + * Version tag on the terminal-write encoding. + * + * `v2` narrowed the payload to the transition's identity. `v1` also folded in the counts and + * the failures a transition carried, which made the digest move whenever the outcome payload + * gained a field. + */ + const val TERMINAL_VERSION: String = "xrun-terminal:v2" /** - * The digest of one terminal write: the status it asserts, when the run finished, the counts it - * records, and the failures it records. + * The digest of one terminal write: the status it asserts, and when the run finished. * - * The run itself is deliberately not part of this. A coordinator that records another invocation - * between a failed terminal write and its retry made the same terminal write both times, and - * folding the run's invocation list in would turn that correct retry into a rejected conflict. + * Two things stay out of it deliberately. The run, because a coordinator that records another + * invocation between a terminal write and its retry made the same terminal write both times, + * and folding the run's invocation list in would turn that correct retry into a rejected + * conflict. And the counts and failures the transition carries, because those are the outcome + * a run reports, while the digest names which terminal write this is — see the class doc. */ @JvmStatic - fun ofTerminal( - status: ExtractionRunStatus, - finishedAt: Instant, - counts: ExtractionRunCounts?, - failures: List?, - ): String { + fun ofTerminal(status: ExtractionRunStatus, finishedAt: Instant): String { val payload = fields( - "counts" to (counts?.let(::encodeCounts) ?: ABSENT), - "failures" to (failures?.let(::encodeFailures) ?: ABSENT), "finishedAt" to encodeInstant(finishedAt), "status" to status.name, ) return digest(TERMINAL_VERSION + "|" + payload) } - private fun encodeCounts(counts: ExtractionRunCounts): String = fields( - "chunksProcessed" to counts.chunksProcessed.toString(), - "entitiesResolved" to counts.entitiesResolved.toString(), - "propositionsExtracted" to counts.propositionsExtracted.toString(), - "propositionsPersisted" to counts.propositionsPersisted.toString(), - "propositionsRejected" to counts.propositionsRejected.toString(), - "sourcesRead" to counts.sourcesRead.toString(), - ) - - private fun encodeFailures(failures: List): String { - val encoded = failures.map { failure -> - fields( - "at" to encodeInstant(failure.at), - "code" to failure.code.name, - "invocation" to ( - failure.invocation - ?.let { "${it.invocationIndex}/${it.attempt}" } - ?: ABSENT - ), - "measure" to ( - failure.measure - ?.let { "${it.quantity.name}=${it.value}" } - ?: ABSENT - ), - "providerStatus" to (failure.providerStatus?.toString() ?: ABSENT), - "stage" to (failure.stage?.name ?: ABSENT), - ) - }.sorted() - return buildString { - append(encoded.size).append('|') - encoded.forEach { appendSized(it) } - } - } - /** * Fixed-width rendering: seconds since the epoch, a dot, then nanoseconds padded to nine * digits. Two instants that compare equal always render identically, and no instant renders diff --git a/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunStore.kt b/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunStore.kt index e6773fc1..2a9286e3 100644 --- a/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunStore.kt +++ b/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunStore.kt @@ -257,13 +257,25 @@ interface ExtractionRunStore { /** * Ends a run: compare-and-set from `RUNNING` to the transition's terminal status. * - * **A replay needs the identical payload, finish time included.** A coordinator retrying after a - * crash it never saw the answer to must reuse the transition it built the first time, or read - * the run back with [findRun] and stop if it has already ended. Minting a fresh `finishedAt` on - * the retry produces a different fingerprint, which is an incompatible rewrite and is rejected — - * safe, and the opposite of what a caller expecting idempotency would predict. It is the + * **A replay needs the identical finish time.** A coordinator retrying after a crash it never + * saw the answer to must reuse the transition it built the first time, or read the run back + * with [findRun] and stop if it has already ended. Minting a fresh `finishedAt` on the retry + * produces a different fingerprint, which is an incompatible rewrite and is rejected — safe, + * and the opposite of what a caller expecting idempotency would predict. It is the * transition-side twin of the rule [save] states for start times. * + * **A run's outcome is written once.** The fingerprint covers the terminal status and the + * finish time; the counts and failures a transition carries stay outside it. A retry that + * agrees on status and finish time replays whatever numbers it names, and the run keeps what + * the first accepted terminal write delivered. A coordinator with better numbers than the ones + * that landed has to record them before it ends the run. + * + * **An applied transition emits one [com.embabel.dice.common.ExtractionRunTransitioned], a + * replay emits none.** A store notifies its listener after the write it just made, once, for + * the call that ended the run. A replay changed nothing, so it announces nothing: a coordinator + * retrying a call whose answer it never saw would otherwise notify every downstream consumer a + * second time for a run that ended once. A rejected write announces nothing either. + * * @param key The run to end. * @param transition What the run ended as. * @return The terminal run, and whether this call ended it or replayed a write already diff --git a/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunTransition.kt b/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunTransition.kt index 14f1d9fa..3483eab1 100644 --- a/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunTransition.kt +++ b/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunTransition.kt @@ -37,8 +37,12 @@ import java.util.Collections * for the same reason from the other side: a run that retried past a failed attempt and finished * still happened. * - * [fingerprint] is what a store compares a retry against; see [ExtractionRunFingerprint] for the - * encoding and why it is specified rather than left to a serializer. + * **[fingerprint] names the write, and the counts and failures ride beside it.** A store compares a + * repeated terminal write against the digest of [status] and [finishedAt] alone. Two transitions + * that agree on those are the same write, so the second replays and the run keeps the counts and + * failures the first one delivered — a run's outcome is written once. See + * [ExtractionRunFingerprint] for the encoding, and for why a payload that grows leaves the digest + * where it is. * * EXPERIMENTAL. The shape may still change while extraction runs (DICE #67) land. * @@ -74,9 +78,11 @@ class ExtractionRunTransition @JvmOverloads constructor( /** * The digest a store compares a repeated terminal write against, computed once at construction. + * + * It covers this transition's identity — [status] and [finishedAt] — and nothing else. See + * [ExtractionRunFingerprint] for why [counts] and [failures] travel as data beside it. */ - val fingerprint: String = - ExtractionRunFingerprint.ofTerminal(status, finishedAt, counts, this.failures) + val fingerprint: String = ExtractionRunFingerprint.ofTerminal(status, finishedAt) /** * Derives the terminal run this transition produces from the running one it is applied to. diff --git a/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/InMemoryExtractionRunStore.kt b/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/InMemoryExtractionRunStore.kt index 0472e8db..2ee3437c 100644 --- a/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/InMemoryExtractionRunStore.kt +++ b/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/InMemoryExtractionRunStore.kt @@ -15,6 +15,8 @@ */ package com.embabel.dice.proposition.extraction +import com.embabel.dice.common.DiceEventListener +import com.embabel.dice.common.ExtractionRunTransitioned import org.jetbrains.annotations.ApiStatus import java.time.Instant @@ -54,12 +56,23 @@ import java.time.Instant * contract is neither — and a host running the shipped in-memory backend would have one. The tests * read through the contract like any other caller. * + * **A run that ends announces itself once.** `transition` hands an [ExtractionRunTransitioned] to + * [listener] for the call that ended the run, after the write has landed and outside the monitor, + * so a slow listener holds up no other writer. A replay and a rejected write announce nothing. + * * Nothing here survives the JVM, and two instances know nothing about each other. * * EXPERIMENTAL. The shape may still change while extraction runs (DICE #67) land. + * + * @property listener Notified when a run ends. Defaults to [DiceEventListener.DEV_NULL], so a host + * that has nothing listening constructs the store the same way it always did. Handlers run inline + * on the calling thread, and throw isolation belongs to the listener — wrap it in + * `SafeDiceEventListener` for graceful degradation. */ @ApiStatus.Experimental -class InMemoryExtractionRunStore : ExtractionRunStore { +class InMemoryExtractionRunStore @JvmOverloads constructor( + private val listener: DiceEventListener = DiceEventListener.DEV_NULL, +) : ExtractionRunStore { private val lock = Any() @@ -162,27 +175,32 @@ class InMemoryExtractionRunStore : ExtractionRunStore { key: ExtractionRunKey, transition: ExtractionRunTransition, ): ExtractionRunTransitionResult { - synchronized(lock) { + val result = synchronized(lock) { val stored = runs[key] ?: throw ExtractionRunNotFoundException(key) if (stored.status.isTerminal) { val recorded = terminalWrites[key] - if (recorded == transition.fingerprint) { - return ExtractionRunTransitionResult( - stored, - ExtractionRunTransitionOutcome.REPLAYED, + if (recorded != transition.fingerprint) { + throw ExtractionRunConflictException( + key, + "already ended as ${stored.status} under a different terminal write; " + + "this one claims ${transition.status}", ) } - throw ExtractionRunConflictException( - key, - "already ended as ${stored.status} under a different terminal write; " + - "this one claims ${transition.status}", - ) + ExtractionRunTransitionResult(stored, ExtractionRunTransitionOutcome.REPLAYED) + } else { + val terminal = transition.applyTo(stored) + runs[key] = terminal + terminalWrites[key] = transition.fingerprint + ExtractionRunTransitionResult(terminal, ExtractionRunTransitionOutcome.APPLIED) } - val terminal = transition.applyTo(stored) - runs[key] = terminal - terminalWrites[key] = transition.fingerprint - return ExtractionRunTransitionResult(terminal, ExtractionRunTransitionOutcome.APPLIED) } + // Announced outside the monitor, so a listener that blocks or reads the store back holds up + // no other writer and sees the terminal run already committed. Exactly one call per run + // reaches the applied branch above; a replay reports the run and stays silent. + if (result.isApplied) { + listener.onEvent(ExtractionRunTransitioned(result.run)) + } + return result } override fun findRun(key: ExtractionRunKey): ExtractionRun? = synchronized(lock) { runs[key] } diff --git a/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunFingerprintTest.kt b/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunFingerprintTest.kt index be1420c5..92d8b98a 100644 --- a/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunFingerprintTest.kt +++ b/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunFingerprintTest.kt @@ -23,10 +23,14 @@ import java.time.Instant /** * The canonical encoding behind terminal-write idempotency. * - * Two things have to hold. Two payloads that mean the same must produce the same digest, or a - * correct retry is rejected as an incompatible rewrite. Two payloads that differ must produce - * different digests, or an incompatible rewrite is accepted as a retry — which is the failure that - * loses audit evidence. + * Two things have to hold. Two writes that mean the same must produce the same digest, or a correct + * retry is rejected as an incompatible rewrite. Two writes that name different transitions must + * produce different digests, or an incompatible rewrite is accepted as a retry — which is the + * failure that loses audit evidence. + * + * What "mean the same" covers is the terminal status and the finish time, and nothing else. The + * counts and failures a transition delivers stay outside the digest, which is what lets DICE #69 + * grow the outcome payload without moving a digest already stored beside a run. */ class ExtractionRunFingerprintTest { @@ -46,101 +50,10 @@ class ExtractionRunFingerprintTest { invocation = invocation, ) - // ---- order independence ---- - - @Test - fun `the failure list's order does not move the fingerprint`() { - val one = failure(ExtractionFailureCode.RATE_LIMITED, ExtractionFailureStage.MODEL_CALL) - val two = failure(ExtractionFailureCode.DECODE_FAILED, ExtractionFailureStage.RESPONSE_DECODE) - val three = failure(ExtractionFailureCode.MODEL_TIMEOUT, invocation = ExtractionInvocationId(1, 2)) - - val forwards = ExtractionRunTransition.failed(FINISHED_AT, failures = listOf(one, two, three)) - val backwards = ExtractionRunTransition.failed(FINISHED_AT, failures = listOf(three, two, one)) - val shuffled = ExtractionRunTransition.failed(FINISHED_AT, failures = listOf(two, three, one)) - - assertThat(forwards.fingerprint).isEqualTo(backwards.fingerprint) - assertThat(forwards.fingerprint).isEqualTo(shuffled.fingerprint) - } + // ---- what the digest covers ---- @Test - fun `two failures that differ only in one field still differ`() { - val base = failure( - ExtractionFailureCode.MODEL_TIMEOUT, - ExtractionFailureStage.MODEL_CALL, - FINISHED_AT, - ExtractionInvocationId(1, 2), - ) - val variants = listOf( - "code" to failure( - ExtractionFailureCode.RATE_LIMITED, - ExtractionFailureStage.MODEL_CALL, - FINISHED_AT, - ExtractionInvocationId(1, 2), - ), - "stage" to failure( - ExtractionFailureCode.MODEL_TIMEOUT, - ExtractionFailureStage.RESPONSE_DECODE, - FINISHED_AT, - ExtractionInvocationId(1, 2), - ), - "no stage" to failure( - ExtractionFailureCode.MODEL_TIMEOUT, - null, - FINISHED_AT, - ExtractionInvocationId(1, 2), - ), - "providerStatus" to failure( - ExtractionFailureCode.MODEL_TIMEOUT, - ExtractionFailureStage.MODEL_CALL, - FINISHED_AT, - ExtractionInvocationId(1, 2), - providerStatus = 504, - ), - "measure" to failure( - ExtractionFailureCode.MODEL_TIMEOUT, - ExtractionFailureStage.MODEL_CALL, - FINISHED_AT, - ExtractionInvocationId(1, 2), - measure = ExtractionFailureMeasure(ExtractionFailureQuantity.ELAPSED_MILLIS, 30_000), - ), - "at" to failure( - ExtractionFailureCode.MODEL_TIMEOUT, - ExtractionFailureStage.MODEL_CALL, - FINISHED_AT.plusNanos(1), - ExtractionInvocationId(1, 2), - ), - "invocation index" to failure( - ExtractionFailureCode.MODEL_TIMEOUT, - ExtractionFailureStage.MODEL_CALL, - FINISHED_AT, - ExtractionInvocationId(2, 2), - ), - "attempt" to failure( - ExtractionFailureCode.MODEL_TIMEOUT, - ExtractionFailureStage.MODEL_CALL, - FINISHED_AT, - ExtractionInvocationId(1, 3), - ), - "no invocation" to failure( - ExtractionFailureCode.MODEL_TIMEOUT, - ExtractionFailureStage.MODEL_CALL, - FINISHED_AT, - null, - ), - ) - val baseline = ExtractionRunTransition.failed(FINISHED_AT, failures = listOf(base)).fingerprint - - variants.forEach { (name, variant) -> - assertThat(ExtractionRunTransition.failed(FINISHED_AT, failures = listOf(variant)).fingerprint) - .describedAs("a failure differing in %s", name) - .isNotEqualTo(baseline) - } - } - - // ---- the whole payload ---- - - @Test - fun `changing any component of a terminal write changes its fingerprint`() { + fun `changing the transition's identity changes its fingerprint`() { val baseline = ExtractionRunTransition.completed( finishedAt = FINISHED_AT, counts = ExtractionRunCounts(propositionsExtracted = 3, propositionsPersisted = 3), @@ -152,20 +65,15 @@ class ExtractionRunFingerprintTest { counts = baseline.counts, failures = baseline.failures, ), - "finishedAt" to ExtractionRunTransition.completed( - finishedAt = FINISHED_AT.plusNanos(1), - counts = baseline.counts, - failures = baseline.failures, - ), - "counts" to ExtractionRunTransition.completed( + "the other status" to ExtractionRunTransition.failed( finishedAt = FINISHED_AT, - counts = ExtractionRunCounts(propositionsExtracted = 3, propositionsPersisted = 2), + counts = baseline.counts, failures = baseline.failures, ), - "failures" to ExtractionRunTransition.completed( - finishedAt = FINISHED_AT, + "finishedAt" to ExtractionRunTransition.completed( + finishedAt = FINISHED_AT.plusNanos(1), counts = baseline.counts, - failures = emptyList(), + failures = baseline.failures, ), ) @@ -177,38 +85,59 @@ class ExtractionRunFingerprintTest { } @Test - fun `absent and empty are different claims`() { - // Null means keep what the run recorded and a value means replace it, so the encoding has - // to tell them apart or a store would replay one as the other. - val keptCounts = ExtractionRunTransition.completed(FINISHED_AT, counts = null) - val zeroCounts = ExtractionRunTransition.completed(FINISHED_AT, counts = ExtractionRunCounts()) - assertThat(keptCounts.fingerprint).isNotEqualTo(zeroCounts.fingerprint) - - val keptFailures = ExtractionRunTransition.completed(FINISHED_AT, failures = null) - val noFailures = ExtractionRunTransition.completed(FINISHED_AT, failures = emptyList()) - assertThat(keptFailures.fingerprint).isNotEqualTo(noFailures.fingerprint) - } + fun `the outcome a transition carries never moves its fingerprint`() { + // The property DICE #69 rests on. A run's counts and failures are data the terminal write + // delivers; the digest names which terminal write it is. A payload that grows a field, or + // a retry that carries better numbers, leaves every recorded digest matchable. + val identity = ExtractionRunTransition.completed(FINISHED_AT).fingerprint - @Test - fun `one failure never collides with two that carry the same field values between them`() { - // Length-prefixing and the count prefix are what make this hold. A delimiter-joined - // encoding would render ["a|b"] and ["a", "b"] the same way, and a list with no count - // in front of it lets a shorter list be a prefix of a longer one. - val single = ExtractionRunTransition.failed( - FINISHED_AT, - failures = listOf( - failure(ExtractionFailureCode.INTERNAL, ExtractionFailureStage.CHUNKING, providerStatus = 500), + val carrying = listOf( + "counts" to ExtractionRunTransition.completed( + FINISHED_AT, + counts = ExtractionRunCounts(propositionsExtracted = 3, propositionsPersisted = 3), ), - ) - val split = ExtractionRunTransition.failed( - FINISHED_AT, - failures = listOf( - failure(ExtractionFailureCode.INTERNAL, ExtractionFailureStage.CHUNKING), - failure(ExtractionFailureCode.INTERNAL, providerStatus = 500), + "zero counts" to ExtractionRunTransition.completed(FINISHED_AT, counts = ExtractionRunCounts()), + "no failures" to ExtractionRunTransition.completed(FINISHED_AT, failures = emptyList()), + "one failure" to ExtractionRunTransition.completed( + FINISHED_AT, + failures = listOf(failure(ExtractionFailureCode.RATE_LIMITED, ExtractionFailureStage.MODEL_CALL)), + ), + "several failures" to ExtractionRunTransition.completed( + FINISHED_AT, + counts = ExtractionRunCounts(entitiesResolved = 9), + failures = listOf( + failure(ExtractionFailureCode.INTERNAL, ExtractionFailureStage.CHUNKING), + failure( + ExtractionFailureCode.MODEL_TIMEOUT, + ExtractionFailureStage.MODEL_CALL, + providerStatus = 504, + measure = ExtractionFailureMeasure(ExtractionFailureQuantity.ELAPSED_MILLIS, 30_000), + ), + ), ), ) - assertThat(single.fingerprint).isNotEqualTo(split.fingerprint) + carrying.forEach { (name, transition) -> + assertThat(transition.fingerprint) + .describedAs("a terminal write carrying %s names the same transition", name) + .isEqualTo(identity) + } + } + + @Test + fun `counts and failures still reach the run, even though they stay out of the digest`() { + // The other half of the rule, so "outside the digest" is never read as "dropped". What a + // transition carries is what the terminal run holds; equality on the transition sees it too. + val running = ExtractionRunFixtures.runningRun("run-outcome-data") + val counts = ExtractionRunCounts(propositionsExtracted = 3, propositionsPersisted = 2) + val failures = listOf(failure(ExtractionFailureCode.RATE_LIMITED, ExtractionFailureStage.MODEL_CALL)) + + val terminal = ExtractionRunTransition.completed(FINISHED_AT, counts, failures).applyTo(running) + + assertThat(terminal.counts).isEqualTo(counts) + assertThat(terminal.failures).isEqualTo(failures) + assertThat(ExtractionRunTransition.completed(FINISHED_AT, counts, failures)) + .isNotEqualTo(ExtractionRunTransition.completed(FINISHED_AT)) } @Test @@ -266,7 +195,7 @@ class ExtractionRunFingerprintTest { ) assertThat(pinned.fingerprint) - .isEqualTo("66e8ef75aeb2c1f12b241aebd9f943dc26c654fb18cb4181ed8cecd4994c4309") + .isEqualTo("b32bd0dd33ff0fe21397e0601f91b9f149250af9822d5fce633ad621baa44611") } @Test @@ -279,20 +208,20 @@ class ExtractionRunFingerprintTest { @Test fun `the encoding carries a version tag`() { - // A reader meeting a version it does not know matches nothing rather than guessing. - assertThat(ExtractionRunFingerprint.TERMINAL_VERSION).isEqualTo("xrun-terminal:v1") + // A reader meeting a version it does not know matches nothing at all, with no guessing. The + // tag moved to v2 when the payload narrowed to the transition's identity, so a digest + // recorded under v1 matches nothing written now. + assertThat(ExtractionRunFingerprint.TERMINAL_VERSION).isEqualTo("xrun-terminal:v2") } @Test - fun `the fingerprint is a function of the payload and nothing else`() { + fun `the fingerprint is a function of the transition's identity and nothing else`() { val transition = ExtractionRunTransition.completed(FINISHED_AT) assertThat(transition.fingerprint).isEqualTo( ExtractionRunFingerprint.ofTerminal( status = ExtractionRunStatus.COMPLETED, finishedAt = FINISHED_AT, - counts = null, - failures = null, ), ) } diff --git a/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunLifecycleTest.kt b/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunLifecycleTest.kt index aff17bdb..3b71c587 100644 --- a/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunLifecycleTest.kt +++ b/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunLifecycleTest.kt @@ -15,6 +15,8 @@ */ package com.embabel.dice.proposition.extraction +import com.embabel.dice.common.DiceEvent +import com.embabel.dice.common.ExtractionRunTransitioned import com.embabel.dice.proposition.extraction.ExtractionRunFixtures.CONTEXT import com.embabel.dice.proposition.extraction.ExtractionRunFixtures.FINISHED_AT import com.embabel.dice.proposition.extraction.ExtractionRunFixtures.OTHER_CONTEXT @@ -172,17 +174,14 @@ class ExtractionRunLifecycleTest { @Test fun `an incompatible terminal rewrite is rejected rather than overwriting`() { + // Incompatible means a write naming a different transition: another terminal status, or + // another finish time. What the write carries is data it delivers and is decided by + // `a retry carrying different counts and failures replays` below. val incompatible = listOf( "a different status" to ExtractionRunTransition.failed(FINISHED_AT), + "the third status" to ExtractionRunTransition.cancelled(FINISHED_AT), "a different finish time" to ExtractionRunTransition.completed(FINISHED_AT.plusSeconds(1)), - "different counts" to ExtractionRunTransition.completed( - finishedAt = FINISHED_AT, - counts = ExtractionRunCounts(propositionsPersisted = 12), - ), - "a different failure list" to ExtractionRunTransition.completed( - finishedAt = FINISHED_AT, - failures = listOf(ExtractionFailure(ExtractionFailureCode.RATE_LIMITED)), - ), + "a finish time a nanosecond later" to ExtractionRunTransition.completed(FINISHED_AT.plusNanos(1)), ) val recorded = ExtractionRunTransition.completed( finishedAt = FINISHED_AT, @@ -205,26 +204,66 @@ class ExtractionRunLifecycleTest { } @Test - fun `keeping counts and replacing them with the same values are the same terminal write`() { + fun `a retry carrying different counts and failures replays, and the first write's outcome stands`() { val run = ExtractionRunFixtures.runningRun( lineage = ExtractionRunLineage.root(ExtractionRunRef("run-counts")), counts = ExtractionRunCounts(propositionsPersisted = 4), ) store.save(run) - store.transition(run.key(), ExtractionRunTransition.completed(FINISHED_AT, counts = null)) + val landed = store.transition( + run.key(), + ExtractionRunTransition.completed(FINISHED_AT, counts = null), + ).run - // Null means keep and a value means replace, so they are different claims even when they - // land on the same numbers. Rejecting the second is the honest answer: the store cannot - // tell a caller who meant "leave them" from one who meant "these are final". - assertThatThrownBy { - store.transition( - run.key(), - ExtractionRunTransition.completed( - FINISHED_AT, - counts = ExtractionRunCounts(propositionsPersisted = 4), - ), - ) - }.isInstanceOf(ExtractionRunConflictException::class.java) + // The digest names the transition — this status, this finish time — so a second write + // agreeing on both is the same terminal write however different its numbers. A run's + // outcome is written once, and the retry gets back what already landed. + val retry = store.transition( + run.key(), + ExtractionRunTransition.completed( + FINISHED_AT, + counts = ExtractionRunCounts(propositionsPersisted = 99), + failures = listOf(ExtractionFailure(ExtractionFailureCode.RATE_LIMITED)), + ), + ) + + assertThat(retry.outcome).isEqualTo(ExtractionRunTransitionOutcome.REPLAYED) + assertThat(retry.run).isEqualTo(landed) + assertThat(store.findRun(run.key())?.counts?.propositionsPersisted).isEqualTo(4) + assertThat(store.findRun(run.key())?.failures).isEmpty() + } + + // ---- announcing a run that ended ---- + + @Test + fun `the call that ends a run announces it once, and a replay announces nothing`() { + val received = mutableListOf() + val listening = InMemoryExtractionRunStore { event -> received += event } + val run = started("run-announce") + listening.save(run) + listening.recordInvocation(run.key(), ExtractionInvocationRecord.planned(0)) + assertThat(received).describedAs("no write before the terminal one announces anything").isEmpty() + + val transition = ExtractionRunTransition.completed(FINISHED_AT) + val applied = listening.transition(run.key(), transition) + + assertThat(received).containsExactly(ExtractionRunTransitioned(applied.run, received.single().timestamp)) + + listening.transition(run.key(), transition) + listening.transition(run.key(), transition) + assertThat(received).describedAs("a replay changed nothing, so it announces nothing").hasSize(1) + } + + @Test + fun `a store with nothing listening ends runs the same way`() { + // The listener has a no-op default, so a host that never wired one up constructs the store + // as it always did and every lifecycle rule above still holds. + val silent = InMemoryExtractionRunStore() + val run = started("run-no-listener") + silent.save(run) + + assertThat(silent.transition(run.key(), ExtractionRunTransition.completed(FINISHED_AT)).outcome) + .isEqualTo(ExtractionRunTransitionOutcome.APPLIED) } // ---- nothing reaches COMPLETED except through the transition ---- diff --git a/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunValueTypesTest.kt b/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunValueTypesTest.kt index 27e88a99..c0bdb34f 100644 --- a/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunValueTypesTest.kt +++ b/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunValueTypesTest.kt @@ -292,6 +292,7 @@ class ExtractionRunValueTypesTest { ExtractionRunFingerprint::class.java, ExtractionRunNotFoundException::class.java, ExtractionRunConflictException::class.java, + com.embabel.dice.common.ExtractionRunTransitioned::class.java, ).forEach { type -> assertThat(isMarkedExperimental(type)) .describedAs("%s is marked experimental", type.simpleName) diff --git a/docs/design/extraction-runs.md b/docs/design/extraction-runs.md index e03d1747..863c0fa4 100644 --- a/docs/design/extraction-runs.md +++ b/docs/design/extraction-runs.md @@ -369,9 +369,11 @@ version included` and `null counts and failures on a transition keep what the st values replace them`. `counts` and `failures` are nullable and follow one rule: null keeps what the run recorded, a value -replaces it. An empty failure list is a value. That distinction reaches the fingerprint, so -"leave the counts alone" and "these counts are final" are two different terminal writes even when -they land on the same numbers. +replaces it. An empty failure list is a value. The distinction decides what the terminal run holds; +it stays out of the fingerprint, which names the transition and leaves the outcome it delivers to the +run. See +["The digest covers the transition's identity"](#the-digest-covers-the-transitions-identity-and-the-outcome-rides-beside-it) +below. Invocation records do not travel on a transition. They arrive through `recordInvocation` while the run is still running, keyed by `(invocationIndex, attempt)`, and a terminal run takes no more — @@ -449,7 +451,7 @@ header generation. ### Idempotency: insert-or-compare, never overwrite -Every terminal write carries a fingerprint of its payload. A store records the fingerprint of the +Every terminal write carries a fingerprint of its identity. A store records the fingerprint of the write that terminalized a run, and a second write against that run is decided by comparison: | Second write | Result | @@ -468,6 +470,42 @@ attempt between a terminal write it never saw the answer to and its retry made t write both times, and folding the run's invocation list in would turn that correct retry into a rejected conflict. +#### The digest covers the transition's identity, and the outcome rides beside it + +Two fields reach the digest: the terminal status, and the finish time. Two writes that agree on both +are the same terminal write. + +The counts and failures a transition carries stay outside it. They are the outcome a run reports, +and a run's outcome is written once — the first accepted terminal write is what the audit keeps, and +a retry naming the same status and finish time replays against it whatever numbers it carries. A +coordinator holding better numbers than the ones that landed records them before it ends the run; +after the run has ended there is nothing left to correct, which is the same promise `transition` +makes about everything else on a finished run. + +This is what keeps the persisted format still. DICE #69 adds typed product outcomes to what a +terminal write reports, and every field it adds lands in the counts-and-failures half. None of them +reaches these bytes, so no digest recorded beside a run stops matching and no migration follows #69 +into this store. + +A worked contrast, since the two halves are easy to conflate. `counts = null` means keep what the +run recorded and `counts = ` means replace it, and those really are different claims — the +terminal run they produce holds different numbers. What they are not is different *writes*: both +name the same status and the same finish time, so whichever arrives second replays. The cross-backend +suite pins both halves in `keeping counts and replacing them are different claims on the run that +lands, and the digest sees neither`, and pins the outcome-once rule in `a retry carrying different +counts and failures replays, and the first write's outcome stands`. + +An earlier version of this rule folded counts and failures into the digest. It made every difference +in the payload an incompatible rewrite, which reads as safe and buys nothing an audit wants: the +first write had already landed and the second was rejected either way, so the only thing the wider +digest changed was whether the caller learned about it as a conflict or as a replay. What it cost +was the persisted format, which moved whenever the outcome payload grew a field. + +`TERMINAL_VERSION` moved from `xrun-terminal:v1` to `xrun-terminal:v2` when the payload narrowed. +The tag is part of the hashed input, so a digest recorded under `v1` matches nothing written now. +Nothing durable holds one — no store outside the in-memory reference has written a run yet — so the +bump costs nothing and the version is what would have made the migration statable if it had. + `save` is the one method that updates in place, and only on a run that is still running. Even there it is fenced four ways: @@ -547,7 +585,28 @@ key and their own compare-and-set, entirely off the header's generation. Run sta the way lineage systems such as OpenLineage model a run: independent writers contribute rows, and no write rewrites a row another writer owns. -**Replay needs the identical payload, finish time included.** A coordinator retrying after a crash +#### A run that ends announces itself once + +`transition` hands an `ExtractionRunTransitioned` to the store's listener for the call that ended +the run, carrying the run in its terminal state. A replay announces nothing, and so does a rejected +write. A coordinator retrying a terminal write whose answer it never saw would otherwise notify +every downstream consumer a second time for a run that ended once — which is the whole reason +`REPLAYED` is a distinct outcome at all. + +The listener arrives the way every other DICE listener does: a constructor collaborator defaulting +to `DiceEventListener.DEV_NULL`, the same shape `EventEmittingPropositionRepository` uses. Nothing +is wired automatically. A host that has nothing listening builds the store exactly as it did before, +and a host that wants the signal passes a listener in. Handlers run inline on the calling thread and +throw isolation belongs to the listener, so a host that needs graceful degradation wraps its +listener in `SafeDiceEventListener`. + +The announcement happens after the write has landed and outside the store's own lock, so a listener +that blocks, or that reads the run back, holds up no other writer and sees the terminal run already +committed. The cross-backend suite pins the count directly — one announcement per applied +transition, none for a replay, none for a rejection, none for a `save` or a `recordInvocation`, and +one between eight threads racing to end the same run. + +**Replay needs the identical finish time.** A coordinator retrying after a crash it never saw the answer to must reuse the transition it built the first time, or read the run back with `findRun` and stop if it has already ended. Minting a fresh `finishedAt` on the retry produces a different fingerprint, which is an incompatible rewrite and is rejected. That is safe and it is @@ -569,17 +628,11 @@ payload: - every token is length-prefixed, `:`, because a delimiter-joined encoding lets `["a;b"]` and `["a", "b"]` hash the same, and a length prefix keeps two tokens apart whatever characters either one carries; -- every collection is preceded by its element count, so a shorter list cannot be a prefix of a +- a field set is preceded by how many fields it holds, so a shorter one cannot be a prefix of a longer one; - fields are emitted as `(name, value)` pairs sorted by name; -- a collection whose order carries no meaning is sorted, which is what makes two coordinators - recording the same failures in a different order the same terminal write — the order is Kotlin's - natural `String` order, comparing UTF-16 code units, not UTF-8 byte order and not a locale - collation, and it is named because a backend re-implementing the sort in a query would pick a - different one and get a different digest; - instants render as `.` — fixed width, and independent of `java.time`'s own formatting, whose precision varies with the value; -- absent is its own marker, so null and empty stay distinguishable; - SHA-256, lowercase hex, with a version tag on the input so a reader meeting a version it does not know matches nothing rather than guessing. From 75d9fd01cf1a43e62f855ca73ca65a794b22ef98 Mon Sep 17 00:00:00 2001 From: James Dunnam <7660553+jimador@users.noreply.github.com> Date: Wed, 2 Sep 2026 02:14:10 -0400 Subject: [PATCH 3/7] State the protected-content reference as it now stands The absent-by-decision list still said no reference type exists, while the sections above it describe the shipped specification. The bullet now records both facts: the first cut was removed for having no runtime path, and the interface returned as a written contract the host implements. Signed-off-by: James Dunnam <7660553+jimador@users.noreply.github.com> --- docs/design/extraction-runs.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/design/extraction-runs.md b/docs/design/extraction-runs.md index 863c0fa4..dfb724f8 100644 --- a/docs/design/extraction-runs.md +++ b/docs/design/extraction-runs.md @@ -833,12 +833,12 @@ public surface. - **No proposition-to-run relation.** Attribution from a claim to the runs that produced or confirmed it is its own slice, on canonical saved ids, and run identity stays out of source-provenance equality. -- **No protected-content reference type.** A first cut (`ProtectedContentRef`, +- **Protected-content reference: specification only.** A first cut (`ProtectedContentRef`, `ProtectedContentClassification`, `ProtectedContentHandle`) landed and was removed again: nothing - in DICE attached one to an `ExtractionRun`, read one, or enforced its retention, so it was a shape - with no runtime path exercising it. It returns with the first runtime path that needs it — a - writer, a reader, or retention behaviour. A value type with no consumer does not stay on the - branch. + in DICE attached one to an `ExtractionRun`, read one, or enforced its retention. The interface + returned as a written contract — an opaque `handle` and an `expiresAt`, with the host owning + writer, reader and retention. DICE stores none of its content, and the first runtime path that + needs the reference brings its implementation. - **No per-invocation requested configuration.** The requested configuration is one record on the run header. A later slice that needs to vary settings per call adds a separate requested record keyed by invocation index rather than a field on the observed record, which would collapse the From aac4f38b0be797b0d09a2fd5dc10ca3bb29edd02 Mon Sep 17 00:00:00 2001 From: James Dunnam <7660553+jimador@users.noreply.github.com> Date: Wed, 2 Sep 2026 16:33:02 -0400 Subject: [PATCH 4/7] State what a saved run's version is after each kind of save The KDoc on ExtractionRunStore.save said a first save returned the version the caller named plus one. It returns 0: the version the caller named, now confirmed by the store. An accepted update returns the stored version plus one, and a replay returns the stored version unchanged. The contract test now pins all three. Signed-off-by: James Dunnam <7660553+jimador@users.noreply.github.com> --- .../AbstractExtractionRunStoreContractTest.kt | 36 +++++++++++++++++++ .../extraction/ExtractionRunStore.kt | 5 +-- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/dice-storage/src/test/kotlin/com/embabel/dice/storage/AbstractExtractionRunStoreContractTest.kt b/dice-storage/src/test/kotlin/com/embabel/dice/storage/AbstractExtractionRunStoreContractTest.kt index 68643135..d57c9c74 100644 --- a/dice-storage/src/test/kotlin/com/embabel/dice/storage/AbstractExtractionRunStoreContractTest.kt +++ b/dice-storage/src/test/kotlin/com/embabel/dice/storage/AbstractExtractionRunStoreContractTest.kt @@ -930,6 +930,42 @@ abstract class AbstractExtractionRunStoreContractTest { assertEquals("service-current", record.configuredService) } + @Test + fun `a first save lands at version 0, an accepted update raises it by one, and a replay leaves it alone`() { + // The version a save returns is the store's word on what is now stored: a first save names 0 + // and gets 0 back, a header change accepted at the stored version gets that version plus + // one, and a save whose content already matches keeps whatever is stored. + val store = store() + val run = running("contract-version-progression") + val inserted = store.save(run) + assertEquals(0L, inserted.version) + assertEquals(0L, store.findRun(run.key())?.version) + + val changed = ExtractionRun( + contextId = tenant, + lineage = run.lineage, + status = ExtractionRunStatus.RUNNING, + startedAt = startedAt, + counts = ExtractionRunCounts(propositionsPersisted = 3), + version = 0, + ) + val updated = store.save(changed) + assertEquals(1L, updated.version) + assertEquals(1L, store.findRun(run.key())?.version) + + val replay = ExtractionRun( + contextId = tenant, + lineage = run.lineage, + status = ExtractionRunStatus.RUNNING, + startedAt = startedAt, + counts = ExtractionRunCounts(propositionsPersisted = 3), + version = 1, + ) + val replayed = store.save(replay) + assertEquals(1L, replayed.version) + assertEquals(1L, store.findRun(run.key())?.version) + } + @Test fun `a stale header save is rejected, and the header it read is left in place`() { // Writer A reads the run at version 0, does real work, and saves what it found. That save diff --git a/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunStore.kt b/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunStore.kt index 2a9286e3..eec51607 100644 --- a/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunStore.kt +++ b/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunStore.kt @@ -189,8 +189,9 @@ interface ExtractionRunStore { * @param run The run to record. Must be [ExtractionRunStatus.RUNNING] and carry no * [ExtractionRun.finishedAt]. * @return The stored run, including any invocation records already recorded against it through - * [recordInvocation]. Its [ExtractionRun.version] is one higher than [run]'s when the header - * changed, and unchanged when the save was a no-op. + * [recordInvocation]. Its [ExtractionRun.version] is `0` after a first save (the version the + * caller named, now confirmed by the store), the stored version plus one after an accepted + * update, and the stored version unchanged when the save replayed as a no-op. * @throws IllegalArgumentException if [run] is not running, if it carries a finish time, or if * it is a first save naming a version other than `0`. A terminal status reaches the store only * through [transition], which is what keeps `COMPLETED` behind its precondition; a running run From 74eb6c7b189b2c74a2731fd07b2e574963f0342f Mon Sep 17 00:00:00 2001 From: James Dunnam <7660553+jimador@users.noreply.github.com> Date: Wed, 9 Sep 2026 17:13:30 -0400 Subject: [PATCH 5/7] Cap what the reference run store keeps InMemoryExtractionRunStore kept every run and every terminal fingerprint forever, which is unbounded retention for a store meant as a reference and sometimes a bridge before a host has a database. Add a maxRuns constructor parameter, defaulting to 10,000, as the last parameter so the existing Java descriptors survive. When an insert would push the store past the cap, evict the oldest ended runs by startedAt until it fits again, dropping each evicted run's terminal fingerprint with it. A run still RUNNING is never evicted, so a store where every run happens to be running can grow past the cap; that breach logs once at warn, not on every insert. Say plainly in the class KDoc, and in the ExtractionRunStore contract KDoc, that retention past the cap is the reference store's own policy, not something the contract promises, and that a host running it in production is accepting that runs older than the cap are gone. Signed-off-by: James Dunnam <7660553+jimador@users.noreply.github.com> --- .../extraction/ExtractionRunStore.kt | 4 ++ .../extraction/InMemoryExtractionRunStore.kt | 61 ++++++++++++++++++- .../extraction/ExtractionRunLifecycleTest.kt | 47 +++++++++++++- 3 files changed, 108 insertions(+), 4 deletions(-) diff --git a/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunStore.kt b/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunStore.kt index eec51607..9541f59e 100644 --- a/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunStore.kt +++ b/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunStore.kt @@ -102,6 +102,10 @@ import java.time.Instant * takes a positive `limit`, and the reads that can span a long history also take an optional * `since` window. * + * How long a run stays readable is the store's own policy, not this contract's: this contract + * says nothing about retention, and the reference implementation caps how many runs it keeps and + * forgets the oldest ended ones past that cap. + * * **Scope is pushed down, never applied afterwards.** An implementation must restrict to the tenant * inside the query and then limit. Fetching `limit` rows and filtering them by tenant afterwards * would return fewer rows than asked for — or none — whenever a busy neighbouring tenant occupies diff --git a/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/InMemoryExtractionRunStore.kt b/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/InMemoryExtractionRunStore.kt index 2ee3437c..f230246b 100644 --- a/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/InMemoryExtractionRunStore.kt +++ b/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/InMemoryExtractionRunStore.kt @@ -18,6 +18,7 @@ package com.embabel.dice.proposition.extraction import com.embabel.dice.common.DiceEventListener import com.embabel.dice.common.ExtractionRunTransitioned import org.jetbrains.annotations.ApiStatus +import org.slf4j.LoggerFactory import java.time.Instant /** @@ -62,18 +63,39 @@ import java.time.Instant * * Nothing here survives the JVM, and two instances know nothing about each other. * + * **This is the reference implementation, and it forgets.** It holds at most [maxRuns] runs, and + * when a new run would push it past that cap it evicts the oldest ended runs, by + * [ExtractionRun.startedAt], until it fits again. A run still `RUNNING` is never evicted for the + * cap, so a store where every stored run happens to be running can grow past [maxRuns]; when that + * happens it says so with a single `warn` log for the breach, not one per insert. A host running + * this store in production is accepting that a run older than the cap is gone for good: it cannot + * be found, paged, or walked as an ancestor once evicted. Retention is a real, durable policy on a + * database-backed store, kept for as long as an operator decides; this store exists so a host can + * record and read runs before it has one of those, and forgetting the oldest is the cost of holding + * every run in one JVM's memory. + * * EXPERIMENTAL. The shape may still change while extraction runs (DICE #67) land. * * @property listener Notified when a run ends. Defaults to [DiceEventListener.DEV_NULL], so a host - * that has nothing listening constructs the store the same way it always did. Handlers run inline - * on the calling thread, and throw isolation belongs to the listener — wrap it in - * `SafeDiceEventListener` for graceful degradation. + * that has nothing listening constructs the store the same way it always did. Handlers run + * inline on the calling thread; a listener that throws does not undo the write that already + * landed, because the store catches and logs it and still reports the transition as committed. + * @property maxRuns The most runs this store holds before it starts forgetting the oldest ended + * ones. Must be positive. The default, 10,000, is enough for a host trying the reference store + * out or bridging a gap before it wires up a durable one. */ @ApiStatus.Experimental class InMemoryExtractionRunStore @JvmOverloads constructor( private val listener: DiceEventListener = DiceEventListener.DEV_NULL, + private val maxRuns: Int = 10_000, ) : ExtractionRunStore { + init { + require(maxRuns > 0) { "maxRuns must be positive, was $maxRuns" } + } + + private val logger = LoggerFactory.getLogger(InMemoryExtractionRunStore::class.java) + private val lock = Any() private val runs = LinkedHashMap() @@ -81,6 +103,10 @@ class InMemoryExtractionRunStore @JvmOverloads constructor( /** The fingerprint of the terminal write that ended each run, for comparing a retry against. */ private val terminalWrites = HashMap() + /** Set once the store has grown past [maxRuns] with nothing left to evict, so the breach logs + * once, not on every insert while it lasts. Cleared once the store fits again. */ + private var overCapacityWarned = false + override fun save(run: ExtractionRun): ExtractionRun { require(run.status == ExtractionRunStatus.RUNNING) { "save records a running run; ${run.status} is terminal and belongs to transition()" @@ -105,6 +131,7 @@ class InMemoryExtractionRunStore @JvmOverloads constructor( // door never originates a row. val inserted = rebuild(run, invocations = emptyList(), version = 0L) runs[key] = inserted + evictOverflow() return inserted } if (stored.status.isTerminal) { @@ -274,6 +301,34 @@ class InMemoryExtractionRunStore @JvmOverloads constructor( require(limit > 0) { "limit must be positive, was $limit" } } + /** + * Evicts the oldest ended runs, by [ExtractionRun.startedAt], until the store fits under + * [maxRuns] again. Called from inside the monitor an insert already holds, so it never takes + * the lock itself. A run still `RUNNING` is never a candidate: if evicting every ended run + * still leaves the store over the cap, it logs the breach once and stops. + */ + private fun evictOverflow() { + while (runs.size > maxRuns) { + val oldest = runs.values.filter { it.status.isTerminal }.minByOrNull { it.startedAt } + if (oldest == null) { + if (!overCapacityWarned) { + overCapacityWarned = true + logger.warn( + "InMemoryExtractionRunStore holds {} runs, over its cap of {}, and every " + + "one of them is still running, so none can be evicted", + runs.size, + maxRuns, + ) + } + return + } + val evictedKey = oldest.key() + runs.remove(evictedKey) + terminalWrites.remove(evictedKey) + } + overCapacityWarned = false + } + /** Re-lists the run with its invocations and version replaced, since [ExtractionRun] publishes * no `copy`. */ private fun rebuild( diff --git a/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunLifecycleTest.kt b/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunLifecycleTest.kt index 3b71c587..51f4606a 100644 --- a/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunLifecycleTest.kt +++ b/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunLifecycleTest.kt @@ -238,7 +238,7 @@ class ExtractionRunLifecycleTest { @Test fun `the call that ends a run announces it once, and a replay announces nothing`() { val received = mutableListOf() - val listening = InMemoryExtractionRunStore { event -> received += event } + val listening = InMemoryExtractionRunStore(listener = { event -> received += event }) val run = started("run-announce") listening.save(run) listening.recordInvocation(run.key(), ExtractionInvocationRecord.planned(0)) @@ -635,4 +635,49 @@ class ExtractionRunLifecycleTest { assertThat(store.transition(theirs.key(), ExtractionRunTransition.failed(FINISHED_AT)).outcome) .isEqualTo(ExtractionRunTransitionOutcome.APPLIED) } + + // ---- retention cap ---- + + @Test + fun `the reference store forgets the oldest ended runs past its cap`() { + val capped = InMemoryExtractionRunStore(maxRuns = 3) + val ended = (0..2).map { index -> + val run = ExtractionRunFixtures.runningRun( + "run-cap-$index", + CONTEXT, + STARTED_AT.plusSeconds(index.toLong()), + ) + capped.save(run) + capped.transition(run.key(), ExtractionRunTransition.completed(FINISHED_AT)) + run + } + + // A fourth run pushes the store to four, one over the cap of three, so the oldest ended + // run is the one forgotten. + val newest = ExtractionRunFixtures.runningRun("run-cap-newest", CONTEXT, STARTED_AT.plusSeconds(10)) + capped.save(newest) + + assertThat(capped.findRun(ended[0].key())).describedAs("the oldest ended run").isNull() + assertThat(capped.findRun(ended[1].key())).isNotNull() + assertThat(capped.findRun(ended[2].key())).isNotNull() + assertThat(capped.findRun(newest.key())).isNotNull() + } + + @Test + fun `running runs are never evicted for the cap`() { + val capped = InMemoryExtractionRunStore(maxRuns = 2) + val running = (0..2).map { index -> + val run = ExtractionRunFixtures.runningRun( + "run-running-$index", + CONTEXT, + STARTED_AT.plusSeconds(index.toLong()), + ) + capped.save(run) + run + } + + // Every stored run is still RUNNING, so the store has nothing to evict: it grows past its + // cap of two, and none of them is forgotten. + running.forEach { assertThat(capped.findRun(it.key())).describedAs(it.ref.runId).isNotNull() } + } } From 83736343ee28376969050c3acbfa52807e080146 Mon Sep 17 00:00:00 2001 From: James Dunnam <7660553+jimador@users.noreply.github.com> Date: Wed, 9 Sep 2026 17:15:16 -0400 Subject: [PATCH 6/7] Read a tenant's runs from the tenant's own index page() filtered and sorted runs.values while holding the monitor, so every scoped read was O(all tenants' runs), and a busy neighbour tenant paid the cost of every other tenant's page too. Keep a per-tenant index, contextId to that tenant's own run keys in insertion order, maintained in exactly two places: an insert adds a key, and eviction removes one. A run's value is always looked up fresh in the main map, so nothing has to touch the index when a header is saved again, a run transitions, or an invocation is recorded against it. runsInContext, childrenOf and runsOfRoot now iterate only their own tenant's candidates before filtering, ordering and limiting; the rule that scope comes before the limit is unchanged, only the candidate set a page starts from shrinks. Signed-off-by: James Dunnam <7660553+jimador@users.noreply.github.com> --- .../extraction/InMemoryExtractionRunStore.kt | 37 ++++++++++++++++--- .../extraction/ExtractionRunStoreReadsTest.kt | 34 +++++++++++++++++ 2 files changed, 65 insertions(+), 6 deletions(-) diff --git a/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/InMemoryExtractionRunStore.kt b/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/InMemoryExtractionRunStore.kt index f230246b..04616e28 100644 --- a/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/InMemoryExtractionRunStore.kt +++ b/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/InMemoryExtractionRunStore.kt @@ -103,6 +103,14 @@ class InMemoryExtractionRunStore @JvmOverloads constructor( /** The fingerprint of the terminal write that ended each run, for comparing a retry against. */ private val terminalWrites = HashMap() + /** + * Every tenant's own keys, in insertion order, so a scoped read never has to walk another + * tenant's runs to find its own. Maintained in exactly two places: an insert adds a key here, + * and eviction removes one. A key's value is always looked up fresh in [runs], so nothing here + * needs to change when a run already stored is updated, transitioned or recorded against. + */ + private val runsByContext = HashMap>() + /** Set once the store has grown past [maxRuns] with nothing left to evict, so the breach logs * once, not on every insert while it lasts. Cleared once the store fits again. */ private var overCapacityWarned = false @@ -131,6 +139,7 @@ class InMemoryExtractionRunStore @JvmOverloads constructor( // door never originates a row. val inserted = rebuild(run, invocations = emptyList(), version = 0L) runs[key] = inserted + runsByContext.getOrPut(key.contextId.value) { LinkedHashSet() }.add(key) evictOverflow() return inserted } @@ -239,7 +248,7 @@ class InMemoryExtractionRunStore @JvmOverloads constructor( contextIdValue: String, limit: Int, since: Instant?, - ): List = page(limit) { run -> + ): List = page(contextIdValue, limit) { run -> run.contextId.value == contextIdValue && startedAtOrAfter(run, since) } @@ -247,7 +256,7 @@ class InMemoryExtractionRunStore @JvmOverloads constructor( contextIdValue: String, parentRunId: String, limit: Int, - ): List = page(limit) { run -> + ): List = page(contextIdValue, limit) { run -> run.contextId.value == contextIdValue && run.parentRef?.runId == parentRunId } @@ -256,7 +265,7 @@ class InMemoryExtractionRunStore @JvmOverloads constructor( rootRunId: String, limit: Int, since: Instant?, - ): List = page(limit) { run -> + ): List = page(contextIdValue, limit) { run -> run.contextId.value == contextIdValue && run.rootRef.runId == rootRunId && startedAtOrAfter(run, since) @@ -282,18 +291,29 @@ class InMemoryExtractionRunStore @JvmOverloads constructor( /** * Filter, then order, then limit — in that order, because a page that limited first would drop - * a tenant's runs behind a busier neighbour's and report the shortfall as an empty tenant. + * a tenant's runs behind a busier neighbour's and report the shortfall as an empty tenant. The + * candidates themselves come only from [contextIdValue]'s own index, so a busy neighbouring + * tenant is never even looked at, let alone scanned. */ - private fun page(limit: Int, matches: (ExtractionRun) -> Boolean): List { + private fun page( + contextIdValue: String, + limit: Int, + matches: (ExtractionRun) -> Boolean, + ): List { requirePositiveLimit(limit) return synchronized(lock) { - runs.values + candidatesInContext(contextIdValue) .filter(matches) .sortedWith(NEWEST_FIRST) .take(limit) } } + /** The runs stored under [contextIdValue], read fresh from [runs] so a value updated since + * insert (a header save, a transition, a recorded invocation) is never stale here. */ + private fun candidatesInContext(contextIdValue: String): List = + runsByContext[contextIdValue]?.mapNotNull { runs[it] }.orEmpty() + private fun startedAtOrAfter(run: ExtractionRun, since: Instant?): Boolean = since == null || !run.startedAt.isBefore(since) @@ -325,6 +345,11 @@ class InMemoryExtractionRunStore @JvmOverloads constructor( val evictedKey = oldest.key() runs.remove(evictedKey) terminalWrites.remove(evictedKey) + val tenantIndex = runsByContext[evictedKey.contextId.value] + tenantIndex?.remove(evictedKey) + if (tenantIndex?.isEmpty() == true) { + runsByContext.remove(evictedKey.contextId.value) + } } overCapacityWarned = false } diff --git a/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunStoreReadsTest.kt b/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunStoreReadsTest.kt index 74c7cce0..61854c90 100644 --- a/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunStoreReadsTest.kt +++ b/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunStoreReadsTest.kt @@ -16,6 +16,7 @@ package com.embabel.dice.proposition.extraction import com.embabel.dice.proposition.extraction.ExtractionRunFixtures.CONTEXT +import com.embabel.dice.proposition.extraction.ExtractionRunFixtures.FINISHED_AT import com.embabel.dice.proposition.extraction.ExtractionRunFixtures.OTHER_CONTEXT import com.embabel.dice.proposition.extraction.ExtractionRunFixtures.STARTED_AT import org.assertj.core.api.Assertions.assertThat @@ -55,6 +56,18 @@ class ExtractionRunStoreReadsTest { assertThat(page).allSatisfy { assertThat(it.contextId).isEqualTo(CONTEXT) } } + @Test + fun `a busy neighbour tenant does not change what a page returns`() { + (1..20).forEach { store.save(run("neighbour-$it", OTHER_CONTEXT, it.toLong())) } + (1..3).forEach { store.save(run("mine-$it", CONTEXT, it.toLong())) } + + // The limit is well above this tenant's own count, so nothing here turns on the limit + // cutting the neighbour's runs off first: only the candidate set a page reads from. + val page = store.runsInContext(CONTEXT, limit = 10, since = null) + + assertThat(page.map { it.ref.runId }).containsExactly("mine-3", "mine-2", "mine-1") + } + @Test fun `the lineage reads scope before they limit too`() { val root = ExtractionRunRef("root-1") @@ -272,6 +285,27 @@ class ExtractionRunStoreReadsTest { assertThat(store.runsInContext(OTHER_CONTEXT, 10, null)).containsExactly(theirs) } + // ---- retention cap ---- + + @Test + fun `eviction removes a run from its tenant's reads`() { + val capped = InMemoryExtractionRunStore(maxRuns = 2) + val first = ExtractionRunFixtures.runningRun("run-evict-1", CONTEXT, STARTED_AT) + capped.save(first) + capped.transition(first.key(), ExtractionRunTransition.completed(FINISHED_AT)) + val second = ExtractionRunFixtures.runningRun("run-evict-2", CONTEXT, STARTED_AT.plusSeconds(1)) + capped.save(second) + capped.transition(second.key(), ExtractionRunTransition.completed(FINISHED_AT)) + + // A third insert pushes the store past its cap of two, evicting run-evict-1, the oldest + // ended run, from the tenant index along with the run itself. + val third = ExtractionRunFixtures.runningRun("run-evict-3", CONTEXT, STARTED_AT.plusSeconds(2)) + capped.save(third) + + assertThat(capped.runsInContext(CONTEXT, limit = 10, since = null).map { it.ref.runId }) + .containsExactly("run-evict-3", "run-evict-2") + } + // ---- invocation records ---- @Test From 68c5f8a8aa9b541cbf8bbb8911e5eeac289f062b Mon Sep 17 00:00:00 2001 From: James Dunnam <7660553+jimador@users.noreply.github.com> Date: Wed, 9 Sep 2026 17:18:03 -0400 Subject: [PATCH 7/7] Keep a committed transition committed when the listener throws transition announced the terminal run outside the monitor and let the listener's exception reach the caller after the write had landed. A caller retrying on that exception got REPLAYED and no event either time. The announcement is now caught: the failure is logged at error with the run's key and the result is returned as applied, because the store's job ended when the write did. The design note and the changelog say so. Signed-off-by: James Dunnam <7660553+jimador@users.noreply.github.com> --- CHANGELOG.md | 22 +++++++++++++++++++ .../extraction/InMemoryExtractionRunStore.kt | 10 ++++++++- .../extraction/ExtractionRunLifecycleTest.kt | 17 ++++++++++++++ docs/design/extraction-runs.md | 22 +++++++++++++++---- 4 files changed, 66 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e4142903..6614123a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1602,3 +1602,25 @@ and the consumer PRs that deliver it). `ExtractionRunTransitioned` is new and carries `@ApiStatus.Experimental` like every other type in this train, added to the same class-file assertion. Design note: [docs/design/extraction-runs.md](docs/design/extraction-runs.md). + +- **PR #98 review round 2: the reference store now bounds itself.** Three findings from that + review, closed in `InMemoryExtractionRunStore`. A host running the reference store in production + is accepting all three: the store now forgets old runs, a busy tenant no longer changes how fast + another tenant's page comes back, and a failing listener no longer looks like a failed transition. + **The cap.** `InMemoryExtractionRunStore` kept every run and every terminal fingerprint forever. + A new `maxRuns` constructor parameter, defaulting to 10,000 and added last so `@JvmOverloads` + keeps the existing Java descriptors, evicts the oldest ended runs by `startedAt` once an insert + would push the store over it. A run still `RUNNING` is never evicted; a store where every run + happens to be running can grow past the cap, and that logs once at `warn`, not on every insert. + **The tenant index.** `page()` filtered and sorted every tenant's runs while holding the monitor, + so a scoped read cost was proportional to the whole store, not to the tenant asking. A per-tenant + index of run keys, maintained on insert and eviction, means `runsInContext`, `childrenOf` and + `runsOfRoot` now only ever look at their own tenant's runs; the rule that scope is applied before + the limit is unchanged. **The listener.** `transition` let a throwing listener's exception reach + the caller after the terminal write had already landed, so a caller retrying on that exception saw + `REPLAYED` and no event for either attempt. The announcement is now wrapped in a catch that logs + the failure at `error` with the run's key and still returns the result as applied. + **Compatibility: additive.** All three land through a single new constructor parameter with a + default; every existing call site, Kotlin or Java, keeps compiling and keeps its prior behavior + short of the fixes themselves. Design note: + [docs/design/extraction-runs.md](docs/design/extraction-runs.md). diff --git a/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/InMemoryExtractionRunStore.kt b/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/InMemoryExtractionRunStore.kt index 04616e28..014ab86a 100644 --- a/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/InMemoryExtractionRunStore.kt +++ b/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/InMemoryExtractionRunStore.kt @@ -234,7 +234,15 @@ class InMemoryExtractionRunStore @JvmOverloads constructor( // no other writer and sees the terminal run already committed. Exactly one call per run // reaches the applied branch above; a replay reports the run and stays silent. if (result.isApplied) { - listener.onEvent(ExtractionRunTransitioned(result.run)) + try { + listener.onEvent(ExtractionRunTransitioned(result.run)) + } catch (t: Throwable) { + // The write already landed above; a listener throwing here is the listener's own + // problem, not a reason to tell the caller the transition failed. A caller that saw + // that would retry a write already committed and get REPLAYED back, with no event + // for it to have ever acted on either time. + logger.error("DiceEventListener threw announcing the transition for run {}", key, t) + } } return result } diff --git a/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunLifecycleTest.kt b/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunLifecycleTest.kt index 51f4606a..406be635 100644 --- a/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunLifecycleTest.kt +++ b/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunLifecycleTest.kt @@ -266,6 +266,23 @@ class ExtractionRunLifecycleTest { .isEqualTo(ExtractionRunTransitionOutcome.APPLIED) } + @Test + fun `a listener that throws does not un-commit the transition`() { + val throwing = InMemoryExtractionRunStore(listener = { throw IllegalStateException("listener boom") }) + val run = started("run-throwing-listener") + throwing.save(run) + val transition = ExtractionRunTransition.completed(FINISHED_AT) + + val result = throwing.transition(run.key(), transition) + + assertThat(result.outcome).isEqualTo(ExtractionRunTransitionOutcome.APPLIED) + assertThat(throwing.findRun(run.key())?.status).isEqualTo(ExtractionRunStatus.COMPLETED) + // A retry sees the write already landed and replays, exactly as it would have if the + // listener had never thrown at all. + assertThat(throwing.transition(run.key(), transition).outcome) + .isEqualTo(ExtractionRunTransitionOutcome.REPLAYED) + } + // ---- nothing reaches COMPLETED except through the transition ---- @Test diff --git a/docs/design/extraction-runs.md b/docs/design/extraction-runs.md index dfb724f8..dacf6493 100644 --- a/docs/design/extraction-runs.md +++ b/docs/design/extraction-runs.md @@ -596,9 +596,12 @@ every downstream consumer a second time for a run that ended once — which is t The listener arrives the way every other DICE listener does: a constructor collaborator defaulting to `DiceEventListener.DEV_NULL`, the same shape `EventEmittingPropositionRepository` uses. Nothing is wired automatically. A host that has nothing listening builds the store exactly as it did before, -and a host that wants the signal passes a listener in. Handlers run inline on the calling thread and -throw isolation belongs to the listener, so a host that needs graceful degradation wraps its -listener in `SafeDiceEventListener`. +and a host that wants the signal passes a listener in. Handlers run inline on the calling thread, +and a listener that throws no longer reaches the caller: the store catches it, logs it at `error` +with the run's key, and still reports the transition as committed. The write already landed before +the announcement ran, so a caller told the listener failed would only retry a write already made +and get `REPLAYED` back, with no event to show for either attempt. `SafeDiceEventListener` is still there for a caller who wants a listener that never throws in the +first place; the store no longer depends on that, and backs it up on its own. The announcement happens after the write has landed and outside the store's own lock, so a listener that blocks, or that reads the run back, holds up no other writer and sees the terminal run already @@ -689,7 +692,10 @@ the query and then limits. Fetching `limit` rows and filtering by tenant afterwa rows than asked for — or none — whenever a busy neighbouring tenant occupies the head of the index, and the caller cannot tell that from a tenant with no runs. This is the drift-report store's rule carried over, and it is why none of the scoped reads has a default body: a default that filtered in -memory would be inherited silently by every backend that forgot to override it. +memory would be inherited silently by every backend that forgot to override it. The in-memory +reference store keeps this cheap for itself too: a per-tenant index of that tenant's own run keys, +maintained on insert and eviction, so a scoped read never has to look at another tenant's runs at +all, let alone filter them out one by one. The `ContextId`-typed overloads do have default bodies and are a different thing — they forward to the `String`-typed method that is the override point, and cannot return the wrong rows because they @@ -722,6 +728,14 @@ instance holds every tenant's runs, so an "everything in the store" method would the shipped backend a cross-tenant unbounded read on a contract that is neither. The tests read through the contract like any other caller. +It also caps how many runs it keeps: a constructor parameter, `maxRuns`, defaulting to 10,000. +Past that cap, an insert evicts the oldest runs that have already ended, by `startedAt`, until the +store fits again; a run still `RUNNING` is never evicted, so a store where every run happens to be +running can grow past the cap, and it logs that once, not on every insert. Retention past +that point is a durable store's own policy, kept for as long as an operator decides; the reference +store caps because holding every run ever seen in one JVM's memory forever is not something a host +running it in production should be signing up for by default. + Compare-and-set is real there, not simulated — every write and read runs inside one monitor, so the read of a run's status and the write that changes it cannot interleave. A durable store gets the same guarantee from its transaction, and the cross-backend suite races two threads to end one run