diff --git a/CHANGELOG.md b/CHANGELOG.md index defbb0ba..4e7dfaf4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -642,7 +642,7 @@ and the consumer PRs that deliver it). application with no `DeclaredSchemaSource` bean sees no change. Nothing appears on an application's HTTP surface until it imports `DiceRestConfiguration`. -- Optional source revisions in the `dice` core provenance model, the first slice of DICE #64. +- Optional source revisions in the `dice` core provenance model, the first slice of DICE #64. **EXPERIMENTAL** (shape may change before 1.0) — opt-in: a store implements `SourceRevisionQueryCapable`. `ProvenanceEntry` gains a sixth field, `sourceRevision`: an opaque, provider-defined string, non-blank when present, recording which version of a source a claim was read from. `SourceLocator.key()` is untouched, so one document read at two revisions is still one source @@ -1675,8 +1675,9 @@ and the consumer PRs that deliver it). **A header write cannot touch a child row.** Invocation records are their own nodes, so `save` has no way to delete one — the contract's merge-don't-replace rule falls out of the graph model instead of being implemented. One consequence: a durable store keeps identified rows rather than the order a - caller listed them in, so it returns attempts in plan order where the in-memory reference returns - the caller's order. `invocationsOf` is plan order in both. + caller listed them in, so it returns attempts in plan order. `invocationsOf` is plan order in both + backends, and the lineage slice below makes `ExtractionRun.invocations` plan order too, so the two + agree on the whole run rather than only on that one read. **Every page scopes in the query ahead of its `LIMIT`**, excludes rows with no sort key (Neo4j sorts null largest, so one would sort to the front of a `DESC` order, spend a slot, and then be dropped by the mapper), and skips corrupt rows with a warning rather than failing the whole read. `runsOfRoot` @@ -1720,3 +1721,201 @@ and the consumer PRs that deliver it). failure. `dice-storage` now declares the `neo4j-java-driver` dependency it already ran with through Drivine, so the exception class it checks is visible at compile time too. **Compatibility: additive.** No public signature changes. + +- **EXPERIMENTAL.** Extraction-run lineage: a stored claim can now be traced to the runs that + produced it, and the canonical id it was stored under is no longer thrown away. Two halves. + **Canonical ids come back from a save.** `DrivinePropositionRepository.save` has always answered a fresh + insert of text it already holds with the proposition it already holds — a different id from the one + extraction minted — and `PropositionStore.saveAll` returns `Unit`, so callers never learned it. Any + edge, projection or grounding link written afterwards against the minted id points at a node that + was never stored. Two additive calls carry the answer back: + `PropositionStore.saveAllReturningCanonical` does the same writes as `saveAll` and returns a + `PropositionPersistenceResult` — the stored proposition per input, in input order, plus the + input-id to stored-id map — and `PersistablePropositions.persistReturningCanonical` does the same + persistence `persist` does with structural relationships wired against what the repository + returned. The result type publishes two views that are not interchangeable: + `canonicalPropositions` is positional and can repeat when two inputs deduplicate onto one, + `canonicalIds` is the distinct set for writes that should happen once per stored proposition, and + `distinctCanonicalPropositions` is that same view with the objects attached — what structural + wiring, projection and grounding run over, so inputs that deduplicated together are one unit of + downstream work rather than one per input, which would repeat idempotent edge writes and inflate + the records written about them. `of` also rejects a canonical result carrying a different + `ContextId` from its input: a foreign-tenant proposition has no business reaching a link at all, + so the check belongs where the object enters the pipeline. It + also rejects one stored id answered under two different contexts, which each per-position check + would pass individually while the resolution step handed the earlier position the other tenant's + object. When + one batch names an id twice — two revision results touching one original — every position reports + the store's *last* answer for it, because a replace-by-id store overwrites the first and the first + object is stale from that moment; resolving rather than rejecting matters because `of` runs after + the saves, so throwing would fail an extraction whose propositions are already written. One input + id answered with two *different* stored ids is still rejected. + `DrivinePropositionRepository` itself needed no change — the canonical id was already in its + return value, and it was being dropped a layer up. `EventEmittingPropositionRepository` overrides + the new call for the same reason it overrides `saveAll`: Kotlin's `by delegate` forwards an + interface default straight past the decorator's own `save`. + **The relation.** `(:Proposition {id, contextId})-[:PRODUCED_BY_RUN]->(:ExtractionRun {contextId, + runId})`, behind the new `PropositionRunLinkStore` with `InMemoryPropositionRunLinkStore` in `dice` + main sources and `DrivinePropositionRunLinkStore` in `dice-storage`. Many-to-many in both + directions, which it has to be: one claim is produced by many runs whenever a re-extraction + deduplicates onto a proposition an earlier run created, and both runs are true answers to "what + produced this?". The edge carries no properties, so a replay has nothing to disagree about and the + write is a plain `MERGE`. It is a **dedicated surface rather than four methods on + `ExtractionRunStore`**: the tenant guard has to know whether a *proposition* exists in a tenant, + which a run-header store cannot answer, and welding "what runs exist" to "what claims a run + produced" would make the in-memory run store grow a proposition index it has no business holding. + Both reads are bounded by a positive limit and ordered by id ascending — repeatable without joining + a run header for its start time, which a caller wanting newest-first can do through + `ExtractionRunStore`. + **The write is tenant-guarded and the reads fail closed.** Every statement names `contextId` on + both endpoints, so a cross-tenant edge is not expressible. `link` additionally resolves the run and + then every proposition inside the run's tenant before writing, because "matched nothing" and "you + asked to link a neighbour's claim" are the same silence; an id that resolves in another tenant or + nowhere at all raises `PropositionRunLinkScopeException` naming it, and one out-of-scope id rejects + the whole batch with nothing written. **The preflight names, the write decides.** Validation and + the `MERGE` are separate statements, so under read-committed a proposition deleted or re-tenanted + between them would pass the check and be gone by the write — so the Drivine statement counts its + own matches (`WHERE size(ps) = $expected`) in the snapshot it writes in, and the caller compares + the returned count against the batch size and rolls back on any mismatch. Both backends' reads + also resolve against live endpoint state rather than a remembered link, so deleting a proposition + removes its lineage from both directions on either backend, as detaching the node already did on + a graph. + **Run identity stays out of source provenance.** Nothing here touches `ProvenanceEntry` or + `SourceLocator`, and that is asserted both behaviourally and structurally. Folding a run into + source identity would make evidence from two runs over one document look like evidence from two + documents, and would change what `SourceLocator.key()` means — which is the `:Source` node's key. + The headline invariant is measured on Neo4j: two runs over identical content leave **one + proposition, one source grounding, two run links**. + Design note: [docs/design/extraction-runs.md](docs/design/extraction-runs.md). + **Compatibility: additive, with one behavioural change scoped to run-present flows and one to an + unreleased experimental type.** No existing class loses a member and no signature moves. + `PropositionStore.saveAll` keeps its `Unit` descriptor and its body; `PersistablePropositions.persist` + is untouched, and a test asserts that with nothing deduplicated the two persist paths write + identical edges. `IncrementalPropositionExtraction` gains a `withRunLineage(store)` method rather + than a constructor parameter, and that is a deliberate ABI choice: Kotlin compiles a constructor + with default arguments into one synthetic `(...every parameter..., int mask, + DefaultConstructorMarker)`, which is what a precompiled Kotlin caller links against whenever it + omits an argument. Appending a defaulted parameter rewrites that descriptor and breaks every such + caller with `NoSuchMethodError`; `@JvmOverloads` does not help, because it republishes the Java + overloads those callers never touch. Adding a method adds API; appending a defaulted parameter + moves one. `RunLineageBinaryCompatibilityTest` pins the synthetic descriptor, every + `@JvmOverloads` arity, and that no constructor mentions the lineage store at all. The binding is + **one-time**: a second `withRunLineage` call throws `IllegalStateException` rather than silently + swapping or clearing the store an in-flight extraction is about to record against. + **Compatibility: behavioral, every extraction flow — a fix.** `persistAndProject` now wires + structural relationships, graph projection and grounding against the propositions the repository + returned, on every call, with or without a run. It used to do that only when a + `SourceAnalysisContext.currentRun` was present and to use the pre-save objects otherwise, which + made an audit setting decide whether the graph was written correctly: a host that turned on + extraction runs silently got different edges, and a host that did not kept writing edges against + ids the store does not hold. Under dedup or merge those phantom pre-save ids reached projection and + grounding, so the edges pointed at nodes that were never stored — the same class of defect the + `POST /extract` response fix above closed, arriving here in the write path. Audit + metadata never changes product behaviour: a run now adds a lineage write and nothing else, and a + test runs the same extraction with and without one and compares everything except that write. + Hosts that never passed a run get corrected edges under dedup without changing a line. Lineage is + now written **right after the save, before structural wiring, projection or grounding run**, so a + claim is attributed the moment it exists, not once the rest of the pipeline has also succeeded on + it. **Compatibility: behavioral.** A `STRICT` failure now reaches the caller with only the save + done: canonical claims persisted, structural edges not wired, projection not run, grounding not + run, and no `PRODUCED_BY_RUN` edge. A host that previously saw a `STRICT` failure arrive with a + fully wired, projected and grounded extraction behind it now sees the failure sooner and with less + work done, because none of that later work runs until attribution has succeeded. `LENIENT` still + reaches the old end state and reports success: the link write is skipped and every later pass runs + regardless. The split of `persistReturningCanonical` into `persistCanonicalPropositions` and + `wireStructuralRelationships` stays, both published, the original preserved as their composition, + because the canonical propositions a save returns are what lineage and every later pass wire + against. An earlier cut of this slice ran lineage last, on the reasoning that a `STRICT` failure + should leave a complete extraction behind with only the audit edge missing; that ordering meant a + claim could sit fully wired, projected and grounded with nobody able to say which run produced it, + for as long as the pipeline kept running past the save. Attribution is now checked before any of + that work happens, so a lineage failure means the later passes never ran, and the honest report of + that is a saved, unattributed claim with nothing built around it yet. + **Attribution fails loud by policy.** A new `LineageFailurePolicy` says what happens when lineage + cannot be written, and `STRICT` is the default. Under it, two things fail the extraction: an + analysis carrying a run with no `PropositionRunLinkStore` bound, and a link write that throws. Both + used to be swallowed — the missing store returned quietly and every `RuntimeException` went to a + log line — so the two failures an operator most needs to hear about, lineage wired wrong and + lineage refusing writes, both reported success. An audit surface whose absence is silent is worth + nothing at the one moment it is consulted. `LENIENT` restores the old behaviour for a host that has + decided the claims outweigh their attribution and says so in configuration; the failure carries its + cause either way, so a scope rejection and an outage stay distinguishable. Bound with the store: + `withRunLineage(store, policy)` is a second overload, for the same descriptor reason the + one-argument form exists at all. Failures raise + `LineageNotRecordedException`. An analysis that saved nothing records nothing and fails under + neither policy. + **The async event path is as loud as a direct call** (PR #101 review). `extractPropositions` + used to swallow every exception from an event-published extraction, so a `LineageNotRecordedException` + under `STRICT` was logged at `warn` and lost while the same inputs on a direct call failed. It is + now logged at `error` with the run key and rethrown to the publisher; the queue drain keeps going + past a failing event and raises the first failure once the queue is empty. **Compatibility: + behavioral.** A host publishing `SourceAnalysisRequestEvent`s with a run and `STRICT` lineage + now sees the failure where its event multicaster reports listener exceptions; under `LENIENT` + nothing changes. + **What joining a caller's transaction covers, exactly.** The lineage write joins a caller's + transaction and never opens its own: `REQUIRES_NEW` would suspend that transaction, and a suspended transaction's + uncommitted propositions are invisible, so a host wrapping extraction in `@Transactional` would get + fail-closed lineage on every extraction. Joining means Spring marks the participating transaction + rollback-only when `link` throws — Drivine overrides `doSetRollbackOnly` and the flag is set — but + the flag is write-only: `DrivineTransactionObject` does not implement `SmartTransactionObject`, so + Spring cannot see it and Drivine never reads it when committing. Propagated, then dropped, and + pinned by a test that goes red if either changes. The guarantee therefore covers the failures + `link` raises itself, all of which are thrown after its statements succeeded. It does **not** cover + a statement that fails at the server, which terminates the transaction beneath Spring where no + catch reaches; a test injects that and measures the cost. Under `STRICT` a raised failure reaches + the caller, so inside a host's `@Transactional` the claims and their lineage roll back together — + which is what strict attribution asks for. Hosts that do not wrap extraction in a + transaction are unaffected, since each save has already committed. DICE #67 slice 10 closes the + window by committing claims before recording lineage. + **Behavioural for equality, on an unreleased type:** `ExtractionRun.invocations` is now normalized + to plan order — `(invocationIndex, attempt)` — at construction, so `equals`, `hashCode` and + `toString` are canonical and the two store backends return equal runs for one call sequence. It + used to keep the order the caller supplied, which is the order calls came back, which is not a fact + about the run. `invocationsInPlanOrder()` is now the identity and stays as a named call. + `sourceRevisions` is deliberately left alone: the order sources were read in is data. Nothing has + released `ExtractionRun` — the type is `@ApiStatus.Experimental` and arrived earlier in this same + unreleased train — so no consumer can be depending on the old order, but the change is called out + because a caller comparing runs or reading `invocations[0]` would see it. + **No new schema and no migration.** `ExtractionRunSchema.specs()` is unchanged: both endpoint + labels already carry the uniqueness constraints these statements seek on, and a relationship has no + key of its own, since `MERGE` on a pattern between two matched nodes creates at most one edge and + Neo4j locks the endpoints when it decides to create. Duplicate `PRODUCED_BY_RUN` edges are pinned + from both sides: the in-memory store keeps the relation as a set behind one monitor and a + concurrency test drives eight threads at one pair, and the Drivine store counts the relationships + themselves after linking the same pair twice, because every read in the contract returns refs and + ids, so a duplicate edge is invisible to all of them. A relationship-level uniqueness + constraint is not available to back this up: Drivine's schema vocabulary is node-scoped + (`UniquenessConstraintSpec` takes a label), so there is nothing to declare in a `SchemaCatalog`. + `ExtractionRunSchema` gains a `PRODUCED_BY_RUN_REL` constant. + **Auto-configured behind an off-by-default property.** `dice-storage-autoconfigure` registers + `DrivineExtractionRunStore`, `DrivinePropositionRunLinkStore` and the run schema catalog on two + conditions: `embabel.dice.store.type=graph`, and a new + `embabel.dice.extraction.runs.enabled=true`. The property defaults to false, so a host that + upgrades gets none of the three until it asks for them. They used to be declared only by + `dice-storage`'s own `TestApplication`, so the suite exercised them and no host could get them + without writing the beans by hand; a host that declares its own keeps them, under the same + `@ConditionalOnMissingBean` posture as every store beside them. + **The in-memory link store prunes what it can see is gone** (PR #101 review). A read that finds a + link whose proposition is gone altogether drops the id, and an emptied run + entry with it, so the reference store's memory stops growing with deleted claims; re-saving the + same id does not revive the link. Since nothing reads on a host's behalf, the store also takes a + `maxRuns` constructor parameter, defaulting to 10,000 and added last so `@JvmOverloads` keeps the + existing Java descriptor, and past it evicts the links of the runs linked earliest once the run + store says they have ended; a run still `RUNNING` keeps its links, and a breach with nothing to + evict logs once. **Compatibility: additive.** Every read answers as before, and every existing + constructor call keeps compiling. + + The property exists because of what registration writes. The schema catalog goes to Drivine's + schema manager, which ensures it on startup, so turning the flag on adds three uniqueness + constraints and five range indexes to the host's database. The constraints are + `ExtractionRun(contextId, runId)`, + `ExtractionRunInvocation(contextId, runId, invocationIndex, attempt)` and + `ExtractionRunTerminalWrite(contextId, runId)`. The indexes are `ExtractionRun(contextId)`, + `ExtractionRun(contextId, rootRunId)`, `ExtractionRun(contextId, parentRunId)`, + `ExtractionRun(contextId, startedAtEpochSecond)` and `ExtractionRunInvocation(contextId, runId)`. + The two stores stay unreachable until a caller names a run on an `ExtractionRequest`, and lineage + additionally has to be bound with `withRunLineage`. With the flag off the schema manager is handed + no run specs at all, and a test asserts that across every `SchemaCatalog` bean in the context. A + host constructing `InMemoryExtractionRunStore` for itself is untouched by the flag either way. No + released DICE ever wrote this relationship type. Every new type carries `@ApiStatus.Experimental` + and the shapes may still move while the remaining #67 slices land. diff --git a/dice-storage-autoconfigure/src/main/kotlin/com/embabel/dice/storage/autoconfigure/DiceStorageAutoConfiguration.kt b/dice-storage-autoconfigure/src/main/kotlin/com/embabel/dice/storage/autoconfigure/DiceStorageAutoConfiguration.kt index 9f34969e..ce76cd4f 100644 --- a/dice-storage-autoconfigure/src/main/kotlin/com/embabel/dice/storage/autoconfigure/DiceStorageAutoConfiguration.kt +++ b/dice-storage-autoconfigure/src/main/kotlin/com/embabel/dice/storage/autoconfigure/DiceStorageAutoConfiguration.kt @@ -27,13 +27,18 @@ import com.embabel.dice.projection.lineage.ProjectionRecordStore import com.embabel.dice.proposition.DecayManager import com.embabel.dice.proposition.DecaySweepConfig import com.embabel.dice.proposition.PropositionRepository +import com.embabel.dice.proposition.extraction.ExtractionRunStore +import com.embabel.dice.proposition.extraction.PropositionRunLinkStore import com.embabel.dice.proposition.store.InMemoryDecayManager import com.embabel.dice.proposition.store.InMemoryPropositionRepository import com.embabel.dice.storage.DiceStorageSchema import com.embabel.dice.storage.DrivineChunkHistoryStore import com.embabel.dice.storage.DrivineCollectorRecordStore +import com.embabel.dice.storage.DrivineExtractionRunStore import com.embabel.dice.storage.DrivinePropositionRepository +import com.embabel.dice.storage.DrivinePropositionRunLinkStore import com.embabel.dice.storage.DrivineProjectionRecordStore +import com.embabel.dice.storage.ExtractionRunSchema import com.embabel.dice.storage.GraphDecayManager import com.embabel.dice.storage.LineageSchema import com.embabel.dice.storage.diceStorageCatalog @@ -142,6 +147,82 @@ class DiceStorageAutoConfiguration { persistenceManager: PersistenceManager, ): CollectorRecordStore = DrivineCollectorRecordStore(persistenceManager) + /** + * The durable extraction-run header store. + * + * Two conditions, both required. The graph backend has to be selected + * (`embabel.dice.store.type=graph`), and extraction runs have to be turned on with + * `embabel.dice.extraction.runs.enabled=true`. The property defaults to off, so an upgrading + * host gets none of this until it asks. + * + * The property is what carries the decision, because registering these beans is what puts the + * run schema in front of Drivine's schema manager, and that writes constraints and indexes to + * the host's database on startup. [extractionRunSchema] lists exactly what lands. A host that + * turns the flag on has consented to those writes; one that leaves it alone keeps a database + * with no run labels in it. + */ + @Bean + @ConditionalOnProperty(prefix = "embabel.dice.store", name = ["type"], havingValue = "graph") + @ConditionalOnProperty( + prefix = "embabel.dice.extraction.runs", + name = ["enabled"], + havingValue = "true", + matchIfMissing = false, + ) + @ConditionalOnMissingBean(ExtractionRunStore::class) + fun drivineExtractionRunStore( + persistenceManager: PersistenceManager, + transactionManager: PlatformTransactionManager, + ): ExtractionRunStore = DrivineExtractionRunStore(persistenceManager, transactionManager) + + /** + * Where `(proposition, run)` links go. + * + * Registered on the same two conditions as the run store: the graph backend, and + * `embabel.dice.extraction.runs.enabled=true`. It is reached only by an extraction that carries + * a run, and binding it is still the host's move: `IncrementalPropositionExtraction` takes it + * through `withRunLineage`, so having the bean in the context does not by itself make anything + * record lineage. + */ + @Bean + @ConditionalOnProperty(prefix = "embabel.dice.store", name = ["type"], havingValue = "graph") + @ConditionalOnProperty( + prefix = "embabel.dice.extraction.runs", + name = ["enabled"], + havingValue = "true", + matchIfMissing = false, + ) + @ConditionalOnMissingBean(PropositionRunLinkStore::class) + fun drivinePropositionRunLinkStore( + persistenceManager: PersistenceManager, + ): PropositionRunLinkStore = DrivinePropositionRunLinkStore(persistenceManager) + + /** + * The constraints and indexes the run store and the lineage relation need. + * + * Separate from [lineageRecordSchema] because that one is the projection and collector audit + * trail, which has its own labels and its own lifecycle. Both are ensured on startup by + * Drivine's schema manager. + * + * Behind the same `embabel.dice.extraction.runs.enabled` flag as the two stores, and for the + * reason the flag exists: this catalog is the part that writes to the host's database. Turning + * the flag on ensures three uniqueness constraints — on `ExtractionRun(contextId, runId)`, + * `ExtractionRunInvocation(contextId, runId, invocationIndex, attempt)` and + * `ExtractionRunTerminalWrite(contextId, runId)` — and five range indexes, four on + * `ExtractionRun` (`contextId`; `contextId, rootRunId`; `contextId, parentRunId`; + * `contextId, startedAtEpochSecond`) and one on `ExtractionRunInvocation(contextId, runId)`. + * Leaving it off keeps every one of them out of the catalog the schema manager sees. + */ + @Bean + @ConditionalOnProperty(prefix = "embabel.dice.store", name = ["type"], havingValue = "graph") + @ConditionalOnProperty( + prefix = "embabel.dice.extraction.runs", + name = ["enabled"], + havingValue = "true", + matchIfMissing = false, + ) + fun extractionRunSchema(): SchemaCatalog = SchemaCatalog.of(ExtractionRunSchema.specs()) + /** * The lineage stores' schema, registered as a [DiceStorageSchema] so one bean answers both * questions about it: Drivine ensures its constraints and indexes, and dice's drift observation diff --git a/dice-storage-autoconfigure/src/test/kotlin/com/embabel/dice/storage/autoconfigure/ExtractionRunStoreAutoConfigurationTest.kt b/dice-storage-autoconfigure/src/test/kotlin/com/embabel/dice/storage/autoconfigure/ExtractionRunStoreAutoConfigurationTest.kt new file mode 100644 index 00000000..34ca7b97 --- /dev/null +++ b/dice-storage-autoconfigure/src/test/kotlin/com/embabel/dice/storage/autoconfigure/ExtractionRunStoreAutoConfigurationTest.kt @@ -0,0 +1,233 @@ +/* + * 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.autoconfigure + +import com.embabel.dice.proposition.PropositionStore +import com.embabel.dice.proposition.extraction.ExtractionRunStore +import com.embabel.dice.proposition.extraction.InMemoryExtractionRunStore +import com.embabel.dice.proposition.extraction.InMemoryPropositionRunLinkStore +import com.embabel.dice.proposition.extraction.PropositionRunLinkStore +import com.embabel.dice.storage.DrivineExtractionRunStore +import com.embabel.dice.storage.DrivinePropositionRunLinkStore +import com.embabel.dice.storage.ExtractionRunSchema +import org.assertj.core.api.Assertions.assertThat +import org.drivine.manager.GraphObjectManager +import org.drivine.manager.PersistenceManager +import org.drivine.schema.SchemaCatalog +import org.junit.jupiter.api.Test +import org.mockito.kotlin.mock +import org.springframework.boot.autoconfigure.AutoConfigurations +import org.springframework.boot.test.context.assertj.AssertableApplicationContext +import org.springframework.boot.test.context.runner.ApplicationContextRunner +import org.springframework.context.annotation.Bean +import org.springframework.context.annotation.Configuration +import org.springframework.transaction.PlatformTransactionManager + +/** + * Wiring for the extraction-run store, the lineage link store, and the run schema catalog. + * + * These used to be declared only by `dice-storage`'s own `TestApplication`, which meant the suite + * exercised them and no host could get them without writing the beans by hand. They are registered + * here behind two conditions: the graph backend has to be selected, and + * `embabel.dice.extraction.runs.enabled` has to be `true`. A host that declares its own stores still + * wins. + * + * **Why the flag.** The schema catalog is the part with an effect a host can see. Drivine's schema + * manager ensures every [SchemaCatalog] bean on startup, so registering [ExtractionRunSchema] writes + * three constraints and five indexes into the host's database. That is a decision a host makes on + * purpose, so the default is off and the tests below pin both halves: nothing lands with the flag + * absent, and everything lands with it on. + */ +class ExtractionRunStoreAutoConfigurationTest { + + private val runner = ApplicationContextRunner() + .withConfiguration(AutoConfigurations.of(DiceStorageAutoConfiguration::class.java)) + .withUserConfiguration(StubGraphInfrastructure::class.java) + + @Test + fun `the graph backend with extraction runs enabled registers both stores and their schema`() { + runner + .withPropertyValues( + "embabel.dice.store.type=graph", + "embabel.dice.extraction.runs.enabled=true", + ) + .run { ctx -> + assertThat(ctx).hasSingleBean(ExtractionRunStore::class.java) + assertThat(ctx).hasSingleBean(PropositionRunLinkStore::class.java) + assertThat(ctx.getBean(ExtractionRunStore::class.java)) + .isInstanceOf(DrivineExtractionRunStore::class.java) + assertThat(ctx.getBean(PropositionRunLinkStore::class.java)) + .isInstanceOf(DrivinePropositionRunLinkStore::class.java) + + // The constraints the run key and the terminal write depend on. Without them the + // store's MERGE and CREATE upserts are not race-free, so the schema travels with the + // beans once the flag is on, and a host does not opt into it separately. + assertThat(ctx).hasBean("extractionRunSchema") + assertThat(runSchemaLabelsReachingTheSchemaManager(ctx)) + .containsExactlyInAnyOrderElementsOf(ExtractionRunSchema.LABELS) + } + } + + @Test + fun `the default context on the graph backend registers none of the three beans`() { + // The property is absent, which is the default a host upgrades into. None of the three + // beans exist, and the schema manager is handed no ExtractionRun specs at all, so nothing + // is written to the host's database on startup. + runner + .withPropertyValues("embabel.dice.store.type=graph") + .run { ctx -> + assertThat(ctx).doesNotHaveBean(ExtractionRunStore::class.java) + assertThat(ctx).doesNotHaveBean(PropositionRunLinkStore::class.java) + assertThat(ctx).doesNotHaveBean("extractionRunSchema") + assertThat(runSchemaSpecsReachingTheSchemaManager(ctx)).isEmpty() + } + } + + @Test + fun `the flag set to false registers none of the three beans`() { + runner + .withPropertyValues( + "embabel.dice.store.type=graph", + "embabel.dice.extraction.runs.enabled=false", + ) + .run { ctx -> + assertThat(ctx).doesNotHaveBean(ExtractionRunStore::class.java) + assertThat(ctx).doesNotHaveBean(PropositionRunLinkStore::class.java) + assertThat(ctx).doesNotHaveBean("extractionRunSchema") + assertThat(runSchemaSpecsReachingTheSchemaManager(ctx)).isEmpty() + } + } + + @Test + fun `without the graph backend neither store is registered`() { + // Both conditions are required, so the flag alone buys a non-graph host nothing. + runner + .withPropertyValues("embabel.dice.extraction.runs.enabled=true") + .run { ctx -> + assertThat(ctx).doesNotHaveBean(ExtractionRunStore::class.java) + assertThat(ctx).doesNotHaveBean(PropositionRunLinkStore::class.java) + assertThat(ctx).doesNotHaveBean("extractionRunSchema") + } + } + + @Test + fun `an explicitly configured store type other than graph registers neither`() { + runner + .withPropertyValues( + "embabel.dice.store.type=in-memory", + "embabel.dice.extraction.runs.enabled=true", + ) + .run { ctx -> + assertThat(ctx).doesNotHaveBean(ExtractionRunStore::class.java) + assertThat(ctx).doesNotHaveBean(PropositionRunLinkStore::class.java) + } + } + + @Test + fun `a host that declares its own stores keeps them`() { + // ConditionalOnMissingBean, from the host's side: someone with their own backend, or a + // recording decorator around ours, must not end up with two. + runner + .withPropertyValues( + "embabel.dice.store.type=graph", + "embabel.dice.extraction.runs.enabled=true", + ) + .withUserConfiguration(HostSuppliedStores::class.java) + .run { ctx -> + assertThat(ctx).hasSingleBean(ExtractionRunStore::class.java) + assertThat(ctx).hasSingleBean(PropositionRunLinkStore::class.java) + assertThat(ctx.getBean(ExtractionRunStore::class.java)) + .isNotInstanceOf(DrivineExtractionRunStore::class.java) + assertThat(ctx.getBean(PropositionRunLinkStore::class.java)) + .isNotInstanceOf(DrivinePropositionRunLinkStore::class.java) + } + } + + @Test + fun `a host running the in-memory run store keeps it with the flag off`() { + // The in-memory path is the host's own construction and owes nothing to this wiring. With + // the flag absent its stores are still there and still usable, and the flag's only effect + // is that no graph beans and no run schema join them. + runner + .withPropertyValues("embabel.dice.store.type=graph") + .withUserConfiguration(HostSuppliedInMemoryStores::class.java) + .run { ctx -> + assertThat(ctx).hasSingleBean(ExtractionRunStore::class.java) + assertThat(ctx).hasSingleBean(PropositionRunLinkStore::class.java) + assertThat(ctx.getBean(ExtractionRunStore::class.java)) + .isInstanceOf(InMemoryExtractionRunStore::class.java) + assertThat(ctx.getBean(PropositionRunLinkStore::class.java)) + .isInstanceOf(InMemoryPropositionRunLinkStore::class.java) + assertThat(ctx).doesNotHaveBean("extractionRunSchema") + assertThat(runSchemaSpecsReachingTheSchemaManager(ctx)).isEmpty() + } + } + + /** + * Every `ExtractionRun` schema item any [SchemaCatalog] bean in the context carries. + * + * Drivine's schema manager collects the catalog beans and ensures what they hold, so this is + * what would actually be written to a host's database on startup. + */ + private fun runSchemaSpecsReachingTheSchemaManager(ctx: AssertableApplicationContext): List = + ctx.getBeansOfType(SchemaCatalog::class.java).values + .flatMap { catalog -> catalog.items } + .filter { spec -> spec.label in ExtractionRunSchema.LABELS } + .map { spec -> "${spec.kind} ${spec.label}${spec.properties}" } + + /** The distinct run labels those items name. */ + private fun runSchemaLabelsReachingTheSchemaManager(ctx: AssertableApplicationContext): List = + ctx.getBeansOfType(SchemaCatalog::class.java).values + .flatMap { catalog -> catalog.items } + .map { spec -> spec.label } + .filter { label -> label in ExtractionRunSchema.LABELS } + .distinct() + + /** What Drivine would supply in a real application, as mocks. Nothing here is called. */ + @Configuration(proxyBeanMethods = false) + open class StubGraphInfrastructure { + + @Bean + open fun persistenceManager(): PersistenceManager = mock() + + @Bean + open fun graphObjectManager(): GraphObjectManager = mock() + + @Bean + open fun transactionManager(): PlatformTransactionManager = mock() + } + + @Configuration(proxyBeanMethods = false) + open class HostSuppliedStores { + + @Bean + open fun hostRunStore(): ExtractionRunStore = mock() + + @Bean + open fun hostLinkStore(): PropositionRunLinkStore = mock() + } + + @Configuration(proxyBeanMethods = false) + open class HostSuppliedInMemoryStores { + + @Bean + open fun hostRunStore(): ExtractionRunStore = InMemoryExtractionRunStore() + + @Bean + open fun hostLinkStore(runStore: ExtractionRunStore): PropositionRunLinkStore = + InMemoryPropositionRunLinkStore(runStore, mock()) + } +} diff --git a/dice-storage/src/main/kotlin/com/embabel/dice/storage/DrivinePropositionRunLinkStore.kt b/dice-storage/src/main/kotlin/com/embabel/dice/storage/DrivinePropositionRunLinkStore.kt new file mode 100644 index 00000000..e3b43634 --- /dev/null +++ b/dice-storage/src/main/kotlin/com/embabel/dice/storage/DrivinePropositionRunLinkStore.kt @@ -0,0 +1,334 @@ +/* + * 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.ExtractionRunKey +import com.embabel.dice.proposition.extraction.ExtractionRunNotFoundException +import com.embabel.dice.proposition.extraction.ExtractionRunRef +import com.embabel.dice.proposition.extraction.PropositionRunLinkScopeException +import com.embabel.dice.proposition.extraction.PropositionRunLinkStore +import org.drivine.manager.PersistenceManager +import org.jetbrains.annotations.ApiStatus +import org.drivine.query.QuerySpecification +import org.slf4j.LoggerFactory +import org.springframework.transaction.annotation.Transactional + +/** + * Drivine / Neo4j implementation of [PropositionRunLinkStore]. + * + * ## The graph + * + * `(:Proposition {id, contextId})-[:PRODUCED_BY_RUN]->(:ExtractionRun {contextId, runId})` + * + * One edge type, no properties on it. A bare edge says one thing — this run produced this claim — + * and there is nothing on it for a replay to disagree about, which is what lets the write be a plain + * `MERGE`. A timestamp would have to be `ON CREATE SET` to stay idempotent, and it would duplicate + * what the run header's `startedAt` already records. + * + * The relation is many-to-many in both directions and the graph is the right shape for it. One run + * points at every claim it produced; one claim points at every run that produced it, which is the + * normal outcome of re-extraction rather than an edge case. + * + * ## The tenant guard is a MATCH, and then it is a check + * + * Every statement here names `contextId` on both endpoints, so an edge between two tenants cannot + * be matched or created by anything in this class. That makes the reads fail closed for free: a + * neighbour's run is not in this tenant's pattern. + * + * A write needs more than "matched nothing", because "nothing matched" and "you asked to link a + * neighbour's proposition" are the same silence. So [link] resolves the run and then the + * propositions before it writes, and names what did not resolve. Both checks and the write run in + * one transaction, and the exception rolls it back, so a batch with one out-of-scope id leaves the + * graph exactly as it found it. + * + * ## No new constraint, and why that is not an oversight + * + * Neither endpoint label is new. `Proposition(id)` and `ExtractionRun(contextId, runId)` already + * carry uniqueness constraints, and both are what these statements seek on: the write anchors on + * the run, the proposition lookup anchors on the id, and the inverse read anchors on the run and + * expands backwards. A relationship has no key of its own to constrain — `MERGE` on a pattern + * between two matched nodes creates at most one edge — so there is nothing here for a constraint to + * make race-free that the endpoint constraints do not already. + * + * See [ExtractionRunSchema] for the relationship type name and the constraints the run end depends + * on. + * + * Every statement is parameterized; nothing caller-derived is interpolated into Cypher. + * + * EXPERIMENTAL. The shape may still change while extraction runs (DICE #67) land. The marker is on + * this class rather than inherited from [PropositionRunLinkStore]: annotations on an interface do + * not carry to its implementations, so a host looking at the concrete bean it declares would see + * nothing. + * + * @param persistenceManager Drivine's handle on the `neo` datasource. + */ +@ApiStatus.Experimental +@Transactional +open class DrivinePropositionRunLinkStore( + private val persistenceManager: PersistenceManager, +) : PropositionRunLinkStore { + + private val logger = LoggerFactory.getLogger(DrivinePropositionRunLinkStore::class.java) + + /** + * Records the links, joining a caller's transaction when there is one. + * + * **Propagation is `REQUIRED`, and that is a decision rather than a default.** The alternative, + * `REQUIRES_NEW`, would guarantee that a failure here can never affect a caller's transaction — + * but it would also suspend that transaction, and a new one cannot see rows the suspended one + * has not committed. A host that wraps extraction in `@Transactional` saves its propositions in + * that transaction, so under `REQUIRES_NEW` every one of them would be invisible to the scope + * check and lineage would fail closed on every such extraction. Joining the caller means + * lineage resolves the claims it is about, and the two commit or roll back together. + * + * **What joining costs, stated exactly.** Spring marks a participating transaction rollback-only + * when an inner method throws, and it does so here: Drivine's manager overrides + * `doSetRollbackOnly` and the shared transaction object's `rollbackOnly` flag is set. The + * caller's commit survives anyway because that flag is write-only — `DrivineTransactionObject` + * does not implement `SmartTransactionObject`, so Spring's `isGlobalRollbackOnly` cannot see it, + * and Drivine never reads it in `doCommit`. The marking is propagated and then dropped. That is + * behaviour, not contract, so it is pinned rather than assumed: `lineage inside a caller's + * transaction sees its writes and cannot condemn it` goes red if Drivine implements + * `SmartTransactionObject` or starts reading the flag. + * + * **The guarantee covers application-level failures only.** Everything this method raises by + * itself — tenant guard, run-not-found, scope rejection, batch-changed — is thrown from Kotlin + * after its statements succeeded, so the Bolt transaction is healthy and a caller that catches + * really can carry on. A statement that fails *at the server* is not covered: it terminates the + * transaction beneath Spring, and no catch can undo that. A deadlock between two runs linking + * overlapping propositions is the realistic version: nothing orders the node locks two + * concurrent `MERGE` batches take over the same propositions. `a server-side failure + * inside a caller's transaction is the window best-effort does not cover` demonstrates it and + * measures the cost: the caller's later writes are lost with it. + * + * That window closes when the run coordinator commits claims before recording lineage (DICE #67 + * slice 10), which is what makes lineage a genuinely separate write rather than one that shares + * a caller's fate. Until then it is a real limitation of running extraction inside an ambient + * transaction, and hosts that do not wrap extraction are unaffected. + */ + @Transactional + override fun link(key: ExtractionRunKey, propositionIds: Collection): Int { + val ids = propositionIds.distinct() + if (ids.isEmpty()) return 0 + ExtractionRunSchema.requireStorableTenant(key.contextId) + + if (!runExists(key)) throw ExtractionRunNotFoundException(key) + + val inScope = propositionsInContext(key.contextId.value, ids) + val outOfScope = ids.filterNot { it in inScope } + if (outOfScope.isNotEmpty()) { + throw PropositionRunLinkScopeException(key, outOfScope) + } + + val row = singleRow( + MERGE_LINKS, + mapOf( + "contextId" to key.contextId.value, + "runId" to key.runRef.runId, + "propositionIds" to ids, + "expected" to ids.size, + ), + ) + val linked = (row?.get("linked") as? Number)?.toInt() + if (linked != ids.size) { + // The batch changed between the preflight above and this write. Read-committed means + // the two statements see two snapshots, so a proposition deleted or re-tenanted in + // between passes the check and is gone by the time the MERGE runs. The statement itself + // refuses to write a partial batch, so nothing has been written here — but this method + // has already promised all-or-nothing, so it has to say so rather than return a count + // that would read as success. The throw also rolls the transaction back, which covers + // the case where the statement's own guard is not the thing that stopped it. + throw batchChangedUnderUs(key, ids) + } + logger.debug( + "Linked {} propositions to run {} in context {}", + linked, key.runRef.runId, key.contextId.value, + ) + return linked + } + + /** + * Names what went missing during the write, by re-reading in the transaction that is about to + * roll back. + * + * The second read sees the state the write saw, so it can say which ids no longer resolve — + * which is what an operator needs and what the preflight's message would have said if the batch + * had been broken when it ran. + */ + private fun batchChangedUnderUs( + key: ExtractionRunKey, + ids: List, + ): RuntimeException { + if (!runExists(key)) return ExtractionRunNotFoundException(key) + val stillInScope = propositionsInContext(key.contextId.value, ids) + val vanished = ids.filterNot { it in stillInScope } + logger.warn( + "Linking {} propositions to run {} in context {} wrote nothing: {} still resolve", + ids.size, key.runRef.runId, key.contextId.value, stillInScope.size, + ) + return PropositionRunLinkScopeException(key, vanished.ifEmpty { ids }) + } + + @Transactional(readOnly = true) + override fun runsOf( + contextIdValue: String, + propositionId: String, + limit: Int, + ): List { + requirePositiveLimit(limit) + return queryRows( + RUNS_OF_PROPOSITION, + mapOf( + "contextId" to contextIdValue, + "propositionId" to propositionId, + "limit" to limit, + ), + ).mapNotNull { row -> row["runId"]?.toString()?.let(::ExtractionRunRef) } + } + + @Transactional(readOnly = true) + override fun propositionsOf(key: ExtractionRunKey, limit: Int): List { + requirePositiveLimit(limit) + return queryRows( + PROPOSITIONS_OF_RUN, + mapOf( + "contextId" to key.contextId.value, + "runId" to key.runRef.runId, + "limit" to limit, + ), + ).mapNotNull { row -> row["propositionId"]?.toString() } + } + + // ---- plumbing ---- + + private fun runExists(key: ExtractionRunKey): Boolean = + singleRow( + RUN_EXISTS, + mapOf("contextId" to key.contextId.value, "runId" to key.runRef.runId), + ) != null + + /** + * Which of [ids] this tenant actually holds. One round trip, whatever the batch size. + * + * This is the preflight, and its job is to *name* what is out of scope, not to decide whether + * the write may proceed — [MERGE_LINKS] decides that, in the same snapshot it writes in. + * + * `protected open` so a test can stand in the window between this check and the write, which is + * the only way to exercise the guard that closes it. Overriding it in production would weaken + * the error messages and nothing else. + */ + protected open fun propositionsInContext(contextIdValue: String, ids: List): Set = + queryRows( + PROPOSITIONS_IN_CONTEXT, + mapOf("contextId" to contextIdValue, "propositionIds" to ids), + ).mapNotNull { row -> row["propositionId"]?.toString() }.toSet() + + private fun queryRows(statement: String, bindings: Map): List> { + @Suppress("UNCHECKED_CAST") + val spec = QuerySpecification.withStatement(statement).bind(bindings) as QuerySpecification + return persistenceManager.query(spec).filterIsInstance>() + } + + private fun singleRow(statement: String, bindings: Map): Map<*, *>? = + queryRows(statement, bindings).firstOrNull() + + private fun requirePositiveLimit(limit: Int) { + require(limit > 0) { "limit must be positive, was $limit" } + } + + private companion object { + + /** Does this tenant hold this run? A seek on the run's uniqueness constraint. */ + private val RUN_EXISTS = """ + MATCH (n:ExtractionRun {contextId: ${'$'}contextId, runId: ${'$'}runId}) + RETURN {runId: n.runId} AS row + """.trimIndent() + + /** + * Which of the given proposition ids this tenant holds. + * + * The id seek uses the `Proposition(id)` uniqueness constraint and the tenant is checked on + * what comes back, rather than being part of the seek. Proposition ids are minted globally + * unique, so an id that resolves to another tenant's proposition resolves to exactly one + * node, and it is that node's tenant that decides. + */ + private val PROPOSITIONS_IN_CONTEXT = """ + MATCH (p:Proposition) + WHERE p.id IN ${'$'}propositionIds AND p.contextId = ${'$'}contextId + RETURN {propositionId: p.id} AS row + """.trimIndent() + + /** + * The write, and the authority on whether the batch is still writable. + * + * Both endpoints carry the tenant, so a cross-tenant edge is not expressible here — the + * caller has already rejected one by name, and this is the second answer to the same + * question. `MERGE` on the pattern between two already-matched nodes creates at most one + * edge, so a replay writes nothing new and counts the same. + * + * **`WHERE size(ps) = $expected` is what makes the batch atomic**, and it is not the same + * check as the caller's preflight even though it reads the same predicate. The preflight + * runs in its own statement, so under read-committed it sees an earlier snapshot: a + * proposition deleted or re-tenanted between the two passes the preflight and is gone by + * now. Counting the matches *inside the statement that writes them* closes that window — + * the count and the `MERGE` see one snapshot, so either every proposition is here and all + * the edges are written, or the row is filtered out and none of them are. The preflight + * stays, because it is what can name the ids; this is what decides. + */ + private val MERGE_LINKS = """ + MATCH (n:ExtractionRun {contextId: ${'$'}contextId, runId: ${'$'}runId}) + WITH n + MATCH (p:Proposition) + WHERE p.id IN ${'$'}propositionIds AND p.contextId = ${'$'}contextId + WITH n, collect(p) AS ps + WHERE size(ps) = ${'$'}expected + UNWIND ps AS p + MERGE (p)-[r:PRODUCED_BY_RUN]->(n) + WITH count(r) AS linked + RETURN {linked: linked} AS row + """.trimIndent() + + /** + * Which runs produced this claim, in one tenant. + * + * Anchored on the proposition's id constraint, with the tenant asserted on both ends so a + * read can never cross. `DISTINCT` before the limit, because a page that spent a slot on a + * repeated run id would come back short. + */ + private val RUNS_OF_PROPOSITION = """ + MATCH (p:Proposition {id: ${'$'}propositionId})-[:PRODUCED_BY_RUN]->(n:ExtractionRun) + WHERE p.contextId = ${'$'}contextId AND n.contextId = ${'$'}contextId + WITH DISTINCT n.runId AS runId ORDER BY runId ASC + LIMIT ${'$'}limit + RETURN {runId: runId} AS row + """.trimIndent() + + /** + * The inverse: which claims this run produced. + * + * Anchored on the run so the expansion starts from one node rather than from a tenant's + * worth of propositions, and scoped before the limit — the tenant is in the pattern, not a + * filter applied to a page that was already cut. + */ + private val PROPOSITIONS_OF_RUN = """ + MATCH (n:ExtractionRun {contextId: ${'$'}contextId, runId: ${'$'}runId})<-[:PRODUCED_BY_RUN]-(p:Proposition) + WHERE p.contextId = ${'$'}contextId + WITH DISTINCT p.id AS propositionId ORDER BY propositionId ASC + LIMIT ${'$'}limit + RETURN {propositionId: propositionId} AS row + """.trimIndent() + } +} diff --git a/dice-storage/src/main/kotlin/com/embabel/dice/storage/ExtractionRunSchema.kt b/dice-storage/src/main/kotlin/com/embabel/dice/storage/ExtractionRunSchema.kt index e0302297..f2b32945 100644 --- a/dice-storage/src/main/kotlin/com/embabel/dice/storage/ExtractionRunSchema.kt +++ b/dice-storage/src/main/kotlin/com/embabel/dice/storage/ExtractionRunSchema.kt @@ -50,6 +50,21 @@ object ExtractionRunSchema { /** Header to the terminal write that ended it. */ const val ENDED_BY_REL: String = "ENDED_BY" + /** + * A claim to the run that produced it: `(:Proposition)-[:PRODUCED_BY_RUN]->(:ExtractionRun)`. + * + * Named for the run rather than left as a bare `PRODUCED_BY`, because at least three things in + * DICE produce a proposition — an extraction run, a collector run, and later a commit — and each + * will want to say so. The target label disambiguates a pattern; the name has to disambiguate a + * grep. + * + * It adds nothing to [specs]. Both endpoint labels already carry the uniqueness constraints + * these statements seek on, and a relationship has no key of its own: `MERGE` on a pattern + * between two matched nodes creates at most one edge, whoever else is writing. See + * [DrivinePropositionRunLinkStore]. + */ + const val PRODUCED_BY_RUN_REL: String = "PRODUCED_BY_RUN" + /** * The longest tenant id this store will write. * 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 d57c9c74..8afa3ffd 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 @@ -1512,6 +1512,49 @@ abstract class AbstractExtractionRunStoreContractTest { assertEquals(1, store.invocationsOf(run.key()).count { it.id == id }) } + @Test + fun `a run with several attempts reads back equal to the run that was saved`() { + // The case that catches a backend disagreeing with the reference on invocation order. A + // durable store keeps identified rows and returns them in plan order; an in-memory one used + // to return the order the caller listed. Since `equals` compares the list, a run whose + // attempts arrived out of plan order came back unequal from one backend and equal from the + // other. `ExtractionRun` normalizes to plan order at construction, so both agree — and one + // attempt per call, which is all the rest of this suite uses, could never show it. + // + // The attempts arrive through recordInvocation, the door that owns invocation state. A + // header save carries none of them, so arrival order here is the order of the calls. + val store = store() + val arrivalOrder = listOf( + ExtractionInvocationRecord(id = ExtractionInvocationId(1, 2)), + ExtractionInvocationRecord(id = ExtractionInvocationId(0, 1)), + ExtractionInvocationRecord(id = ExtractionInvocationId(2, 1)), + ExtractionInvocationRecord(id = ExtractionInvocationId(1, 1)), + ) + val run = ExtractionRun( + contextId = tenant, + lineage = ExtractionRunLineage.root(ExtractionRunRef("contract-multi-attempt")), + status = ExtractionRunStatus.RUNNING, + startedAt = startedAt, + invocations = arrivalOrder, + ) + + store.save(run) + arrivalOrder.forEach { store.recordInvocation(run.key(), it) } + val read = store.findRun(run.key()) + + assertEquals(run, read) + assertEquals( + listOf( + ExtractionInvocationId(0, 1), + ExtractionInvocationId(1, 1), + ExtractionInvocationId(1, 2), + ExtractionInvocationId(2, 1), + ), + read?.invocations?.map { it.id }, + ) + assertEquals(read?.invocations, store.invocationsOf(run.key())) + } + @Test fun `recording against a run nobody started is rejected`() { val store = store() diff --git a/dice-storage/src/test/kotlin/com/embabel/dice/storage/AbstractPropositionRunLinkStoreContractTest.kt b/dice-storage/src/test/kotlin/com/embabel/dice/storage/AbstractPropositionRunLinkStoreContractTest.kt new file mode 100644 index 00000000..7de0e0c5 --- /dev/null +++ b/dice-storage/src/test/kotlin/com/embabel/dice/storage/AbstractPropositionRunLinkStoreContractTest.kt @@ -0,0 +1,319 @@ +/* + * 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.EntityMention +import com.embabel.dice.proposition.MentionRole +import com.embabel.dice.proposition.Proposition +import com.embabel.dice.proposition.extraction.ExtractionRun +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.PropositionRunLinkScopeException +import com.embabel.dice.proposition.extraction.PropositionRunLinkStore +import org.junit.jupiter.api.Assertions.assertEquals +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 [PropositionRunLinkStore]: the tenant guard on the write, idempotency, + * and the two bounded reads. Each subclass supplies a store already holding the fixtures below, so a + * backend that disagrees with the in-memory reference fails at authoring time. + * + * The cases here are the ones a backend gets wrong in a way a single-backend test would miss. A + * write that MERGEs without resolving its endpoints accepts a cross-tenant link silently. A read + * that scopes on the run id and forgets the tenant returns a neighbour's rows. A batch that writes + * as it validates leaves half a link set behind when one id is rejected. + */ +abstract class AbstractPropositionRunLinkStoreContractTest { + + protected val tenant: ContextId = ContextId("link-tenant") + protected val neighbour: ContextId = ContextId("link-neighbour") + protected val startedAt: Instant = Instant.parse("2026-08-31T10:15:30Z") + + /** Runs the fixture stores under both tenants, by id. */ + protected val fixtureRunIds: List = listOf("link-run-a", "link-run-b") + + /** + * Propositions the fixture stores in [tenant]. + * + * The neighbour's are separate ids rather than the same ones, because a proposition id is + * globally unique — one id is one proposition in one tenant, and the graph carries a uniqueness + * constraint saying so. Run ids are the opposite: host-minted and tenant-qualified, so both + * tenants hold [fixtureRunIds], which is the arrangement that catches a query scoping on the run + * id and forgetting the tenant. + */ + protected val fixturePropositionIds: List = listOf("prop-1", "prop-2", "prop-3") + + /** Propositions the fixture stores in [neighbour]. */ + protected val neighbourPropositionIds: List = listOf("nb-prop-1", "nb-prop-2") + + /** + * A proposition in [tenant] that exactly one test deletes. + * + * It gets its own id because a backend may not be able to put a deleted proposition back: on + * Drivine, deleting a `:Proposition` with raw Cypher and re-saving the same id through the + * repository returns a node with a null `contextId`. One disposable fixture is cheaper than + * making every backend's seeding re-entrant. + */ + protected val disposablePropositionId: String = "prop-disposable" + + /** + * Removes a proposition from whatever store backs this store's proposition end, as a host + * deleting a claim would. + * + * Deletion is not on [PropositionRunLinkStore] — a lineage store does not own propositions — so + * the suite needs each backend to do it, and then holds both to the same answer about what the + * reads say afterwards. + */ + protected abstract fun deleteProposition(id: String) + + /** + * A store whose backing graph already holds [fixtureRunIds] under both tenants, + * [fixturePropositionIds] in [tenant], [neighbourPropositionIds] in [neighbour], and no links. + */ + protected abstract fun store(): PropositionRunLinkStore + + protected fun run(runId: String, contextId: ContextId): ExtractionRun = ExtractionRun( + contextId = contextId, + lineage = ExtractionRunLineage.root(ExtractionRunRef(runId)), + status = ExtractionRunStatus.RUNNING, + startedAt = startedAt, + ) + + protected fun proposition(id: String, contextId: ContextId): Proposition = Proposition( + id = id, + contextId = contextId, + text = "$id in ${contextId.value}", + mentions = listOf( + EntityMention(span = "Alice", type = "Person", resolvedId = "e-alice", role = MentionRole.SUBJECT), + ), + confidence = 0.9, + grounding = listOf("chunk-1"), + ) + + private fun key(runId: String, contextId: ContextId = tenant) = + ExtractionRunKey(contextId, ExtractionRunRef(runId)) + + // ---- the write ---- + + @Test + fun `a link is readable from both ends`() { + val store = store() + + assertEquals(2, store.link(key("link-run-a"), listOf("prop-1", "prop-2"))) + + assertEquals(listOf("prop-1", "prop-2"), store.propositionsOf(key("link-run-a"), 10)) + assertEquals( + listOf(ExtractionRunRef("link-run-a")), + store.runsOf(tenant, "prop-1", 10), + ) + } + + @Test + fun `linking is idempotent and counts the same on a replay`() { + val store = store() + + val first = store.link(key("link-run-a"), listOf("prop-1", "prop-2")) + val second = store.link(key("link-run-a"), listOf("prop-1", "prop-2")) + + assertEquals(first, second) + assertEquals(listOf("prop-1", "prop-2"), store.propositionsOf(key("link-run-a"), 10)) + } + + @Test + fun `a repeated id in one call is one link`() { + val store = store() + + assertEquals(1, store.link(key("link-run-a"), listOf("prop-1", "prop-1", "prop-1"))) + assertEquals(listOf("prop-1"), store.propositionsOf(key("link-run-a"), 10)) + } + + @Test + fun `one proposition links to many runs and one run links to many propositions`() { + // The many-to-many claim, which is what makes re-extraction expressible at all. + val store = store() + + store.link(key("link-run-a"), listOf("prop-1", "prop-2")) + store.link(key("link-run-b"), listOf("prop-1", "prop-3")) + + assertEquals( + listOf(ExtractionRunRef("link-run-a"), ExtractionRunRef("link-run-b")), + store.runsOf(tenant, "prop-1", 10), + ) + assertEquals(listOf("prop-1", "prop-2"), store.propositionsOf(key("link-run-a"), 10)) + assertEquals(listOf("prop-1", "prop-3"), store.propositionsOf(key("link-run-b"), 10)) + } + + @Test + fun `an empty batch is a no-op`() { + val store = store() + + assertEquals(0, store.link(key("link-run-a"), emptyList())) + assertTrue(store.propositionsOf(key("link-run-a"), 10).isEmpty()) + } + + @Test + fun `the single-proposition convenience writes the same link`() { + val store = store() + + assertEquals(1, store.link(key("link-run-a"), "prop-2")) + assertEquals(listOf("prop-2"), store.propositionsOf(key("link-run-a"), 10)) + } + + // ---- the tenant guard ---- + + @Test + fun `a link against a run this tenant does not hold is rejected`() { + val store = store() + + assertThrows(ExtractionRunNotFoundException::class.java) { + store.link(key("link-run-absent"), listOf("prop-1")) + } + assertTrue(store.runsOf(tenant, "prop-1", 10).isEmpty()) + } + + @Test + fun `a run cannot claim a neighbour's proposition`() { + // `nb-prop-1` exists — it is just someone else's. That is the case a backend that MERGEs + // without resolving its endpoints gets wrong, and it is not the same as an id nobody holds. + val store = store() + + val rejected = assertThrows(PropositionRunLinkScopeException::class.java) { + store.link(key("link-run-a"), listOf("nb-prop-1")) + } + + assertEquals(listOf("nb-prop-1"), rejected.propositionIds) + assertTrue(store.propositionsOf(key("link-run-a"), 10).isEmpty()) + assertTrue(store.runsOf(tenant, "nb-prop-1", 10).isEmpty()) + } + + @Test + fun `a link naming a proposition nobody holds is rejected and writes nothing`() { + val store = store() + + val rejected = assertThrows(PropositionRunLinkScopeException::class.java) { + store.link(key("link-run-a"), listOf("prop-1", "prop-nowhere")) + } + + assertEquals(listOf("prop-nowhere"), rejected.propositionIds) + // Nothing partial: the good id in the same batch was not written either. + assertTrue(store.propositionsOf(key("link-run-a"), 10).isEmpty()) + } + + // ---- bounded, scoped reads ---- + + @Test + fun `both reads are bounded and ordered by id`() { + val store = store() + store.link(key("link-run-a"), listOf("prop-3", "prop-1", "prop-2")) + store.link(key("link-run-b"), listOf("prop-1")) + + assertEquals(listOf("prop-1", "prop-2"), store.propositionsOf(key("link-run-a"), 2)) + assertEquals( + listOf(ExtractionRunRef("link-run-a")), + store.runsOf(tenant, "prop-1", 1), + ) + } + + @Test + fun `both reads reject a limit that is not positive`() { + val store = store() + + listOf(0, -1).forEach { limit -> + assertThrows(IllegalArgumentException::class.java) { + store.propositionsOf(key("link-run-a"), limit) + } + assertThrows(IllegalArgumentException::class.java) { + store.runsOf(tenant, "prop-1", limit) + } + } + } + + @Test + fun `every read fails closed across tenants`() { + // Both tenants hold a run called `link-run-a`. Only the neighbour's has links, and nothing + // this tenant asks returns them. + val store = store() + store.link(key("link-run-a", neighbour), listOf("nb-prop-1", "nb-prop-2")) + + assertTrue(store.propositionsOf(key("link-run-a", tenant), 10).isEmpty()) + assertTrue(store.runsOf(tenant, "nb-prop-1", 10).isEmpty()) + assertEquals( + listOf("nb-prop-1", "nb-prop-2"), + store.propositionsOf(key("link-run-a", neighbour), 10), + ) + assertEquals( + listOf(ExtractionRunRef("link-run-a")), + store.runsOf(neighbour, "nb-prop-1", 10), + ) + } + + @Test + fun `link reports exactly the ids it was given`() { + // The number is the batch's own size, on the first write and on a replay. A backend that + // returned "how many were new" would report 2 then 0 and invite a caller to read 0 as + // failure; one that linked a subset and returned that subset's size would be reporting a + // partial write as a success. + val store = store() + + assertEquals(2, store.link(key("link-run-a"), listOf("prop-1", "prop-2"))) + assertEquals(2, store.link(key("link-run-a"), listOf("prop-1", "prop-2"))) + assertEquals(3, store.link(key("link-run-a"), listOf("prop-1", "prop-2", "prop-3"))) + } + + @Test + fun `a link to a deleted proposition disappears from both reads`() { + // A link is about a claim that exists. Delete the claim and the lineage goes with it: on a + // graph because the edge is detached with the node, and a reference implementation has to + // agree rather than keep answering from a map the deletion never reached. Otherwise + // `runsOf` outlives its subject and the audit reports lineage for something the store no + // longer holds. + val store = store() + store.link(key("link-run-a"), listOf(disposablePropositionId, "prop-1")) + + assertEquals( + listOf(ExtractionRunRef("link-run-a")), + store.runsOf(tenant, disposablePropositionId, 10), + ) + assertTrue(disposablePropositionId in store.propositionsOf(key("link-run-a"), 10)) + + deleteProposition(disposablePropositionId) + + assertTrue( + store.runsOf(tenant, disposablePropositionId, 10).isEmpty(), + "a deleted proposition has no runs", + ) + assertEquals( + listOf("prop-1"), + store.propositionsOf(key("link-run-a"), 10), + "the run keeps the claims that still exist and loses the one that does not", + ) + } + + @Test + fun `a read for an unknown proposition or run is empty rather than an error`() { + val store = store() + + assertTrue(store.runsOf(tenant, "prop-nowhere", 10).isEmpty()) + assertTrue(store.propositionsOf(key("link-run-absent"), 10).isEmpty()) + } +} diff --git a/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivinePropositionRunLinkStoreContractIntegrationTest.kt b/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivinePropositionRunLinkStoreContractIntegrationTest.kt new file mode 100644 index 00000000..94d9f87e --- /dev/null +++ b/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivinePropositionRunLinkStoreContractIntegrationTest.kt @@ -0,0 +1,148 @@ +/* + * 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.ExtractionRunKey +import com.embabel.dice.proposition.extraction.ExtractionRunRef +import com.embabel.dice.proposition.extraction.PropositionRunLinkStore +import org.drivine.manager.PersistenceManager +import org.drivine.query.QuerySpecification +import org.junit.jupiter.api.AfterAll +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.BeforeAll +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.TestInstance +import org.springframework.beans.factory.annotation.Autowired +import org.springframework.boot.test.context.SpringBootTest + +/** + * The Drivine store against the same cross-backend contract the in-memory reference passes. + * + * A real graph is where the tenant guard has to hold: the run store and the proposition repository + * are separate aggregates, and the link is the one write in this train that touches both. + * + * **The endpoint nodes are seeded once and only the links are cleared between tests.** Deleting a + * `:Proposition` with raw Cypher and then re-saving the same id through `DrivinePropositionRepository` + * does not produce the node again as written: the second save comes back with a null `contextId`, + * because the repository's object manager still holds the node this class deleted behind its back. + * Seeding per test would therefore leave every test after the first with untenanted propositions, + * and every scoped read would correctly return nothing. Links are relationships and have no such + * problem, so those are what each test starts from a clean slate on. + */ +@SpringBootTest(classes = [TestApplication::class]) +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +class DrivinePropositionRunLinkStoreContractIntegrationTest : AbstractPropositionRunLinkStoreContractTest() { + + @Autowired + private lateinit var linkStore: DrivinePropositionRunLinkStore + + @Autowired + private lateinit var runStore: DrivineExtractionRunStore + + @Autowired + private lateinit var repository: DrivinePropositionRepository + + @Autowired + private lateinit var persistenceManager: PersistenceManager + + @BeforeAll + fun seed() { + wipe() + listOf(tenant, neighbour).forEach { context -> + fixtureRunIds.forEach { runStore.save(run(it, context)) } + } + (fixturePropositionIds + disposablePropositionId).forEach { + repository.save(proposition(it, tenant)) + } + neighbourPropositionIds.forEach { repository.save(proposition(it, neighbour)) } + } + + @BeforeEach + fun clearLinks() { + persistenceManager.execute( + QuerySpecification.withStatement( + "MATCH ()-[r:${ExtractionRunSchema.PRODUCED_BY_RUN_REL}]->() DELETE r", + ), + ) + } + + @AfterAll + fun cleanUp() { + wipe() + } + + private fun wipe() { + (ExtractionRunSchema.LABELS + listOf("Proposition", "Mention", "Source")).forEach { label -> + persistenceManager.execute(QuerySpecification.withStatement("MATCH (n:$label) DETACH DELETE n")) + } + } + + override fun deleteProposition(id: String) { + repository.delete(id) + } + + override fun store(): PropositionRunLinkStore = linkStore + + /** + * Linking the same claim to the same run twice leaves one edge in the graph. + * + * The contract suite already says a replay reports the same count, but a count is what the + * *store* believes. This counts the relationships themselves, because the failure being guarded + * against is precisely one where both answers look right and the graph holds two edges: every + * read here returns run refs and ids, so a duplicate edge is invisible to all of them and shows + * up later as inflated audit answers. + * + * The write is a `MERGE` between two already-matched nodes, which is what makes this hold under + * concurrent writers as well — Neo4j locks the endpoints when the merge decides to create. + */ + @Test + fun `linking the same pair twice leaves exactly one edge`() { + val key = ExtractionRunKey(tenant, ExtractionRunRef(fixtureRunIds.first())) + val propositionId = fixturePropositionIds.first() + + val first = linkStore.link(key, listOf(propositionId)) + val second = linkStore.link(key, listOf(propositionId)) + + assertEquals(first, second, "a replay reports what the first write did") + assertEquals( + 1L, + edgeCount(propositionId, key.runRef.runId), + "the graph holds exactly one PRODUCED_BY_RUN edge for the pair", + ) + + // The same claim named twice inside a single batch is also one edge. + linkStore.link(key, listOf(propositionId, propositionId)) + assertEquals( + 1L, + edgeCount(propositionId, key.runRef.runId), + "a duplicate inside one batch is still one edge", + ) + } + + /** How many `PRODUCED_BY_RUN` edges join this claim to this run, counted in the graph itself. */ + private fun edgeCount(propositionId: String, runId: String): Long = persistenceManager.getOne( + QuerySpecification.withStatement( + """ + MATCH (p:Proposition {id: ${'$'}propositionId}) + -[r:${ExtractionRunSchema.PRODUCED_BY_RUN_REL}]-> + (n:ExtractionRun {contextId: ${'$'}contextId, runId: ${'$'}runId}) + RETURN count(r) AS c + """.trimIndent(), + ).bind(mapOf("propositionId" to propositionId, "contextId" to tenant.value, "runId" to runId)) + .transform(Long::class.java), + ) +} diff --git a/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineRunLineageIntegrationTest.kt b/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineRunLineageIntegrationTest.kt new file mode 100644 index 00000000..b1275ec3 --- /dev/null +++ b/dice-storage/src/test/kotlin/com/embabel/dice/storage/DrivineRunLineageIntegrationTest.kt @@ -0,0 +1,401 @@ +/* + * 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.EntityMention +import com.embabel.dice.proposition.MentionRole +import com.embabel.dice.proposition.Proposition +import com.embabel.dice.proposition.extraction.ExtractionRun +import com.embabel.dice.proposition.extraction.ExtractionRunKey +import com.embabel.dice.proposition.extraction.ExtractionRunLineage +import com.embabel.dice.proposition.extraction.ExtractionRunRef +import com.embabel.dice.proposition.extraction.ExtractionRunStatus +import com.embabel.dice.proposition.extraction.PropositionRunLinkScopeException +import com.embabel.dice.provenance.ProvenanceEntry +import com.embabel.dice.provenance.UriLocator +import org.drivine.manager.PersistenceManager +import org.drivine.query.QuerySpecification +import org.junit.jupiter.api.AfterEach +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertFalse +import org.junit.jupiter.api.Assertions.assertThrows +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test +import org.springframework.beans.factory.annotation.Autowired +import org.springframework.boot.test.context.SpringBootTest +import org.springframework.transaction.PlatformTransactionManager +import org.springframework.transaction.support.TransactionTemplate +import java.time.Instant + +/** + * The invariant the whole slice exists for, measured on a real graph: **two extraction runs over + * identical content leave one proposition, one source grounding, and two run links.** + * + * Every part of it is a separate way to get this wrong. Two propositions means dedup did not fire or + * the second run's canonical id was not consumed. Two `DERIVED_FROM` edges means the run got folded + * into source identity. One run link means the relation cannot hold both runs, and the second run's + * contribution — that it confirmed a claim it did not create — is unrecorded. + */ +@SpringBootTest(classes = [TestApplication::class]) +class DrivineRunLineageIntegrationTest { + + @Autowired + private lateinit var linkStore: DrivinePropositionRunLinkStore + + @Autowired + private lateinit var runStore: DrivineExtractionRunStore + + @Autowired + private lateinit var repository: DrivinePropositionRepository + + @Autowired + private lateinit var persistenceManager: PersistenceManager + + @Autowired + private lateinit var transactionManager: PlatformTransactionManager + + private val tenant = ContextId("lineage-tenant") + private val startedAt: Instant = Instant.parse("2026-08-31T10:15:30Z") + + @AfterEach + fun cleanUp() { + (ExtractionRunSchema.LABELS + listOf("Proposition", "Mention", "Source")).forEach { label -> + persistenceManager.execute(QuerySpecification.withStatement("MATCH (n:$label) DETACH DELETE n")) + } + } + + private fun run(runId: String, contextId: ContextId = tenant) = ExtractionRun( + contextId = contextId, + lineage = ExtractionRunLineage.root(ExtractionRunRef(runId)), + status = ExtractionRunStatus.RUNNING, + startedAt = startedAt, + ) + + /** + * What one extraction of one sentence produces. + * + * Every test names its own [subject], and so its own text and source, because ids and source + * keys are not reusable across tests here: deleting a `:Proposition` with raw Cypher and then + * re-saving the same id through `DrivinePropositionRepository` comes back with a null + * `contextId`, since the repository's object manager still holds the node this class deleted + * behind its back. Two calls with the same [subject] inside one test are the point — that is the + * re-extraction the dedup rule collapses. + */ + private fun extracted(id: String, subject: String) = Proposition( + id = id, + contextId = tenant, + text = "The $subject team ships on Friday", + mentions = listOf( + EntityMention(span = "the team", type = "Person", role = MentionRole.SUBJECT), + ), + confidence = 0.9, + grounding = listOf("chunk-1"), + provenanceEntries = listOf(ProvenanceEntry(locator = locatorFor(subject), chunkId = "chunk-1")), + ) + + private fun locatorFor(subject: String) = UriLocator("https://example.com/$subject.txt") + + private fun count(statement: String, bindings: Map = emptyMap()): Long = + persistenceManager.getOne( + QuerySpecification.withStatement(statement).bind(bindings).transform(Long::class.java), + ) ?: 0L + + @Test + fun `two identical-content runs leave one proposition, one source grounding, and two run links`() { + val first = run("lineage-run-1") + val second = run("lineage-run-2") + runStore.save(first) + runStore.save(second) + + // Run 1 extracts the sentence and stores it. Run 2 extracts the same sentence, mints its own + // id, and the repository answers with the proposition it already holds. + val canonical = repository.save(extracted("minted-by-run-1", "proof")) + val secondPass = repository.save(extracted("minted-by-run-2", "proof")) + + assertEquals(canonical.id, secondPass.id, "the second run's insert deduplicates onto the first") + + linkStore.link(first.key(), listOf(canonical.id)) + linkStore.link(second.key(), listOf(secondPass.id)) + + assertEquals( + 1L, + count("MATCH (p:Proposition {contextId: \$c}) RETURN count(p) AS c", mapOf("c" to tenant.value)), + "one proposition", + ) + assertEquals( + 1L, + count( + "MATCH (:Proposition {id: \$id})-[r:DERIVED_FROM]->(:Source {key: \$key}) RETURN count(r) AS c", + mapOf("id" to canonical.id, "key" to locatorFor("proof").key()), + ), + "one source grounding — run identity never enters source provenance, so the second " + + "run's identical evidence is the same evidence", + ) + assertEquals( + 2L, + count( + "MATCH (:Proposition {id: \$id})-[r:PRODUCED_BY_RUN]->(:ExtractionRun) RETURN count(r) AS c", + mapOf("id" to canonical.id), + ), + "two run links — both runs produced this claim, and the relation holds both", + ) + + // And the same three facts through the store's own reads. + assertEquals( + listOf(ExtractionRunRef("lineage-run-1"), ExtractionRunRef("lineage-run-2")), + linkStore.runsOf(tenant, canonical.id, 10), + ) + assertEquals(listOf(canonical.id), linkStore.propositionsOf(first.key(), 10)) + assertEquals(listOf(canonical.id), linkStore.propositionsOf(second.key(), 10)) + } + + @Test + fun `the store carries the experimental marker its own compatibility note promises`() { + // Annotations on an interface do not reach its implementations, so + // PropositionRunLinkStore being marked says nothing about this class — and this class is + // what a host declares as a bean and reads the KDoc of. The marker has class retention, so + // this reads the class file rather than reflecting. + val name = DrivinePropositionRunLinkStore::class.java.name.replace('.', '/') + ".class" + val bytes = checkNotNull( + DrivinePropositionRunLinkStore::class.java.classLoader.getResourceAsStream(name), + ).use { it.readBytes() } + + assertTrue( + String(bytes, Charsets.ISO_8859_1) + .contains("Lorg/jetbrains/annotations/ApiStatus\$Experimental;"), + "DrivinePropositionRunLinkStore is not marked experimental", + ) + } + + @Test + fun `the link is one edge however many times it is written`() { + val theRun = run("lineage-idempotent") + runStore.save(theRun) + val stored = repository.save(extracted("minted-idempotent", "idempotent")) + + repeat(3) { linkStore.link(theRun.key(), listOf(stored.id)) } + + assertEquals( + 1L, + count( + "MATCH (:Proposition {id: \$id})-[r:PRODUCED_BY_RUN]->(:ExtractionRun) RETURN count(r) AS c", + mapOf("id" to stored.id), + ), + ) + } + + @Test + fun `the edge lands between the two nodes the schema names`() { + val theRun = run("lineage-shape") + runStore.save(theRun) + val stored = repository.save(extracted("minted-shape", "shape")) + + linkStore.link(theRun.key(), listOf(stored.id)) + + assertEquals( + 1L, + count( + """ + MATCH (p:Proposition {id: ${'$'}id, contextId: ${'$'}c}) + -[:${ExtractionRunSchema.PRODUCED_BY_RUN_REL}]-> + (n:ExtractionRun {contextId: ${'$'}c, runId: ${'$'}runId}) + RETURN count(*) AS c + """.trimIndent(), + mapOf("id" to stored.id, "c" to tenant.value, "runId" to "lineage-shape"), + ), + ) + // The edge carries nothing. A bare edge is what lets the write be a plain MERGE. + assertEquals( + 0L, + count( + "MATCH (:Proposition {id: \$id})-[r:PRODUCED_BY_RUN]->() RETURN count(keys(r)[0]) AS c", + mapOf("id" to stored.id), + ), + ) + } + + @Test + fun `lineage inside a caller's transaction sees its writes and cannot condemn it`() { + // Both halves of how `link` behaves when a host wraps extraction in its own transaction. + // + // **It participates.** The proposition saved a line earlier has not committed, and the link + // still resolves it — so lineage and claims commit together or roll back together, which is + // what a host wrapping extraction would want. + // + // **An application-level failure does not condemn the caller.** Spring marks a + // participating transaction rollback-only when the inner method throws, and it does so + // here — Drivine overrides `doSetRollbackOnly` and the shared transaction object's flag is + // set. The commit survives because that flag is write-only: `DrivineTransactionObject` does + // not implement `SmartTransactionObject`, so Spring's `isGlobalRollbackOnly` cannot see it, + // and Drivine never reads it in `doCommit`. Propagated, then dropped. + // + // So `isRollbackOnly` below cannot fail today, and the teeth of this test are the + // post-commit counts. Both are kept deliberately: the flag assertion trips if Drivine + // implements `SmartTransactionObject`, and the counts trip if it starts reading the flag in + // `doCommit`. Either way this goes red before a host discovers it as lost extractions. + // + // Only application-level failures are covered. See the server-side case below. + val theRun = run("lineage-ambient") + runStore.save(theRun) + + TransactionTemplate(transactionManager).execute { status -> + val stored = repository.save(extracted("minted-ambient", "ambient")) + + assertEquals( + 1, + linkStore.link(theRun.key(), listOf(stored.id)), + "the link resolves a proposition this transaction has not committed yet", + ) + + // Exactly what recordRunLineage does: attempt, swallow, carry on. + runCatching { linkStore.link(theRun.key(), listOf("prop-nobody-holds")) } + + assertFalse( + status.isRollbackOnly, + "a failed link must not have condemned the caller's transaction", + ) + } + + assertEquals( + 1L, + count( + "MATCH (p:Proposition {id: \$id}) RETURN count(p) AS c", + mapOf("id" to "minted-ambient"), + ), + "the caller's own write committed", + ) + assertEquals( + 1L, + count("MATCH ()-[r:PRODUCED_BY_RUN]->() RETURN count(r) AS c"), + "and so did the lineage written alongside it", + ) + } + + @Test + fun `a server-side failure inside a caller's transaction is the window best-effort does not cover`() { + // The limit of the non-condemnation guarantee, asserted rather than left to be discovered. + // + // Every failure `link` raises on its own — tenant guard, run-not-found, scope rejection, + // batch-changed — is thrown from Kotlin *after* its statements succeeded, so the Bolt + // transaction is healthy and the caller can carry on. A statement that fails at the server + // is a different thing: it terminates the transaction underneath Spring, and no catch in + // `recordRunLineage` can undo that. A deadlock between two runs linking overlapping + // propositions is the realistic version. + // + // This test injects that failure on the caller's thread-bound transaction and shows what it costs, so the + // shipped guarantee and this test agree. Slice 10's commit-claims-before-lineage is what + // closes it; when that lands, this test is the one that should change. + val theRun = run("lineage-poison") + runStore.save(theRun) + + val poisoning = object : DrivinePropositionRunLinkStore(persistenceManager) { + override fun propositionsInContext(contextIdValue: String, ids: List): Set { + // Rejected by the server, on the transaction the caller is using. + persistenceManager.execute( + QuerySpecification.withStatement("MATCH (n:Proposition) RETURN n.id +"), + ) + return super.propositionsInContext(contextIdValue, ids) + } + } + + val stored = repository.save(extracted("minted-poison", "poison")) + + val outcome = runCatching { + TransactionTemplate(transactionManager).execute { + // Best-effort, exactly as recordRunLineage does it: attempt, swallow, carry on. + runCatching { poisoning.link(theRun.key(), listOf(stored.id)) } + // The caller believes it may keep working. It may not. + repository.save(extracted("minted-after-poison", "after-poison")) + } + } + + assertTrue( + outcome.isFailure, + "a server-side failure is not survivable by catching: the caller's transaction is " + + "already terminated, and this is the residual window the docs must name", + ) + assertEquals( + 0L, + count( + "MATCH (p:Proposition {id: \$id}) RETURN count(p) AS c", + mapOf("id" to "minted-after-poison"), + ), + "and the caller's later write is lost with it", + ) + } + + @Test + fun `a batch that changes under the write links nothing and says so`() { + // The window the preflight cannot close on its own: validation and the MERGE are separate + // statements, so under read-committed a proposition can be deleted between them. It passes + // the check and is gone by the time the write runs. + // + // Standing in that window needs a seam, so this store overrides the preflight to delete one + // proposition *after* reporting both as in scope — exactly what a concurrent writer would + // have done, and deterministic. Without the count guard inside MERGE_LINKS the surviving + // proposition would be linked and the partial batch would commit as a success. + val theRun = run("lineage-toctou") + runStore.save(theRun) + val survives = repository.save(extracted("minted-survives", "survives")) + val vanishes = repository.save(extracted("minted-vanishes", "vanishes")) + + val racing = object : DrivinePropositionRunLinkStore(persistenceManager) { + override fun propositionsInContext(contextIdValue: String, ids: List): Set { + val seen = super.propositionsInContext(contextIdValue, ids) + // The interference, after the check has passed and before the write. + persistenceManager.execute( + QuerySpecification + .withStatement("MATCH (p:Proposition {id: \$id}) DETACH DELETE p") + .bind(mapOf("id" to vanishes.id)), + ) + return seen + } + } + + val rejected = assertThrows(PropositionRunLinkScopeException::class.java) { + racing.link(theRun.key(), listOf(survives.id, vanishes.id)) + } + + assertEquals(listOf(vanishes.id), rejected.propositionIds, "the message names what vanished") + assertEquals( + 0L, + count("MATCH ()-[r:PRODUCED_BY_RUN]->() RETURN count(r) AS c"), + "nothing partial commits: the proposition that survived is not linked either", + ) + } + + @Test + fun `deleting a proposition takes its run links with it and leaves the run standing`() { + val theRun = run("lineage-delete") + runStore.save(theRun) + val stored = repository.save(extracted("minted-delete", "delete")) + linkStore.link(theRun.key(), listOf(stored.id)) + + repository.delete(stored.id) + + assertEquals( + 0L, + count("MATCH ()-[r:PRODUCED_BY_RUN]->() RETURN count(r) AS c"), + "no lineage edge may outlive its proposition", + ) + assertEquals( + ExtractionRunStatus.RUNNING, + runStore.findRun(ExtractionRunKey(tenant, ExtractionRunRef("lineage-delete")))?.status, + "the run is an audit row and survives the claim being deleted", + ) + } +} diff --git a/dice-storage/src/test/kotlin/com/embabel/dice/storage/InMemoryPropositionRunLinkStoreContractTest.kt b/dice-storage/src/test/kotlin/com/embabel/dice/storage/InMemoryPropositionRunLinkStoreContractTest.kt new file mode 100644 index 00000000..9d417d3a --- /dev/null +++ b/dice-storage/src/test/kotlin/com/embabel/dice/storage/InMemoryPropositionRunLinkStoreContractTest.kt @@ -0,0 +1,189 @@ +/* + * 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.ExtractionRunKey +import com.embabel.dice.proposition.extraction.ExtractionRunRef +import com.embabel.dice.proposition.extraction.ExtractionRunStatus +import com.embabel.dice.proposition.extraction.ExtractionRunTransition +import com.embabel.dice.proposition.extraction.InMemoryExtractionRunStore +import com.embabel.dice.proposition.extraction.InMemoryPropositionRunLinkStore +import com.embabel.dice.proposition.extraction.PropositionRunLinkStore +import com.embabel.dice.proposition.store.InMemoryPropositionRepository +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertThrows +import org.junit.jupiter.api.Assertions.assertTrue +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test +import java.time.Instant +import java.util.concurrent.CopyOnWriteArrayList +import java.util.concurrent.CountDownLatch +import java.util.concurrent.Executors +import java.util.concurrent.TimeUnit + +/** + * The reference implementation against the cross-backend contract. It is the executable statement + * of what the Drivine store is held to, so it runs the same suite from the same fixtures. + */ +class InMemoryPropositionRunLinkStoreContractTest : AbstractPropositionRunLinkStoreContractTest() { + + private lateinit var runs: InMemoryExtractionRunStore + private lateinit var propositions: InMemoryPropositionRepository + + override fun store(): PropositionRunLinkStore = store(maxRuns = 10_000) + + private fun store(maxRuns: Int): InMemoryPropositionRunLinkStore { + runs = InMemoryExtractionRunStore() + propositions = InMemoryPropositionRepository() + listOf(tenant, neighbour).forEach { context -> + fixtureRunIds.forEach { runs.save(run(it, context)) } + } + (fixturePropositionIds + disposablePropositionId).forEach { + propositions.save(proposition(it, tenant)) + } + neighbourPropositionIds.forEach { propositions.save(proposition(it, neighbour)) } + return InMemoryPropositionRunLinkStore(runs, propositions, maxRuns) + } + + /** + * Past the cap the store forgets the links of the run it linked earliest, once that run has + * ended. A run still running keeps its links even when it is the oldest, so the cap is a bound + * on finished lineage, not a way to lose a run that is still being attributed to. + */ + @Test + fun `past the cap the earliest linked ended run loses its links and a running one keeps them`() { + val store = store(maxRuns = 2) + val first = ExtractionRunKey(tenant, ExtractionRunRef(fixtureRunIds[0])) + val second = ExtractionRunKey(tenant, ExtractionRunRef(fixtureRunIds[1])) + val third = ExtractionRunKey(tenant, ExtractionRunRef("link-run-c")) + runs.save(run(third.runRef.runId, tenant)) + val id = fixturePropositionIds.first() + store.link(first, listOf(id)) + store.link(second, listOf(id)) + runs.transition(first, completed()) + runs.transition(second, completed()) + + store.link(third, listOf(id)) + + assertThat(store.propositionsOf(first, 10)).isEmpty() + assertThat(store.propositionsOf(second, 10)).containsExactly(id) + assertThat(store.propositionsOf(third, 10)).containsExactly(id) + assertThat(store.runsOf(tenant.value, id, 10).map { it.runId }) + .containsExactly(fixtureRunIds[1], third.runRef.runId) + } + + @Test + fun `a running run is never evicted for the cap`() { + val store = store(maxRuns = 1) + val first = ExtractionRunKey(tenant, ExtractionRunRef(fixtureRunIds[0])) + val second = ExtractionRunKey(tenant, ExtractionRunRef(fixtureRunIds[1])) + val id = fixturePropositionIds.first() + store.link(first, listOf(id)) + + store.link(second, listOf(id)) + + assertThat(store.propositionsOf(first, 10)).containsExactly(id) + assertThat(store.propositionsOf(second, 10)).containsExactly(id) + } + + private fun completed() = ExtractionRunTransition(ExtractionRunStatus.COMPLETED, Instant.now()) + + @Test + fun `the cap must be positive`() { + assertThrows(IllegalArgumentException::class.java) { store(maxRuns = 0) } + } + + override fun deleteProposition(id: String) { + propositions.delete(id) + } + + /** + * The reference store prunes a stale link when a read finds it, and the proof is that the id + * cannot come back: re-saving a proposition under the same id after the prune does not revive + * the link, because the link is gone, not merely hidden. + */ + @Test + fun `a link to a deleted proposition is pruned on read and does not revive with the id`() { + val store = store() + val key = ExtractionRunKey(tenant, ExtractionRunRef(fixtureRunIds.first())) + store.link(key, listOf(disposablePropositionId, fixturePropositionIds.first())) + + deleteProposition(disposablePropositionId) + assertThat(store.propositionsOf(key, 10)).containsExactly(fixturePropositionIds.first()) + + propositions.save(proposition(disposablePropositionId, tenant)) + assertThat(store.propositionsOf(key, 10)).containsExactly(fixturePropositionIds.first()) + assertThat(store.runsOf(tenant.value, disposablePropositionId, 10)).isEmpty() + } + + /** + * Two threads linking the same claim to the same run leave one edge. + * + * `PRODUCED_BY_RUN` is a set relation: a run either produced a claim or it did not, and there is + * no such thing as producing it twice. Re-extraction is the normal case, and lineage is written + * from a path that retries, so concurrent writers landing on the same pair is the expected + * shape here. Duplicated edges would inflate every audit answer built on this + * relation while each individual link still looked correct. + * + * The guard is that the relation is stored as a set behind one monitor, so the check and the + * write that depends on it cannot interleave. Swap that set for a list and this test reports two. + */ + @Test + fun `concurrent links of the same pair leave exactly one edge`() { + val store = store() + val key = ExtractionRunKey(tenant, ExtractionRunRef(fixtureRunIds.first())) + val propositionId = fixturePropositionIds.first() + + val threads = 8 + val ready = CountDownLatch(threads) + val go = CountDownLatch(1) + val failures = CopyOnWriteArrayList() + val pool = Executors.newFixedThreadPool(threads) + try { + repeat(threads) { + pool.submit { + try { + ready.countDown() + // Every thread blocks here, so the writes overlap, with no thread queueing + // up behind another's startup. + go.await() + store.link(key, listOf(propositionId)) + } catch (e: Throwable) { + failures += e + } + } + } + assertTrue(ready.await(10, TimeUnit.SECONDS), "workers did not start") + go.countDown() + pool.shutdown() + assertTrue(pool.awaitTermination(10, TimeUnit.SECONDS), "workers did not finish") + } finally { + pool.shutdownNow() + } + + assertTrue(failures.isEmpty(), "concurrent links failed: $failures") + assertEquals( + listOf(propositionId), + store.propositionsOf(key, 10), + "the run produced this claim once, however many writers said so", + ) + assertEquals( + listOf(key.runRef), + store.runsOf(tenant, propositionId, 10), + "and the claim names that run once, from the other direction", + ) + } +} diff --git a/dice-storage/src/test/kotlin/com/embabel/dice/storage/TestApplication.kt b/dice-storage/src/test/kotlin/com/embabel/dice/storage/TestApplication.kt index edbd5577..3c94c1af 100644 --- a/dice-storage/src/test/kotlin/com/embabel/dice/storage/TestApplication.kt +++ b/dice-storage/src/test/kotlin/com/embabel/dice/storage/TestApplication.kt @@ -202,6 +202,11 @@ open class TestApplication { listener = eventListener, ) + @Bean + open fun propositionRunLinkStore( + persistenceManager: PersistenceManager, + ): DrivinePropositionRunLinkStore = DrivinePropositionRunLinkStore(persistenceManager) + @Bean open fun decayManager( repository: DrivinePropositionRepository, diff --git a/dice/src/main/kotlin/com/embabel/dice/common/SourceAnalysisContext.kt b/dice/src/main/kotlin/com/embabel/dice/common/SourceAnalysisContext.kt index bb9174a3..7fa438e3 100644 --- a/dice/src/main/kotlin/com/embabel/dice/common/SourceAnalysisContext.kt +++ b/dice/src/main/kotlin/com/embabel/dice/common/SourceAnalysisContext.kt @@ -47,8 +47,13 @@ import com.embabel.dice.proposition.extraction.ExtractionRunRef * it to whatever it means. `null` (the default) is the whole of the existing behaviour. * Independent of [perspective], [schema] and [contextId]: setting one never constrains another. * @param currentRun optional reference to the extraction run this analysis belongs to. - * EXPERIMENTAL. Identity only — DICE #67 brings the durable run this reference will key. - * `null` (the default) means the analysis is attributed to no run, which is every caller today. + * EXPERIMENTAL, and unlike [profile] this one is read. An analysis carrying a run has its + * structural wiring, graph projection and grounding run over the propositions the repository + * returned rather than the ones extraction minted — the two differ whenever the backend + * deduplicates, and the returned ones are what is actually stored — and, where the host configured + * a `PropositionRunLinkStore`, each stored proposition is attributed to this run. + * `null` (the default) means the analysis is attributed to no run and behaves exactly as it did + * before extraction runs existed. * @param mintNewEntities whether a mention the resolver could NOT match to an existing entity may * be persisted as a NEW entity node. Default FALSE: unresolved mentions stay unresolved (the * proposition is still persisted; its mention simply carries no resolvedId), so extraction never @@ -175,7 +180,12 @@ data class SourceAnalysisContext @JvmOverloads constructor( /** * Returns a copy that says this analysis belongs to the given extraction run. EXPERIMENTAL. - * Changes no other field and no extraction behaviour — see [currentRun]. + * + * Changes no other field, but — unlike [withProfile] — it does change what the analysis writes. + * An analysis carrying a run wires structural relationships, graph projection and grounding + * against the propositions the repository returned rather than the ones extraction minted, and + * attributes each stored proposition to the run where a `PropositionRunLinkStore` is + * configured. See [currentRun]. */ fun withCurrentRun(currentRun: ExtractionRunRef): SourceAnalysisContext = copy(currentRun = currentRun) diff --git a/dice/src/main/kotlin/com/embabel/dice/common/SourceAnalysisRequestEvent.kt b/dice/src/main/kotlin/com/embabel/dice/common/SourceAnalysisRequestEvent.kt index fd427bf7..a503eff1 100644 --- a/dice/src/main/kotlin/com/embabel/dice/common/SourceAnalysisRequestEvent.kt +++ b/dice/src/main/kotlin/com/embabel/dice/common/SourceAnalysisRequestEvent.kt @@ -66,7 +66,13 @@ abstract class SourceAnalysisRequestEvent( /** * The extraction run this event's analysis belongs to, when the publisher is running one. - * EXPERIMENTAL. Identity only — nothing is stored under it until DICE #67 lands. + * + * EXPERIMENTAL, and returning one is not free the way [profile] is. The async path goes through + * the same `buildContext` call `rememberText` does, so an event carrying a run gets the same + * treatment: structural wiring, graph projection and grounding run over the propositions the + * repository returned instead of the ones extraction minted, and each stored proposition is + * attributed to the run where the host configured a `PropositionRunLinkStore`. Returning null — + * the default — leaves the analysis attributed to no run and behaves as it always did. */ open fun currentRun(): ExtractionRunRef? = null } diff --git a/dice/src/main/kotlin/com/embabel/dice/pipeline/PersistablePropositions.kt b/dice/src/main/kotlin/com/embabel/dice/pipeline/PersistablePropositions.kt index ea79ee3e..a418891f 100644 --- a/dice/src/main/kotlin/com/embabel/dice/pipeline/PersistablePropositions.kt +++ b/dice/src/main/kotlin/com/embabel/dice/pipeline/PersistablePropositions.kt @@ -21,6 +21,7 @@ import com.embabel.agent.rag.service.RelationshipData import com.embabel.agent.rag.service.RetrievableIdentifier import com.embabel.dice.common.EntityExtractionResult import com.embabel.dice.proposition.Proposition +import com.embabel.dice.proposition.PropositionPersistenceResult import com.embabel.dice.proposition.PropositionRepository import com.embabel.dice.proposition.RelationshipTypes @@ -80,6 +81,102 @@ interface PersistablePropositions : EntityExtractionResult, PropositionExtractio createStructuralRelationships(propsToSave, namedEntityDataRepository) } + /** + * Persist the same things [persist] does, and report which stored proposition each extracted + * one landed on. + * + * The difference that matters is one line: structural relationships are wired against the + * propositions the repository returned, not the ones extraction minted. On a deduplicating + * backend those are not the same. `DrivinePropositionRepository` collapses a fresh insert onto + * an existing proposition with identical `(contextId, text)` and returns that one, so a + * `HAS_PROPOSITION` or `MENTIONS` edge written against the minted id points at a node that was + * never stored. Here it points at the node that was. + * + * [persist] is left exactly as it was. This is a second entry point rather than a fix applied + * in place, because changing what [persist] wires would change behaviour for every existing + * caller — including hosts that never asked for extraction runs. + * + * Entity persistence, the referenced-entity filter, and which propositions get saved are all + * identical to [persist]. + * + * @param propositionRepository Where propositions go. + * @param namedEntityDataRepository Where entities go, and what writes the structural edges. + * @return The stored proposition for each one persisted, and the input-id to stored-id mapping. + */ + fun persistReturningCanonical( + propositionRepository: PropositionRepository, + namedEntityDataRepository: NamedEntityDataRepository, + ): PropositionPersistenceResult { + val persisted = persistCanonicalPropositions(propositionRepository, namedEntityDataRepository) + wireStructuralRelationships(persisted, namedEntityDataRepository) + return persisted + } + + /** + * The first half of [persistReturningCanonical]: entities and propositions are saved, and + * nothing else happens. Saved means committed only when no transaction wraps the call; inside + * an ambient transaction the claims land with the caller's commit, not here. + * + * **Split out so a caller can act on saved claims before anything fallible runs.** Structural + * wiring goes through `mergeRelationship`, which can throw, and while it lived inside the same + * call the propositions were already stored by the time it failed — but a caller could not do + * anything about that until the call returned, which it never did. Extraction-run lineage is the + * caller that has to: attribution is a statement about claims that exist, and a failing edge + * write must not be able to strand a stored claim with no record of the run that produced it. + * + * A caller that has no such ordering requirement should use [persistReturningCanonical], which + * is this followed by [wireStructuralRelationships] and is what it always was. + * + * @param propositionRepository Where propositions go. + * @param namedEntityDataRepository Where entities go. + * @return The stored proposition for each one persisted, and the input-id to stored-id mapping. + */ + fun persistCanonicalPropositions( + propositionRepository: PropositionRepository, + namedEntityDataRepository: NamedEntityDataRepository, + ): PropositionPersistenceResult { + val propsToSave = propositionsToPersist() + + // Only persist entities that are actually referenced by propositions being saved + val referencedEntityIds = propsToSave + .flatMap { it.mentions } + .mapNotNull { it.resolvedId } + .toSet() + + newEntities() + .filter { it.id in referencedEntityIds } + .forEach { entity -> + namedEntityDataRepository.save(entity) + } + updatedEntities() + .filter { it.id in referencedEntityIds } + .forEach { entity -> + namedEntityDataRepository.update(entity) + } + + return propositionRepository.saveAllReturningCanonical(propsToSave) + } + + /** + * The second half of [persistReturningCanonical]: the chunk, proposition and entity edges, wired + * against the propositions the repository returned so every edge lands on a node the store + * actually holds. + * + * @param persisted What [persistCanonicalPropositions] returned. + * @param namedEntityDataRepository What writes the edges. + */ + fun wireStructuralRelationships( + persisted: PropositionPersistenceResult, + namedEntityDataRepository: NamedEntityDataRepository, + ) { + // The distinct view: two inputs that deduplicated onto one proposition are one + // proposition, and issuing the same merge twice is duplicate work, not a second edge. + createStructuralRelationships( + persisted.distinctCanonicalPropositions, + namedEntityDataRepository, + ) + } + companion object { const val PROPOSITION_LABEL = "Proposition" diff --git a/dice/src/main/kotlin/com/embabel/dice/proposition/EventEmittingPropositionRepository.kt b/dice/src/main/kotlin/com/embabel/dice/proposition/EventEmittingPropositionRepository.kt index 99f08b8c..c97b41ef 100644 --- a/dice/src/main/kotlin/com/embabel/dice/proposition/EventEmittingPropositionRepository.kt +++ b/dice/src/main/kotlin/com/embabel/dice/proposition/EventEmittingPropositionRepository.kt @@ -146,6 +146,21 @@ open class EventEmittingPropositionRepository( propositions.forEach { save(it) } } + /** + * The same, keeping what each proposition landed on. Overridden for the same reason [saveAll] + * is: `by delegate` forwards an interface default straight to the delegate, so the default body + * would run the delegate's [save] and this decorator would emit nothing. + * + * @param propositions The propositions to persist. + * @return The stored proposition for each input, and the input-id to stored-id mapping. + */ + override fun saveAllReturningCanonical( + propositions: Collection, + ): PropositionPersistenceResult { + val inputs = propositions.toList() + return PropositionPersistenceResult.of(inputs, inputs.map { save(it) }) + } + // ======================================================================== // Explicit vector-capability forwarding // diff --git a/dice/src/main/kotlin/com/embabel/dice/proposition/PropositionPersistenceResult.kt b/dice/src/main/kotlin/com/embabel/dice/proposition/PropositionPersistenceResult.kt new file mode 100644 index 00000000..c0129ab1 --- /dev/null +++ b/dice/src/main/kotlin/com/embabel/dice/proposition/PropositionPersistenceResult.kt @@ -0,0 +1,192 @@ +/* + * 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 + +import org.jetbrains.annotations.ApiStatus +import java.util.Collections + +/** + * What a batch of propositions actually landed on: which stored proposition each one you handed in + * became. + * + * Usually that is the same proposition you passed, with the same id. It is not the same when a + * backend deduplicates. `DrivinePropositionRepository` collapses a fresh insert onto an existing + * proposition with identical `(contextId, text)`, unions the incoming evidence into it, and hands + * back the one that is stored — a different id from the one you minted. Anything that then writes an + * edge, a projection or a grounding link against the id it minted is pointing at a node that does + * not exist. + * + * `saveAll` returns `Unit`, so that mapping used to be thrown away. This type is what carries it + * back. + * + * **Two views, and they are not interchangeable.** [canonicalPropositions] is positional: one entry + * per input, in input order, so it lines up with the list you passed and can repeat a proposition + * when two inputs deduplicated onto one. [canonicalIds] is the distinct set, first-seen order, + * which is what you want for a write that should happen once per stored proposition. + * + * Whichever view you read, every proposition in it is the store's *final* answer for that input — + * see [of] for what that means when one batch names an id twice. + * + * EXPERIMENTAL. The shape may still change while extraction runs (DICE #67) land. + * + * @property canonicalPropositions The stored proposition each input became, one per input, in input + * order + * @property canonicalIdByInputId The id you handed in, mapped to the id it landed on + */ +@ApiStatus.Experimental +class PropositionPersistenceResult private constructor( + canonicalPropositions: List, + canonicalIdByInputId: Map, +) { + + val canonicalPropositions: List = + Collections.unmodifiableList(ArrayList(canonicalPropositions)) + + val canonicalIdByInputId: Map = + Collections.unmodifiableMap(LinkedHashMap(canonicalIdByInputId)) + + /** + * The distinct stored proposition ids, in the order they were first seen. + * + * Two inputs that deduplicated onto one proposition appear once here and twice in + * [canonicalPropositions]. Use this for anything that should be written once per stored + * proposition, such as a run link. + */ + val canonicalIds: List = + Collections.unmodifiableList(ArrayList(this.canonicalPropositions.map { it.id }.distinct())) + + /** + * The distinct stored propositions, first-seen order — [canonicalIds] with the objects attached. + * + * **This is what downstream work should run over.** Two inputs that deduplicated onto one + * proposition are one stored proposition, and projecting it twice, grounding it twice or issuing + * the same structural merge twice is duplicate work whose edges happen to be idempotent — but + * the records written *about* that work are not. A projection recorder writes one row per + * result, so the positional list inflates the audit with rows describing a proposition that was + * projected once. + * + * [canonicalPropositions] stays positional for callers that need to line results up against the + * propositions they passed in. + */ + val distinctCanonicalPropositions: List = + Collections.unmodifiableList(ArrayList(this.canonicalPropositions.distinctBy { it.id })) + + /** The inputs whose id is not the id they landed on — the ones a backend deduplicated. */ + val dedupedInputIds: List = + Collections.unmodifiableList( + ArrayList(this.canonicalIdByInputId.filter { (input, canonical) -> input != canonical }.keys), + ) + + /** True when nothing deduplicated: every input landed on its own id. */ + val isIdentity: Boolean + get() = dedupedInputIds.isEmpty() + + /** The id [inputId] landed on, or null if this batch did not carry it. */ + fun canonicalIdOf(inputId: String): String? = canonicalIdByInputId[inputId] + + override fun toString(): String = + "PropositionPersistenceResult(saved=${canonicalPropositions.size}, " + + "distinct=${canonicalIds.size}, deduped=${dedupedInputIds.size})" + + companion object { + + /** Nothing was persisted. */ + @JvmField + val EMPTY: PropositionPersistenceResult = PropositionPersistenceResult(emptyList(), emptyMap()) + + /** + * Pairs the propositions handed to a store with what the store returned for each, by + * position. + * + * Position is the only pairing available: an id cannot be the key, because a deduplicated + * input comes back under a different one, which is the whole point. Both lists come from one + * call, so they are the same length and in the same order. + * + * **Every position reports the store's final answer for the id that position landed on.** + * Two ways a position goes stale, and normalizing by *canonical* id covers both. One batch + * can name one input id twice — two revision results touching one original — and a + * replace-by-id store overwrites the first save with the second. And two *distinct* inputs + * can deduplicate onto one canonical, where the save that collapses the second also updates + * that canonical: DICE's graph repository unions the incoming evidence into the winner and + * answers with the winner as it then stands. In both cases an earlier position holds an + * object the store has already moved past, and in both cases the ids match, so comparing + * ids cannot catch it. + * + * Resolving rather than rejecting is deliberate. This runs from + * `PersistablePropositions.persistReturningCanonical` *after* the saves have run, so + * throwing would fail an extraction whose propositions are already written — and + * last-write-wins is not a guess, it is what the store has. + * + * @param inputs What was handed to the store, in order. + * @param canonical What the store returned, one per input, in the same order. + * @throws IllegalArgumentException if the lists are different lengths, or if one input id + * appears twice landing on two *different* stored ids. The second is a backend that + * answered two saves of one id with two different propositions, which nothing downstream + * could act on sensibly and which resolution cannot honestly paper over. + */ + @JvmStatic + fun of( + inputs: List, + canonical: List, + ): PropositionPersistenceResult { + require(inputs.size == canonical.size) { + "a store must answer every proposition it was given: ${inputs.size} in, " + + "${canonical.size} out" + } + if (inputs.isEmpty()) return EMPTY + val mapping = LinkedHashMap(inputs.size) + // Keyed by the id landed on, not the id handed in. Those are different keys and only + // the first is wrong: two *distinct* inputs can deduplicate onto one canonical, and the + // save that collapses the second one also updates that canonical — DICE's own graph + // repository unions the incoming evidence into the winner and answers with the winner + // as it then stands. Keyed by input id, the first position would keep the canonical as + // it looked before the second input's evidence merged in, and the wiring passes would + // run over an object the store had already moved past. + val lastAnswerForCanonical = HashMap(inputs.size) + inputs.forEachIndexed { index, input -> + val landedOn = canonical[index] + // A store may answer with a different id; it may never answer with a different + // tenant. Downstream this object is wired, projected and grounded as though it were + // this tenant's, and lineage is the only pass that would refuse it — best-effort, so + // it refuses quietly and the rest still runs. Caught here, where it enters. + require(landedOn.contextId == input.contextId) { + "proposition ${input.id} was answered with ${landedOn.id} from another context" + } + val already = mapping.put(input.id, landedOn.id) + require(already == null || already == landedOn.id) { + "proposition ${input.id} landed on two different stored ids in one batch" + } + // The per-position check above is not enough on its own. Two positions can each be + // answered in their own input's context and still name one stored id between them, + // and the resolution below is keyed by that id — so the later answer would replace + // the earlier one and hand position zero a proposition from another tenant, with + // every individual guard satisfied. One id is one proposition in one tenant, which + // the graph's uniqueness constraint already says, so a batch claiming otherwise is + // describing a store that cannot exist. + val seenUnder = lastAnswerForCanonical.put(landedOn.id, landedOn) + require(seenUnder == null || seenUnder.contextId == landedOn.contextId) { + "stored proposition ${landedOn.id} was answered under two different contexts " + + "in one batch" + } + } + // Every position reports the store's final answer for the id that position landed on, + // so nothing downstream can be handed a proposition a later save in the same batch + // replaced or updated. + val resolved = inputs.map { lastAnswerForCanonical.getValue(mapping.getValue(it.id)) } + return PropositionPersistenceResult(resolved, mapping) + } + } +} diff --git a/dice/src/main/kotlin/com/embabel/dice/proposition/PropositionStore.kt b/dice/src/main/kotlin/com/embabel/dice/proposition/PropositionStore.kt index 4d94851f..b9f1bfc9 100644 --- a/dice/src/main/kotlin/com/embabel/dice/proposition/PropositionStore.kt +++ b/dice/src/main/kotlin/com/embabel/dice/proposition/PropositionStore.kt @@ -111,6 +111,30 @@ interface PropositionStore { propositions.forEach { save(it) } } + /** + * Save multiple propositions and report which stored proposition each one landed on. + * + * [save] has always returned the stored proposition, and on a deduplicating backend that can be + * a different proposition with a different id from the one passed in. [saveAll] returns `Unit` + * and drops all of it, so a caller doing a batch save had no way to learn a canonical id short + * of saving one at a time. This is that batch call with its answer kept. + * + * Same writes, same order, same backend behaviour as [saveAll] — only the return value differs. + * [saveAll] keeps its `Unit` descriptor rather than growing one, because changing it would break + * every implementation and every compiled caller. + * + * A backend that overrides [saveAll] for a batched round trip should override this too, or it + * loses the batching here: the default goes through [save] one at a time, never through + * [saveAll]. + * + * @param propositions The propositions to persist, in order. + * @return The stored proposition for each input, and the input-id to stored-id mapping. + */ + fun saveAllReturningCanonical(propositions: Collection): PropositionPersistenceResult { + val inputs = propositions.toList() + return PropositionPersistenceResult.of(inputs, inputs.map { save(it) }) + } + /** * Find a proposition by its ID. */ 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 81b2e8f9..1077d732 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 @@ -115,7 +115,7 @@ data class ExtractionRunKey( * @property cohortRef The arm within that experiment * @property replayFidelity How much of this run someone could set up again from what it recorded * @property counts How much the run got through - * @property invocations One record per attempt at each planned model call + * @property invocations One record per attempt at each planned model call, normalized to plan order * @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 @@ -153,9 +153,25 @@ class ExtractionRun @JvmOverloads constructor( val sourceRevisions: List = Collections.unmodifiableList(ArrayList(sourceRevisions)) - /** One record per attempt at each planned call, in whatever order the caller supplied. */ + /** + * One record per attempt at each planned call, always in plan order: call 0 before call 1, and + * within a call, first attempt before second. + * + * **Normalized here rather than left as the caller listed it.** Records arrive in the order + * calls came back, which is not the order they were planned in and is not a fact about the run — + * two runs that made the same calls and got the same answers in a different sequence are the + * same run. Since [equals] compares this list element by element, leaving the caller's order + * alone would make those two runs unequal, and it would make a durable backend disagree with the + * in-memory one on the same call sequence: a store keeps identified rows and reads them back in + * plan order, while an in-memory store would hand back the order it was given. + * + * [sourceRevisions] is deliberately not normalized the same way. The order sources were read in + * is data about the run. + */ val invocations: List = - Collections.unmodifiableList(ArrayList(invocations)) + Collections.unmodifiableList( + invocations.sortedWith(compareBy({ it.invocationIndex }, { it.attempt })), + ) /** What went wrong, bounded and said in the failure vocabulary. */ val failures: List = @@ -230,11 +246,11 @@ class ExtractionRun @JvmOverloads constructor( * The invocation records ordered by the plan: call 0 before call 1, and within a call, first * attempt before second. * - * The order records were handed to the constructor is the order calls came back, which is not - * the order they were planned in. This reads the identities that were allocated up front. + * [invocations] is already in that order — it is normalized at construction — so this returns + * it unchanged. Kept as a named call because that is what a caller asking for plan order should + * be able to say, and because it is what the stores promise. */ - fun invocationsInPlanOrder(): List = - invocations.sortedWith(compareBy({ it.invocationIndex }, { it.attempt })) + fun invocationsInPlanOrder(): List = invocations /** Every attempt at the call at [invocationIndex], earliest attempt first. */ fun attemptsOf(invocationIndex: Int): List = diff --git a/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/InMemoryPropositionRunLinkStore.kt b/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/InMemoryPropositionRunLinkStore.kt new file mode 100644 index 00000000..9ea44733 --- /dev/null +++ b/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/InMemoryPropositionRunLinkStore.kt @@ -0,0 +1,204 @@ +/* + * 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 com.embabel.dice.proposition.PropositionStore +import org.jetbrains.annotations.ApiStatus +import org.slf4j.LoggerFactory + +/** + * Reference [PropositionRunLinkStore] that keeps the relation in a map. + * + * It is the executable statement of what the contract means, so a durable backend can be held to + * the same suite, and it lets a host record run lineage before it has a database. + * + * **It needs both endpoint stores, and that is the honest shape.** The tenant guard is a question + * about existence — is there a run under this key, is there a proposition under this id in this + * tenant — and only the stores that hold them can answer. A durable backend asks the same two + * questions in its own statements inside one transaction. A reference implementation that skipped + * the check would accept + * cross-tenant links every durable backend rejects, and the suite that holds the two together would + * be asserting nothing. + * + * Every write and read runs inside one monitor, so a check and the write that depends on it cannot + * interleave with another thread's. A durable store gets the same from its transaction. + * + * There is no unscoped read here, for the same reason [InMemoryExtractionRunStore] has none: one + * instance holds every tenant's links. + * + * **This is the reference implementation, and it forgets.** A link whose proposition is gone + * altogether is dropped the next time a read resolves it, and a run with no links left is dropped + * with it, but nothing reads on a host's behalf, so the store also holds links for at most + * [maxRuns] runs. When a link for a new run would push it past that cap it evicts the runs it + * linked earliest, oldest first, as long as the run store says the run has ended. A run still + * `RUNNING` keeps its links, so a store where every linked run is running can grow past the cap; + * that logs once at `warn`, not on every link. A host running this store in production is + * accepting that the lineage of a run older than the cap is gone for good. Retention is a real, + * durable policy on the graph-backed store; this one exists so a host can record lineage before it + * has one of those. + * + * 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. + * + * @param runStore Where the run end of a link is resolved. + * @param propositionStore Where the proposition end is resolved. + * @param maxRuns The most runs this store keeps links for before it starts forgetting the ones it + * linked earliest. Must be positive. The default, 10,000, matches [InMemoryExtractionRunStore]. + */ +@ApiStatus.Experimental +class InMemoryPropositionRunLinkStore @JvmOverloads constructor( + private val runStore: ExtractionRunStore, + private val propositionStore: PropositionStore, + private val maxRuns: Int = 10_000, +) : PropositionRunLinkStore { + + init { + require(maxRuns > 0) { "maxRuns must be positive, was $maxRuns" } + } + + private val logger = LoggerFactory.getLogger(InMemoryPropositionRunLinkStore::class.java) + + private val lock = Any() + + /** + * Run to the propositions it produced. A set, so a repeated link is one link. Insertion ordered + * by the run's first link, which is the order eviction walks. + */ + private val byRun = LinkedHashMap>() + + /** Set once the store has grown past [maxRuns] with nothing left to evict, so the breach logs once. */ + private var overCapacityWarned = false + + override fun link(key: ExtractionRunKey, propositionIds: Collection): Int { + val ids = propositionIds.distinct() + if (ids.isEmpty()) return 0 + synchronized(lock) { + runStore.findRun(key) ?: throw ExtractionRunNotFoundException(key) + // Out of scope covers both "no such proposition" and "someone else's proposition": + // from inside a tenant those are the same answer, and both mean this run did not + // produce it. Collected in full before anything is written, so a rejected batch leaves + // the relation exactly as it found it. + val outOfScope = ids.filterNot { inContext(it, key.contextId) } + if (outOfScope.isNotEmpty()) { + throw PropositionRunLinkScopeException(key, outOfScope) + } + val linked = byRun.getOrPut(key) { LinkedHashSet() } + linked.addAll(ids) + evictOverflow() + return ids.count { it in linked } + } + } + + /** + * Drops the links of the runs linked earliest until the store holds links for at most [maxRuns] + * runs. Called from inside the monitor a link already holds. A run the run store still reports + * as `RUNNING` is skipped; if every candidate is running the breach is logged once and the store + * stays over the cap. + */ + private fun evictOverflow() { + if (byRun.size <= maxRuns) return + val iterator = byRun.keys.iterator() + while (byRun.size > maxRuns && iterator.hasNext()) { + val candidate = iterator.next() + if (runStore.findRun(candidate)?.status?.isTerminal == false) continue + iterator.remove() + } + if (byRun.size > maxRuns && !overCapacityWarned) { + overCapacityWarned = true + logger.warn( + "InMemoryPropositionRunLinkStore holds links for {} runs, over its cap of {}, and " + + "every one of them is still running, so none can be evicted", + byRun.size, + maxRuns, + ) + } + } + + override fun runsOf( + contextIdValue: String, + propositionId: String, + limit: Int, + ): List { + requirePositiveLimit(limit) + return synchronized(lock) { + // The proposition is checked against the store as it is now, not as it was when the + // link was written. See the note on [propositionsOf]. A read from the wrong tenant fails + // closed and touches nothing; only a claim that is gone altogether is pruned from every + // run that named it. + if (!inContext(propositionId, ContextId(contextIdValue))) { + if (propositionStore.findById(propositionId) == null) prune(propositionId) + return emptyList() + } + byRun.entries + .filter { (key, ids) -> key.contextId.value == contextIdValue && propositionId in ids } + .map { (key, _) -> key.runRef } + .sortedBy { it.runId } + .take(limit) + } + } + + /** + * The propositions this run produced that this tenant still holds. + * + * **Both reads resolve against the proposition store every time, rather than trusting the map.** + * A link records that a run produced a claim; if the claim is deleted, or its id now belongs to + * another tenant, there is nothing left for the link to be about. A graph gets this for free — + * the edge is detached with the node — so a reference implementation answering from its own map + * would keep reporting lineage for claims the store no longer has, and the two backends would + * disagree. Answering from live endpoint state costs a lookup per id and is the only way this + * store can be held to the same contract. + * + * Nothing here is told when a proposition is deleted, so the stale entries are pruned when a + * read finds them: an id the proposition store no longer holds at all is dropped from the run's + * set, and an emptied set is dropped with it. An id that exists but reads as another tenant's + * is filtered and left alone, since a read from the wrong tenant must change nothing. The answer + * is the same either way; what changes is that the map stops growing with claims that no longer + * exist. + */ + override fun propositionsOf(key: ExtractionRunKey, limit: Int): List { + requirePositiveLimit(limit) + return synchronized(lock) { + val ids = byRun[key] ?: return emptyList() + val gone = ids.filter { propositionStore.findById(it) == null } + if (gone.isNotEmpty()) { + ids.removeAll(gone.toSet()) + if (ids.isEmpty()) byRun.remove(key) + } + ids.filter { inContext(it, key.contextId) }.sorted().take(limit) + } + } + + /** Drops a proposition nobody holds any more from every run that named it; a run left with no links is dropped too. */ + private fun prune(propositionId: String) { + val emptied = byRun.entries.filter { (_, ids) -> ids.remove(propositionId) && ids.isEmpty() }.map { it.key } + emptied.forEach { byRun.remove(it) } + } + + /** + * Whether the proposition is one this tenant holds. + * + * `findById` is not tenant-scoped — proposition ids are minted globally unique — so the tenant + * is checked on the proposition that comes back rather than assumed from the lookup. + */ + private fun inContext(propositionId: String, contextId: ContextId): Boolean = + propositionStore.findById(propositionId)?.contextId == contextId + + private fun requirePositiveLimit(limit: Int) { + require(limit > 0) { "limit must be positive, was $limit" } + } +} diff --git a/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/IncrementalPropositionExtraction.kt b/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/IncrementalPropositionExtraction.kt index 434dc7f8..0f280b5a 100644 --- a/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/IncrementalPropositionExtraction.kt +++ b/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/IncrementalPropositionExtraction.kt @@ -33,12 +33,14 @@ import com.embabel.dice.incremental.proposition.PropositionIncrementalAnalyzer import com.embabel.dice.pipeline.ChunkPropositionResult import com.embabel.dice.pipeline.PropositionPipeline import com.embabel.dice.projection.graph.GraphProjectionService +import com.embabel.dice.proposition.PropositionPersistenceResult import com.embabel.dice.proposition.PropositionRepository import org.slf4j.LoggerFactory import org.springframework.context.event.EventListener import org.springframework.scheduling.annotation.Async import java.io.InputStream import java.util.concurrent.ConcurrentLinkedQueue +import java.util.concurrent.atomic.AtomicBoolean import java.util.concurrent.atomic.AtomicInteger import java.util.concurrent.locks.ReentrantLock import java.util.function.Function @@ -117,6 +119,97 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( */ private val mintedEntityPropertiesProvider: (NamedEntity) -> Map = { _ -> emptyMap() }, ) { + + /** + * Where `(proposition, run)` links go, once a host has bound one with [withRunLineage]. + * + * Volatile because it is written on whatever thread builds the extractor and read on the async + * extraction threads. It is written exactly once — [runLineageBound] enforces that — so a + * reader sees either the initial null or the bound store, never a value that later changes + * underneath it. + */ + @Volatile + private var propositionRunLinkStore: PropositionRunLinkStore? = null + + /** + * What happens when lineage cannot be written. Bound with the store, and volatile for the same + * reason: written on the thread that builds the extractor, read on the extraction threads. + */ + @Volatile + private var lineageFailurePolicy: LineageFailurePolicy = LineageFailurePolicy.DEFAULT + + /** Whether [withRunLineage] has been called. Binding is one-time; see there for why. */ + private val runLineageBound = AtomicBoolean(false) + + /** + * Binds the store that records which extraction run produced which claims, and returns this + * extractor so a bean method can do it in one expression. + * + * ```kotlin + * IncrementalPropositionExtraction(pipeline, ..., properties).withRunLineage(linkStore) + * ``` + * + * **Why this is a method and not a constructor parameter.** Kotlin compiles a constructor with + * default arguments into a single synthetic + * `(...every parameter..., int mask, DefaultConstructorMarker)`, and that is what a + * precompiled Kotlin caller links against whenever it omits any argument. Appending a parameter + * rewrites that descriptor, so every such caller would fail with `NoSuchMethodError` — and + * `@JvmOverloads` does not save them, because it republishes the Java overloads that Kotlin + * callers using defaults never touch. Adding a method adds API; appending a defaulted parameter + * moves one. `RunLineageBinaryCompatibilityTest` pins the descriptor. + * + * **Binding is one-time, and a second call is rejected.** The field is read when an analysis + * records lineage, not when it starts, so a later call would redirect or silently erase the + * audit record of an extraction already in flight — the failure would be a missing or misfiled + * lineage row long after the call that caused it. An audit surface should not be swappable at a + * distance. Passing null is a binding like any other: it is how a host says "record none", and + * it cannot later be upgraded, or "bound once" would depend on which value was passed. + * + * Bind before the extractor starts handling events. + * + * Lineage is only ever consulted for an analysis that carries a + * [SourceAnalysisContext.currentRun]; without a run this store is never touched. EXPERIMENTAL. + * + * Binds [LineageFailurePolicy.DEFAULT]. Use the two-argument form to choose. + * + * @param propositionRunLinkStore The lineage store, or null to record no lineage. + * @return this extractor. + * @throws IllegalStateException if lineage has already been bound. + */ + open fun withRunLineage( + propositionRunLinkStore: PropositionRunLinkStore?, + ): IncrementalPropositionExtraction = + withRunLineage(propositionRunLinkStore, LineageFailurePolicy.DEFAULT) + + /** + * Binds the lineage store and says what happens when a lineage write cannot be made. + * + * A separate overload, because a defaulted parameter on the one-argument form would move that + * method's Kotlin descriptor — the whole reason lineage is bound by a method in the first place. `RunLineageBinaryCompatibilityTest` pins it. + * + * **A null store under [LineageFailurePolicy.STRICT] is a legitimate binding**, and it is how a + * host says "record no lineage" while still refusing to run one. It only bites when an analysis + * actually carries a run: with no store bound and a run set, [LineageFailurePolicy.STRICT] + * fails that call. A host that passes runs must bind a store. + * + * @param propositionRunLinkStore The lineage store, or null to record no lineage. + * @param policy What happens when lineage cannot be written. See [LineageFailurePolicy]. + * @return this extractor. + * @throws IllegalStateException if lineage has already been bound. + */ + open fun withRunLineage( + propositionRunLinkStore: PropositionRunLinkStore?, + policy: LineageFailurePolicy, + ): IncrementalPropositionExtraction { + check(runLineageBound.compareAndSet(false, true)) { + "run lineage is already bound on this extractor; it binds once, before the extractor " + + "starts handling events, so an analysis in flight cannot have its audit record " + + "redirected or erased" + } + this.propositionRunLinkStore = propositionRunLinkStore + this.lineageFailurePolicy = policy + return this + } private val analyzer: IncrementalAnalyzer = PropositionIncrementalAnalyzer( propositionPipeline, @@ -309,7 +402,7 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( if (result != null && result.propositions.isNotEmpty()) { logger.info(result.infoString(true, 1)) - persistAndProject(result) + persistAndProject(result, context) logAllPropositions(contextIdProvider.apply(user)) logger.info("Remembered source: {}", sourceId) } else { @@ -319,25 +412,58 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( // -- internal --------------------------------------------------------- + /** + * Drains [pendingEvents] under [extractionLock], one event at a time. + * + * **A lineage failure on one queued event does not cost the rest of the queue.** `processEvent` + * rethrows [LineageNotRecordedException] and does not swallow it, so this loop catches it per + * event, remembers the first one, and keeps polling. Every event that was queued when the drain + * started still runs. Once the queue this call can see is empty, the remembered failure (if any) + * is rethrown, so a STRICT wiring mistake still reaches whoever called [extractPropositions] and + * is not lost behind the events that came after it. + */ private fun processPendingEvents() { if (!extractionLock.tryLock()) { logger.debug("Extraction in progress, {} event(s) queued", pendingEvents.size) return } + var firstFailure: LineageNotRecordedException? = null try { var next = pendingEvents.poll() while (next != null) { - processEvent(next) + try { + processEvent(next) + } catch (e: LineageNotRecordedException) { + if (firstFailure == null) firstFailure = e + } next = pendingEvents.poll() } } finally { extractionLock.unlock() } if (pendingEvents.isNotEmpty()) { - processPendingEvents() + try { + processPendingEvents() + } catch (e: LineageNotRecordedException) { + if (firstFailure == null) firstFailure = e + } } + firstFailure?.let { throw it } } + /** + * Processes one event: builds its context, runs the analyzer, and persists what it finds. + * + * **A STRICT lineage failure reaches the caller; everything else is logged and swallowed.** + * `extractPropositions` is the public entry point an `@Async @EventListener` calls, so a plain + * exception here would only ever reach Spring's executor's uncaught-exception handler, not a + * caller waiting on the result, which is why every other failure is caught and logged and not + * left to propagate. Attribution failing under STRICT is different: a host that bound + * [LineageFailurePolicy.STRICT] asked to hear about it, and an event-published extraction must + * fail exactly as loud as a direct call with the same inputs would. So + * [LineageNotRecordedException] is caught ahead of the general catch, logged at `error` with the + * run key, and rethrown to [processPendingEvents], which is what makes it reach the publisher. + */ private fun processEvent(event: SourceAnalysisRequestEvent) { try { val source = event.incrementalSource() @@ -377,8 +503,15 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( } logger.info(result.infoString(true, 1)) - persistAndProject(result) + persistAndProject(result, context) logAllPropositions(contextIdProvider.apply(event.user)) + } catch (e: LineageNotRecordedException) { + logger.error( + "Lineage not recorded for extraction run {}; attribution was asked for and could " + + "not be recorded", + e.key.runRef.runId, e, + ) + throw e } catch (e: Exception) { logger.warn("Failed to extract propositions", e) } finally { @@ -455,15 +588,42 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( // context and the request always agree about a call. request.sourceLocator?.let { ctx = ctx.withSourceLocator(it) } request.sourceRevision?.let { ctx = ctx.withSourceRevision(it) } - // Carried, never consulted. Nothing downstream of here reads either one — that is what - // "DICE holds profile identity and the host binds policy" means in code, and the run - // reference is identity only until the run store lands. + // The profile is carried and never consulted. Nothing downstream of here reads it — that is + // what "DICE holds profile identity and the host binds policy" means in code. request.profile?.let { ctx = ctx.withProfile(it) } + // The run is read in exactly one place: persistAndProject writes a lineage link for it. It + // changes nothing else about what gets stored. See persistAndProject. request.currentRun?.let { ctx = ctx.withCurrentRun(it) } return ctx } - private fun persistAndProject(result: ChunkPropositionResult) { + /** + * Persists what an analysis produced, then projects and grounds it. + * + * **Everything downstream of the save runs over the propositions the repository returned, on + * every call.** A deduplicating backend answers a fresh insert with the proposition that + * already exists, so the id extraction minted can name a node that was never stored. Projecting + * or grounding against that id writes an edge pointing at nothing. The propositions the save + * handed back are the ones the store actually holds, and they are what the wiring passes get. + * + * **Audit metadata never changes product behaviour, so a run is not a switch.** This used to + * take the canonical propositions only when the analysis carried a run, which made an audit + * setting decide whether the graph was written correctly — a host that turned on extraction runs + * silently got different edges, and a host that did not kept the phantom ones. Whether anyone is + * recording lineage is a question about the audit trail and has no business changing what gets + * stored. A run now adds one thing: the lineage write below. It subtracts and alters nothing. + * + * **Lineage is written right after the claims are saved, before anything else runs on them.** A + * stored claim is attributed the moment it exists, not only once every later pass has also + * succeeded. Under [LineageFailurePolicy.STRICT] a lineage failure fails the whole operation, + * and the state that leaves behind is just the save: the claims are there, attributed or not, + * and structural wiring, projection and grounding never ran. See [recordRunLineage] for exactly + * what that state is. + * + * How durable any of it is depends on the caller: with no ambient transaction each write has + * committed as it was made, and with one they are all still the caller's to commit or roll back. + */ + private fun persistAndProject(result: ChunkPropositionResult, context: SourceAnalysisContext) { val propsToSave = result.propositionsToPersist() val referencedEntityIds = propsToSave .flatMap { it.mentions } @@ -482,7 +642,27 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( logger.info("Updated entity: name='{}', labels={}", entity.name, entity.labels()) } - result.persist(propositionRepository, entityRepository) + val currentRun = context.currentRun + // Saving only; lineage and the structural edges both follow. The save is its own call + // because the canonical propositions it returns are what everything after it, lineage + // included, wires against. + val persisted = result.persistCanonicalPropositions(propositionRepository, entityRepository) + + // Lineage is attributed the moment the claims exist, before anything fallible runs on them. + // Under STRICT this call can fail the whole operation, and when it does the state it leaves + // behind is just the save: the claims are stored, and the passes below never ran. See + // recordRunLineage. + if (currentRun != null) { + recordRunLineage(context, currentRun, persisted) + } + + // The distinct view, not the positional one. Inputs that deduplicated together are one + // stored proposition, and projecting or grounding it once per input inflates the records + // written about that work even though the edges themselves are idempotent. + val toWire = persisted.distinctCanonicalPropositions + + result.wireStructuralRelationships(persisted, entityRepository) + if (newProps > 0 || updatedProps > 0 || newEntitiesToSave > 0) { logger.info( "Persisted: {} new propositions, {} updated propositions, {} new entities", @@ -492,7 +672,7 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( logger.info("No new data to persist (all propositions were duplicates)") } - val projectionResult = graphProjectionService.projectAndPersist(propsToSave) + val projectionResult = graphProjectionService.projectAndPersist(toWire) val persistenceResult = projectionResult.second if (persistenceResult.persistedCount > 0) { logger.info( @@ -505,7 +685,107 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( // `(:Proposition)-[:GROUNDED_IN]->(:)` edges when the // ids resolve to stored entities. No-op when no wiring service // was supplied (default for backward compatibility). - groundingWiringService?.wire(propsToSave) + groundingWiringService?.wire(toWire) + } + + /** + * Attributes the canonical propositions to the run that produced them. + * + * **Loud by default.** A host that gives an extraction a run has asked for attribution, and + * [LineageFailurePolicy.STRICT] — the default — treats "it could not be recorded" as a failure + * of that call. Both ways attribution can go missing fail: no store bound with a run set, and a + * link write that throws. [LineageFailurePolicy.LENIENT] logs each and carries on, for a host + * that has decided in configuration that the claims outweigh their audit trail. + * + * This used to swallow every `RuntimeException` and return quietly on a missing store, which + * meant the two failures an operator most needs to hear about — lineage wired wrong, and lineage + * refusing writes — both reported success. See [LineageFailurePolicy] for why that is the wrong + * default for an audit surface. + * + * **The end state a STRICT failure leaves behind, exactly.** This runs right after the claims + * are saved, before structural wiring, projection or grounding have had a chance to run. So + * when it raises, the claims are persisted and nothing past that point has happened: no + * structural edges, no projection, no grounding. The operation is reported as failed, and what + * failed is the attribution, before anything downstream of the save was attempted. + * + * That ordering is the point. A claim that exists and cannot be attributed should say so before + * the pipeline does anything else with it, not after the graph around it is already built. + * Recording lineage last would let a STRICT failure surface only once structural wiring, + * projection and grounding had all quietly happened to a claim nobody can trace to a run, which + * is a worse thing for an audit failure to hide behind than simply stopping early. Attribution + * is checked the moment there is something to attribute, and the rest of the pipeline only runs + * once that check has passed. + * + * [LineageFailurePolicy.LENIENT] logs the failure and lets the rest of the pipeline run anyway, + * reaching the older end state and reporting success. + * + * **What a raised failure costs depends on who owns the transaction.** With no ambient + * transaction, the shape every entry point takes unless a host wraps it, the save above has + * already committed, so the caller learns that a stored claim is unattributed and structural + * wiring, projection and grounding never ran on it. Inside a host's `@Transactional`, the save + * and the lineage write share that transaction's fate and the failure rolls both back, which is + * what strict attribution asks for. A lineage failure raised by the database itself, below the + * store's own checks, has already terminated that transaction either way, and no policy here can + * undo that. + * + * An analysis that saved nothing records nothing and is not a failure under either policy: there + * is no claim for the audit to be missing. + */ + private fun recordRunLineage( + context: SourceAnalysisContext, + currentRun: ExtractionRunRef, + persisted: PropositionPersistenceResult, + ) { + val key = ExtractionRunKey(context.contextId, currentRun) + val linkStore = propositionRunLinkStore + if (linkStore == null) { + // A wiring mistake, true of every call this extractor will ever make: this analysis asked + // to be attributed and nothing can record it. + onLineageFailure( + key, + LineageNotRecordedException( + key, + "analysis carries extraction run ${currentRun.runId} and no " + + "PropositionRunLinkStore is bound, so its propositions cannot be attributed; " + + "bind one with withRunLineage, or bind LineageFailurePolicy.LENIENT to accept " + + "the gap", + ), + ) + return + } + if (persisted.canonicalIds.isEmpty()) return + try { + val linked = linkStore.link(key, persisted.canonicalIds) + logger.info("Attributed {} propositions to extraction run {}", linked, currentRun.runId) + } catch (e: RuntimeException) { + onLineageFailure( + key, + LineageNotRecordedException( + key, + "could not attribute ${persisted.canonicalIds.size} proposition(s) to extraction " + + "run ${currentRun.runId}", + e, + ), + ) + } + } + + /** + * Raises or logs, per the bound policy. + * + * The whole exception goes to the logger under [LineageFailurePolicy.LENIENT], with its class + * and stack. A scope rejection means this analysis's context disagrees with the tenant its own + * propositions were saved under, which is a pipeline bug; an outage looks quite different, and + * the stack is what tells them apart. + */ + private fun onLineageFailure(key: ExtractionRunKey, failure: LineageNotRecordedException) { + when (lineageFailurePolicy) { + LineageFailurePolicy.STRICT -> throw failure + LineageFailurePolicy.LENIENT -> logger.warn( + "Lineage not recorded for extraction run {}; continuing under LENIENT policy", + key.runRef.runId, failure, + ) + } } private fun logAllPropositions(contextId: String) { diff --git a/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/LineageFailurePolicy.kt b/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/LineageFailurePolicy.kt new file mode 100644 index 00000000..f24d9028 --- /dev/null +++ b/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/LineageFailurePolicy.kt @@ -0,0 +1,98 @@ +/* + * 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 + +/** + * What happens when a run's lineage cannot be written. + * + * A host asks for attribution by giving an extraction a run. This says how much that ask is worth: + * whether an extraction whose lineage could not be recorded is a failure, or a success with a gap + * in the audit. + * + * **[STRICT] is the default, and the reason is that silence is the wrong answer here.** Lineage is + * an audit record. An operator who turns it on and gets no error believes every stored claim can be + * traced to the run that produced it, and the one moment that belief is worth something — after an + * incident, reconstructing what a run wrote — is the moment a silent gap makes it worthless. A + * warning in a log is not a control: nobody reads it in time, and the extraction that dropped the + * record reported success. Asking for attribution and getting none should be loud. + * + * [LENIENT] exists for the host that has decided the claims matter more than their attribution and + * is willing to say so in configuration. It is a deliberate downgrade, chosen once, in the open. + * + * EXPERIMENTAL. The shape may still change while extraction runs (DICE #67) land. + */ +@ApiStatus.Experimental +enum class LineageFailurePolicy { + + /** + * A lineage write that cannot happen fails the extraction. + * + * Two things fail under this policy, and they are the two ways attribution can go missing: + * + * - An analysis carries a run and no [PropositionRunLinkStore] is bound. The host asked for + * attribution and nothing can record it. That is a wiring mistake: it is the same on every + * call and it will never fix itself. + * - The link write itself throws. A run that does not exist, a proposition in another tenant, a + * database that will not take the write. + * + * The exception reaches the caller: the direct caller for a synchronous call, or the event + * publisher for one dispatched through the async event path, where a host using an async event + * multicaster for that path sees it surface in its executor's own error handler. What that costs + * depends on who owns the transaction: with no ambient transaction the claims were already saved + * and stand, so the caller learns that stored claims are unattributed. Inside a host's + * `@Transactional`, the claims and the lineage share that transaction's fate and the failure + * rolls both back, which is what a host running extraction under strict attribution is asking + * for. + */ + STRICT, + + /** + * A lineage write that cannot happen is logged, and the extraction carries on. + * + * The claims are stored and the audit has a gap. Nothing tells a later reader that the gap is + * there, so a host choosing this is accepting that "no run produced this claim" and "the record + * was lost" look identical from the outside. + */ + LENIENT, + ; + + companion object { + + /** What an extractor uses when a host binds lineage without naming a policy. */ + @JvmField + val DEFAULT: LineageFailurePolicy = STRICT + } +} + +/** + * A run's lineage could not be recorded, under [LineageFailurePolicy.STRICT]. + * + * Thrown when an analysis carries a run and no [PropositionRunLinkStore] is bound, or when the link + * write failed. In the second case the store's own exception is the [cause] — a scope rejection and + * a database outage both arrive here, and the cause is what tells them apart. + * + * EXPERIMENTAL. The shape may still change while extraction runs (DICE #67) land. + * + * @property key The run the lineage was for + */ +@ApiStatus.Experimental +class LineageNotRecordedException( + val key: ExtractionRunKey, + message: String, + cause: Throwable? = null, +) : RuntimeException(message, cause) diff --git a/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/PropositionRunLinkStore.kt b/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/PropositionRunLinkStore.kt new file mode 100644 index 00000000..cd7c901b --- /dev/null +++ b/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/PropositionRunLinkStore.kt @@ -0,0 +1,178 @@ +/* + * 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 + +/** + * Which extraction runs produced which claims. + * + * This is the relation that closes DICE #67's gap: a stored claim could not be traced to the run + * that produced it. `ProvenanceEntry` says which source a claim was read from and + * `CollectorTraceStore` says why a collapse happened; neither carries a run. + * + * ## Many-to-many, and why it has to be + * + * One run produces many propositions. One proposition is produced by many runs — that is not an + * edge case, it is the normal outcome of re-extraction. Run a second extraction over the same + * material and the deduplicating store answers with the proposition that already exists, so the + * second run produced nothing new and still produced that claim. Both runs are true answers to + * "what produced this?", and a relation that could hold only one would have to pick, silently. + * That is why this is its own relation rather than a field on the proposition. + * + * ## Only canonical ids go in + * + * A link is written against the id the store holds, never the id extraction minted. Those differ + * whenever a backend deduplicates, and a link against the minted id would point at nothing. + * [com.embabel.dice.proposition.PropositionPersistenceResult] is what carries the canonical ids + * back from a save, and it is what a caller links from. + * + * ## Run identity stays out of source provenance + * + * Nothing here touches `ProvenanceEntry` or `SourceLocator`. Two claims read from the same source + * under two different runs have equal provenance and always will: source identity answers "where + * did this come from", run identity answers "which execution wrote it down", and folding one into + * the other would make evidence from two runs over one document look like evidence from two + * documents. The lineage lives in this relation instead. + * + * ## Tenant-guarded, on both ends + * + * A link is between a proposition and a run *in one tenant*. [link] resolves both ends inside + * `key.contextId` and rejects the write outright if either does not resolve there — a proposition + * that lives in another tenant is not "missing", it is out of scope, and joining it to this + * tenant's run would put a neighbour's claim into this tenant's audit. Nothing partial is written: + * one out-of-scope id rejects the whole batch. + * + * Both reads fail closed the same way. A read in the wrong tenant returns nothing rather than + * crossing. + * + * ## Bounded, and ordered by id + * + * Every read takes a positive limit. Both are ordered by id ascending, which makes a page + * repeatable without joining anything: ordering runs newest-first would mean reading each run's + * header for its start time, and a caller who wants that already has [ExtractionRunStore] and the + * refs this returns. + * + * EXPERIMENTAL. The shape may still change while extraction runs (DICE #67) land. + */ +@ApiStatus.Experimental +interface PropositionRunLinkStore { + + // ---- writes ---- + + /** + * Records that [key]'s run produced each of [propositionIds], in that run's tenant. + * + * Idempotent: the link is merged on the pair, so re-running an extraction that produced the + * same claims writes nothing new and returns the same number. That is what makes the relation + * safe to write from a path that can be retried. + * + * An empty batch is a no-op and touches nothing. + * + * @param key The run, tenant-qualified. + * @param propositionIds Canonical ids of the propositions the run produced. Duplicates in the + * collection are one link. + * @return How many links now join this run to these propositions. Equal on a replay. + * @throws ExtractionRunNotFoundException if this tenant has no run under that id. A claim + * attributed to a run nobody recorded is a dangling audit row. + * @throws PropositionRunLinkScopeException if any id names a proposition this tenant does not + * hold, whether it does not exist at all or belongs to a neighbour. Nothing is written. + * + * **The failures listed above must not damage the caller.** Lineage is written best-effort by a + * caller that catches and carries on, so raising one of them has to leave everything else + * exactly as it was — including a transaction the caller is running in. They are all raised + * after the implementation's own reads and writes have succeeded, which is what makes that + * possible; an implementation that joins a caller's transaction must not condemn it on the way + * out. `DrivinePropositionRunLinkStore`'s integration tests hold the one backend that has a + * transaction to condemn to this. The in-memory reference has none, so the shared contract suite + * has nothing to assert here and does not pretend to. + * + * A failure *below* an implementation — a database that terminates the transaction itself — is + * outside this and outside any catch. See `DrivinePropositionRunLinkStore.link`. + */ + fun link(key: ExtractionRunKey, propositionIds: Collection): Int + + /** [link] for a single proposition. */ + fun link(key: ExtractionRunKey, propositionId: String): Int = + link(key, listOf(propositionId)) + + // ---- reads ---- + + /** + * The runs that produced the proposition, in one tenant, by run id ascending. + * + * @param contextIdValue The tenant. + * @param propositionId The canonical proposition id. + * @param limit The most runs to return. Must be positive. + * @return At most [limit] run references. Empty if the proposition is unknown to this tenant. + * @throws IllegalArgumentException if [limit] is not positive. + */ + fun runsOf(contextIdValue: String, propositionId: String, limit: Int): List + + /** + * [runsOf] for Kotlin callers holding a typed tenant. + * + * @param contextId The tenant. + * @param propositionId The canonical proposition id. + * @param limit The most runs to return. Must be positive. + * @return At most [limit] run references. + */ + fun runsOf(contextId: ContextId, propositionId: String, limit: Int): List = + runsOf(contextId.value, propositionId, limit) + + /** + * The propositions a run produced — the inverse read — by proposition id ascending. + * + * Ids rather than propositions, so this store does not have to read the proposition store to + * answer. The audit projection joins by canonical id anyway. + * + * @param key The run, tenant-qualified. + * @param limit The most ids to return. Must be positive. + * @return At most [limit] canonical proposition ids. Empty if this tenant has no such run. + * @throws IllegalArgumentException if [limit] is not positive. + */ + fun propositionsOf(key: ExtractionRunKey, limit: Int): List +} + +/** How many ids a scope rejection names before it stops counting them out. */ +private const val MESSAGE_ID_LIMIT: Int = 5 + +/** + * A link named a proposition the run's tenant does not hold. + * + * Either the proposition does not exist, or it exists in another tenant. Those are the same answer + * from inside a tenant, and they mean the same thing: this run cannot claim to have produced it. + * + * The message names the run and up to five of the rejected ids. Proposition ids are DICE-minted + * identifiers and carry no content, so naming them is what an operator needs; the count covers the + * rest, so a large batch cannot produce a log line of unbounded length. + * + * EXPERIMENTAL. The shape may still change while extraction runs (DICE #67) land. + * + * @property key The run the link was against + * @property propositionIds Every id that did not resolve in the run's tenant + */ +@ApiStatus.Experimental +class PropositionRunLinkScopeException( + val key: ExtractionRunKey, + val propositionIds: List, +) : RuntimeException( + "run ${key.runRef.runId} in context ${key.contextId.value}: " + + "${propositionIds.size} proposition(s) are not in this context and cannot be linked — " + + propositionIds.take(MESSAGE_ID_LIMIT).joinToString(separator = ", ") + + if (propositionIds.size > MESSAGE_ID_LIMIT) ", …" else "", +) diff --git a/dice/src/test/kotlin/com/embabel/dice/pipeline/CanonicalPersistenceResultTest.kt b/dice/src/test/kotlin/com/embabel/dice/pipeline/CanonicalPersistenceResultTest.kt new file mode 100644 index 00000000..91c4ff5d --- /dev/null +++ b/dice/src/test/kotlin/com/embabel/dice/pipeline/CanonicalPersistenceResultTest.kt @@ -0,0 +1,485 @@ +/* + * 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.pipeline + +import com.embabel.agent.core.ContextId +import com.embabel.agent.core.DataDictionary +import com.embabel.agent.rag.model.NamedEntityData +import com.embabel.dice.common.DiceEvent +import com.embabel.dice.common.PropositionPersisted +import com.embabel.dice.proposition.EntityMention +import com.embabel.dice.proposition.EventEmittingPropositionRepository +import com.embabel.dice.proposition.MentionRole +import com.embabel.dice.proposition.Proposition +import com.embabel.dice.proposition.PropositionPersistenceResult +import com.embabel.dice.proposition.PropositionRepository +import com.embabel.dice.proposition.RelationshipTypes +import com.embabel.dice.proposition.revision.RevisionResult +import com.embabel.dice.proposition.store.InMemoryPropositionRepository +import com.embabel.dice.provenance.ProvenanceEntry +import com.embabel.dice.provenance.SourceLocator +import com.embabel.dice.provenance.UriLocator +import org.assertj.core.api.Assertions.assertThat +import org.assertj.core.api.Assertions.assertThatIllegalArgumentException +import org.junit.jupiter.api.Test + +/** + * What a save landed on, and what happens to the edges written afterwards. + * + * The thing being pinned is one substitution. `DrivinePropositionRepository` answers a fresh insert + * of text it already holds with the proposition it already holds — a different id. Everything + * downstream that writes an edge against the id extraction minted then points at a node that was + * never stored. These tests run over a repository that deduplicates the same way, because one that + * never deduplicates cannot tell the two paths apart. + */ +class CanonicalPersistenceResultTest { + + private val tenant = ContextId("canonical-tenant") + private val schema: DataDictionary = DataDictionary.fromClasses("canonical") + + /** + * `DrivinePropositionRepository`'s dedup rule, in memory: a *new* id carrying text already + * stored in the same context collapses onto the stored proposition, and `save` returns that + * one. An update to an id already stored writes to its own node. + */ + private class DeduplicatingRepository( + private val delegate: PropositionRepository = InMemoryPropositionRepository(), + ) : PropositionRepository by delegate { + + override fun save(proposition: Proposition): Proposition { + val existing = delegate.findAll().firstOrNull { + it.contextId == proposition.contextId && + it.text == proposition.text && + it.id != proposition.id + } + val isUpdate = delegate.findById(proposition.id) != null + return if (existing != null && !isUpdate) existing else delegate.save(proposition) + } + + override fun saveAll(propositions: Collection) { + propositions.forEach { save(it) } + } + + // `by delegate` would forward the interface default straight past this class's save. + override fun saveAllReturningCanonical( + propositions: Collection, + ): PropositionPersistenceResult { + val inputs = propositions.toList() + return PropositionPersistenceResult.of(inputs, inputs.map { save(it) }) + } + } + + /** + * The dedup rule plus the evidence merge that goes with it, as `DrivinePropositionRepository` + * does it: a collapsed insert does not just answer with the winner, it writes the incoming + * evidence into the winner and answers with the winner as it now stands. `reinforceCount` + * stands in for that merge, so a stale answer is visible as a number. + */ + private class MergingDeduplicatingRepository( + private val delegate: PropositionRepository = InMemoryPropositionRepository(), + ) : PropositionRepository by delegate { + + override fun save(proposition: Proposition): Proposition { + val existing = delegate.findAll().firstOrNull { + it.contextId == proposition.contextId && + it.text == proposition.text && + it.id != proposition.id + } + val isUpdate = delegate.findById(proposition.id) != null + if (existing == null || isUpdate) return delegate.save(proposition) + return delegate.save(existing.copy(reinforceCount = existing.reinforceCount + 1)) + } + + override fun saveAll(propositions: Collection) { + propositions.forEach { save(it) } + } + + override fun saveAllReturningCanonical( + propositions: Collection, + ): PropositionPersistenceResult { + val inputs = propositions.toList() + return PropositionPersistenceResult.of(inputs, inputs.map { save(it) }) + } + } + + private fun proposition( + text: String, + id: String, + grounding: List = listOf("chunk-1"), + ) = Proposition( + id = id, + contextId = tenant, + text = text, + mentions = listOf( + EntityMention( + span = "Alice", + type = "Person", + resolvedId = "entity-alice", + role = MentionRole.SUBJECT, + ), + ), + confidence = 0.9, + grounding = grounding, + ) + + private fun persistable(vararg propositions: Proposition): PersistablePropositions = + object : PersistablePropositions { + override val propositions: List = propositions.toList() + override val revisionResults: List = emptyList() + override fun newEntities(): List = emptyList() + override fun updatedEntities(): List = emptyList() + override fun referenceOnlyEntities(): List = emptyList() + } + + private fun edgeTargets(repo: TrackingEntityRepository, type: String): List = + repo.relationshipsOfType(type).map { it.target.id } + + private fun edgeSources(repo: TrackingEntityRepository, type: String): List = + repo.relationshipsOfType(type).map { it.source.id } + + // ---- what the store hands back ---- + + @Test + fun `saveAll drops the canonical id and the new call keeps it`() { + val repository = DeduplicatingRepository() + repository.save(proposition("Alice likes coffee", id = "canonical")) + val minted = proposition("Alice likes coffee", id = "minted-by-extraction") + + // The old call returns Unit. Everything a caller could learn about where the write landed + // is gone by the time it returns. + repository.saveAll(listOf(minted)) + assertThat(repository.count()).isEqualTo(1) + + val result = repository.saveAllReturningCanonical(listOf(minted)) + + assertThat(result.canonicalIdOf("minted-by-extraction")).isEqualTo("canonical") + assertThat(result.canonicalIds).containsExactly("canonical") + assertThat(result.dedupedInputIds).containsExactly("minted-by-extraction") + assertThat(result.isIdentity).isFalse() + } + + @Test + fun `with nothing to deduplicate the mapping is the identity`() { + val repository = DeduplicatingRepository() + + val result = repository.saveAllReturningCanonical( + listOf( + proposition("Alice likes coffee", id = "one"), + proposition("Bob likes tea", id = "two"), + ), + ) + + assertThat(result.isIdentity).isTrue() + assertThat(result.canonicalIds).containsExactly("one", "two") + assertThat(result.canonicalIdByInputId["one"]).isEqualTo("one") + assertThat(result.canonicalIdByInputId["two"]).isEqualTo("two") + } + + @Test + fun `two inputs that deduplicate onto one appear twice positionally and once distinctly`() { + val repository = DeduplicatingRepository() + repository.save(proposition("Alice likes coffee", id = "canonical")) + + val result = repository.saveAllReturningCanonical( + listOf( + proposition("Alice likes coffee", id = "minted-a"), + proposition("Alice likes coffee", id = "minted-b"), + ), + ) + + assertThat(result.canonicalPropositions.map { it.id }) + .containsExactly("canonical", "canonical") + assertThat(result.canonicalIds).containsExactly("canonical") + } + + @Test + fun `an empty batch is the empty result`() { + val result = DeduplicatingRepository().saveAllReturningCanonical(emptyList()) + + assertThat(result.canonicalPropositions).isEmpty() + assertThat(result.canonicalIds).isEmpty() + assertThat(result.isIdentity).isTrue() + } + + @Test + fun `one input id twice resolves every position to the object that ended up stored`() { + // A batch can name one id twice — two revision results touching one original, say. An + // ordinary replace-by-id store answers the first save with the first object and then + // overwrites it, so the first answer is stale the moment the second save lands. Both answers + // carry the same id, so the "landed on two different ids" guard cannot see it. + // + // Resolving rather than rejecting, deliberately: `of` is called from + // persistReturningCanonical *after* the saves have run, so throwing there would fail + // an extraction whose propositions are already written. Last-write-wins is also simply what + // the store holds. + val repository = DeduplicatingRepository() + val first = proposition("Alice likes coffee", id = "same-id") + val second = first.copy(text = "Alice likes coffee, strongly") + + val result = repository.saveAllReturningCanonical(listOf(first, second)) + + assertThat(result.canonicalPropositions).hasSize(2) + assertThat(result.canonicalPropositions.map { it.text }) + .describedAs("no position may hold the object the second save overwrote") + .containsExactly("Alice likes coffee, strongly", "Alice likes coffee, strongly") + assertThat(result.canonicalIds).containsExactly("same-id") + assertThat(result.canonicalIdOf("same-id")).isEqualTo("same-id") + // And what the store actually holds is what the result reports. + assertThat(repository.findById("same-id")?.text).isEqualTo("Alice likes coffee, strongly") + } + + @Test + fun `two inputs converging on one canonical both report the store's final object`() { + // The dedup case, where the second save also *updates* the canonical — which is exactly + // what DrivinePropositionRepository does: it unions the incoming provenance into the winner + // and returns the winner as it now stands. Resolving by input id is not enough here, + // because minted-a and minted-b are different input ids; the first position would keep the + // canonical as it looked before minted-b's evidence merged in, and structural wiring, + // projection and grounding would run over that stale object. + val repository = MergingDeduplicatingRepository() + repository.save(proposition("Alice likes coffee", id = "canonical")) + + val result = repository.saveAllReturningCanonical( + listOf( + proposition("Alice likes coffee", id = "minted-a"), + proposition("Alice likes coffee", id = "minted-b"), + ), + ) + + assertThat(result.canonicalPropositions.map { it.reinforceCount }) + .describedAs("both positions report the canonical as it finally stands") + .containsExactly(2, 2) + assertThat(result.canonicalIds).containsExactly("canonical") + assertThat(repository.findById("canonical")?.reinforceCount).isEqualTo(2) + } + + @Test + fun `a canonical result from another tenant is rejected`() { + // Lineage would refuse to link it — the run link store resolves every proposition inside the + // run's tenant — but lineage is the last step of the pipeline, and it only runs when the + // analysis carries a run. By the time it could refuse, structural wiring, projection and + // grounding have already run over a foreign-tenant object, writing this tenant's edges + // against a neighbour's claim; an analysis with no run never reaches that check at all. + // The check belongs where the object enters the pipeline. + val mine = proposition("Alice likes coffee", id = "mine") + val theirs = mine.copy(id = "theirs", contextId = ContextId("someone-else")) + + assertThatIllegalArgumentException().isThrownBy { + PropositionPersistenceResult.of(inputs = listOf(mine), canonical = listOf(theirs)) + }.withMessageContaining("another context") + } + + @Test + fun `one canonical id answered under two contexts is rejected`() { + // The hole the per-position tenant check leaves open. Both positions pass it — input A was + // answered with an A-tenant object, input B with a B-tenant one — and both answers carry the + // same id, so the "two different stored ids" check passes too. Then final-answer + // normalization, keyed by canonical id alone, overwrites the A answer with the B one and + // hands position 0 a foreign-context proposition. Every guard individually satisfied, and + // the composition still wrong. + // + // One id is one proposition in one tenant — the graph carries a uniqueness constraint + // saying so — and a batch claiming otherwise is describing a store that cannot exist. + val underA = proposition("Alice likes coffee", id = "shared") + val underB = underA.copy(contextId = ContextId("someone-else")) + + assertThatIllegalArgumentException().isThrownBy { + PropositionPersistenceResult.of( + inputs = listOf(underA, underB), + canonical = listOf(underA, underB), + ) + }.withMessageContaining("under two different contexts") + } + + @Test + fun `co-deduplicated inputs are one unit of downstream work`() { + // The positional list repeats by design, so callers can line results up with what they + // passed. Handing that list to projection, grounding and structural wiring means the same + // stored proposition is projected twice and grounded twice, which is duplicate work and — + // because the projection recorder writes one record per result — duplicate audit rows. + val repository = DeduplicatingRepository() + repository.save(proposition("Alice likes coffee", id = "canonical")) + + val result = repository.saveAllReturningCanonical( + listOf( + proposition("Alice likes coffee", id = "minted-a"), + proposition("Alice likes coffee", id = "minted-b"), + ), + ) + + assertThat(result.canonicalPropositions.map { it.id }) + .describedAs("the positional view still lines up with the inputs") + .containsExactly("canonical", "canonical") + assertThat(result.distinctCanonicalPropositions.map { it.id }) + .describedAs("the downstream view is one entry per stored proposition") + .containsExactly("canonical") + assertThat(result.distinctCanonicalPropositions.map { it.id }) + .isEqualTo(result.canonicalIds) + } + + @Test + fun `structural wiring runs once per stored proposition, not once per input`() { + val repository = DeduplicatingRepository() + repository.save(proposition("Alice likes coffee", id = "canonical")) + val entities = TrackingEntityRepository(schema) + + persistable( + proposition("Alice likes coffee", id = "minted-a"), + proposition("Alice likes coffee", id = "minted-b"), + ).persistReturningCanonical(repository, entities) + + assertThat(edgeTargets(entities, RelationshipTypes.HAS_PROPOSITION)) + .describedAs("one HAS_PROPOSITION edge, not the same merge issued twice") + .containsExactly("canonical") + } + + @Test + fun `a repeated input id that lands on two different stored ids is still rejected`() { + // Resolution is for one id answered twice with the same identity. Two different canonical + // ids for one input id is an incoherent backend, and staying loud about that is worth more + // than guessing which one is real. + val one = proposition("Alice likes coffee", id = "same-id") + val landedElsewhere = proposition("Alice likes coffee", id = "somewhere-else") + + assertThatIllegalArgumentException().isThrownBy { + PropositionPersistenceResult.of( + inputs = listOf(one, one), + canonical = listOf(one, landedElsewhere), + ) + }.withMessageContaining("two different stored ids") + } + + @Test + fun `a backend that answers fewer propositions than it was given is rejected`() { + assertThatIllegalArgumentException().isThrownBy { + PropositionPersistenceResult.of( + inputs = listOf(proposition("Alice likes coffee", id = "one")), + canonical = emptyList(), + ) + }.withMessageContaining("every proposition it was given") + } + + // ---- which id the edges are written against ---- + + @Test + fun `persist wires structural edges against the id extraction minted`() { + // The behaviour on main, pinned so the new variant is visibly a second path rather than a + // silent replacement. The HAS_PROPOSITION edge points at a node the store does not hold. + val repository = DeduplicatingRepository() + repository.save(proposition("Alice likes coffee", id = "canonical")) + val entities = TrackingEntityRepository(schema) + + persistable(proposition("Alice likes coffee", id = "minted")).persist(repository, entities) + + assertThat(edgeTargets(entities, RelationshipTypes.HAS_PROPOSITION)).containsExactly("minted") + assertThat(repository.findById("minted")).isNull() + } + + @Test + fun `the new variant wires structural edges against the canonical id`() { + val repository = DeduplicatingRepository() + repository.save(proposition("Alice likes coffee", id = "canonical")) + val entities = TrackingEntityRepository(schema) + + val result = persistable(proposition("Alice likes coffee", id = "minted")) + .persistReturningCanonical(repository, entities) + + assertThat(result.canonicalIdOf("minted")).isEqualTo("canonical") + assertThat(edgeTargets(entities, RelationshipTypes.HAS_PROPOSITION)).containsExactly("canonical") + assertThat(edgeSources(entities, RelationshipTypes.MENTIONS)).containsExactly("canonical") + assertThat(repository.findById("canonical")).isNotNull() + } + + @Test + fun `with nothing deduplicated both variants write the same edges`() { + // The compatibility claim in one test: with no substitution the new path is the old path. + // Anything else would be a behaviour change hiding behind a dedup-only assertion. + val legacy = TrackingEntityRepository(schema) + val canonical = TrackingEntityRepository(schema) + val one = proposition("Alice likes coffee", id = "one") + val two = proposition("Bob likes tea", id = "two", grounding = listOf("chunk-2")) + + persistable(one, two).persist(DeduplicatingRepository(), legacy) + persistable(one, two).persistReturningCanonical(DeduplicatingRepository(), canonical) + + assertThat(canonical.createdRelationships).isEqualTo(legacy.createdRelationships) + } + + @Test + fun `the new variant persists the same propositions persist does`() { + val viaPersist = DeduplicatingRepository() + val viaCanonical = DeduplicatingRepository() + val one = proposition("Alice likes coffee", id = "one") + val two = proposition("Bob likes tea", id = "two") + + persistable(one, two).persist(viaPersist, TrackingEntityRepository(schema)) + persistable(one, two).persistReturningCanonical(viaCanonical, TrackingEntityRepository(schema)) + + assertThat(viaCanonical.findAll().map { it.id }.sorted()) + .isEqualTo(viaPersist.findAll().map { it.id }.sorted()) + } + + // ---- the decorator keeps emitting ---- + + @Test + fun `the event-emitting decorator emits for the new call and reports canonical ids`() { + // The by-delegate trap, asserted rather than argued. `PropositionRepository by delegate` + // generates a forwarder for every interface member the class does not declare, so an + // inherited default body would run against the *delegate's* save and this decorator would + // emit nothing. The override is what stops that, and nothing else in the suite would notice + // if it were deleted. + val delegate = DeduplicatingRepository() + delegate.save(proposition("Alice likes coffee", id = "canonical")) + val emitted = mutableListOf() + val decorated = EventEmittingPropositionRepository(delegate) { emitted += it } + + val result = decorated.saveAllReturningCanonical( + listOf( + proposition("Alice likes coffee", id = "minted"), + proposition("Bob likes tea", id = "fresh"), + ), + ) + + assertThat(emitted.filterIsInstance().map { it.proposition.id }) + .describedAs("one event per proposition, carrying what was stored") + .containsExactly("canonical", "fresh") + assertThat(result.canonicalIdOf("minted")).isEqualTo("canonical") + assertThat(result.canonicalIdOf("fresh")).isEqualTo("fresh") + } + + // ---- run identity stays out of source provenance ---- + + @Test + fun `run identity is not part of provenance or locator equality`() { + // Two claims read from one source under two different runs have equal provenance, and this + // is where that is nailed down. Folding run identity into source identity would make + // evidence from two runs over one document look like evidence from two documents — and it + // would change what `SourceLocator.key()` means, which is the `:Source` node's key. + val locator = UriLocator("https://example.com/doc") + val underOneRun = ProvenanceEntry(locator = locator, chunkId = "chunk-1") + val underAnotherRun = ProvenanceEntry(locator = locator, chunkId = "chunk-1") + + assertThat(underOneRun).isEqualTo(underAnotherRun) + assertThat(underOneRun.hashCode()).isEqualTo(underAnotherRun.hashCode()) + assertThat(locator.key()).isEqualTo(UriLocator("https://example.com/doc").key()) + + // Structural, not just behavioural: neither type has anywhere to put a run. + val provenanceFields = ProvenanceEntry::class.java.declaredFields.map { it.name } + assertThat(provenanceFields).noneMatch { it.contains("run", ignoreCase = true) } + assertThat(SourceLocator::class.java.declaredFields.map { it.name }) + .noneMatch { it.contains("run", ignoreCase = true) } + } +} diff --git a/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionInvocationIdentityTest.kt b/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionInvocationIdentityTest.kt index 712fc2aa..58c8c39d 100644 --- a/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionInvocationIdentityTest.kt +++ b/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionInvocationIdentityTest.kt @@ -15,6 +15,7 @@ */ package com.embabel.dice.proposition.extraction +import com.embabel.dice.provenance.SourceRevisionRef import org.assertj.core.api.Assertions.assertThat import org.assertj.core.api.Assertions.assertThatIllegalArgumentException import org.junit.jupiter.api.Test @@ -65,11 +66,74 @@ class ExtractionInvocationIdentityTest { val run = runWith(completed) - // Stored in arrival order, read back in plan order. - assertThat(run.invocations.map { it.invocationIndex }).containsExactly(2, 0, 3, 1) + // Handed to the run in arrival order, held in plan order. Arrival order is not a fact about + // the run — the same four calls answered in a different sequence are the same run — and + // equals compares this list, so normalizing at construction is what makes those two runs + // equal and what makes the two store backends agree. + assertThat(run.invocations.map { it.invocationIndex }).containsExactly(0, 1, 2, 3) assertThat(run.invocationsInPlanOrder().map { it.invocationIndex }).containsExactly(0, 1, 2, 3) } + @Test + fun `two runs whose calls came back in different orders are equal`() { + // The whole reason invocations are normalized. Before, these two compared unequal, and a + // durable store — which keeps identified rows and reads them back in plan order — could not + // return a run equal to the one an in-memory store returned for the same call sequence. + val plan = ExtractionInvocationRecord.plan(3) + val answered = { order: List -> + runWith( + order.map { planIndex -> + plan[planIndex].copy( + outcome = ExtractionInvocationOutcome.SUCCEEDED, + startedAt = startedAt, + finishedAt = startedAt.plusSeconds(1), + ) + }, + ) + } + + val arrivedForwards = answered(listOf(0, 1, 2)) + val arrivedBackwards = answered(listOf(2, 1, 0)) + + assertThat(arrivedForwards).isEqualTo(arrivedBackwards) + assertThat(arrivedForwards.hashCode()).isEqualTo(arrivedBackwards.hashCode()) + assertThat(arrivedForwards.toString()).isEqualTo(arrivedBackwards.toString()) + } + + @Test + fun `retries of one call sort after that call's first attempt and before the next call`() { + val run = runWith( + listOf( + ExtractionInvocationRecord.planned(1).retry(), + ExtractionInvocationRecord.planned(2), + ExtractionInvocationRecord.planned(0), + ExtractionInvocationRecord.planned(1), + ), + ) + + assertThat(run.invocations.map { it.id }).containsExactly( + ExtractionInvocationId(0, 1), + ExtractionInvocationId(1, 1), + ExtractionInvocationId(1, 2), + ExtractionInvocationId(2, 1), + ) + } + + @Test + fun `the order sources were read in is data and is left alone`() { + // Normalization is for invocations only. Two runs that read the same sources in different + // orders read them in different orders, and that is a fact about the run. + val first = SourceRevisionRef("source-a", "rev-1") + val second = SourceRevisionRef("source-b", "rev-1") + + val forwards = runWith(emptyList(), sourceRevisions = listOf(first, second)) + val backwards = runWith(emptyList(), sourceRevisions = listOf(second, first)) + + assertThat(forwards.sourceRevisions).containsExactly(first, second) + assertThat(backwards.sourceRevisions).containsExactly(second, first) + assertThat(forwards).isNotEqualTo(backwards) + } + @Test fun `an attempt numbers a retry of the same call`() { val first = ExtractionInvocationRecord.planned(2).copy( @@ -268,11 +332,15 @@ class ExtractionInvocationIdentityTest { .map { it.name } .toSet() - private fun runWith(invocations: List): ExtractionRun = ExtractionRun( + private fun runWith( + invocations: List, + sourceRevisions: List = emptyList(), + ): ExtractionRun = ExtractionRun( contextId = ExtractionRunFixtures.CONTEXT, lineage = ExtractionRunLineage.root(ExtractionRunFixtures.RUN), status = ExtractionRunStatus.RUNNING, startedAt = startedAt, + sourceRevisions = sourceRevisions, invocations = invocations, ) } 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 c0bdb34f..1e180d84 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 @@ -293,6 +293,9 @@ class ExtractionRunValueTypesTest { ExtractionRunNotFoundException::class.java, ExtractionRunConflictException::class.java, com.embabel.dice.common.ExtractionRunTransitioned::class.java, + PropositionRunLinkStore::class.java, + InMemoryPropositionRunLinkStore::class.java, + PropositionRunLinkScopeException::class.java, ).forEach { type -> assertThat(isMarkedExperimental(type)) .describedAs("%s is marked experimental", type.simpleName) diff --git a/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/IncrementalPropositionExtractionTest.kt b/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/IncrementalPropositionExtractionTest.kt index 607bcb4b..549c5c52 100644 --- a/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/IncrementalPropositionExtractionTest.kt +++ b/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/IncrementalPropositionExtractionTest.kt @@ -161,23 +161,36 @@ class IncrementalPropositionExtractionTest { } @Test - fun `the run reference reaches the context and nothing consumes it`() { + fun `the run reference reaches the context by one route and drives only the lineage write`() { // PR #94's review comment was that buildContext accepted a currentRun and put it on the // context while nothing read it, so the parameter came out and the type came out with it. - // The run is back now, on the request, and the same question applies: what reads it? - // Nothing does, and this pins the two facts that make that checkable. + // The run is back on the request, and now one thing does read it: persistAndProject writes + // the lineage link. That is the whole of what a run does. // - // First: persistAndProject — the method that actually saves the extracted propositions — - // takes only a ChunkPropositionResult and never the context that would carry a run. It has - // exactly one overload, so a second one taking the context (a run reference's only route - // into a write) cannot hide behind a check that only confirms one particular arity exists. + // The earlier form of this test asserted that persistAndProject never saw the context at + // all, which was how "nothing consumes the run" stayed checkable. Lineage makes that false + // by design. What replaces it is narrower and still mechanical: the context reaches exactly + // one method that writes, and the only run-shaped thing reachable from there is the lineage + // recorder. A second consumer would have to appear here. val persistAndProjectOverloads = IncrementalPropositionExtraction::class.java .declaredMethods.filter { it.name == "persistAndProject" } assertEquals(1, persistAndProjectOverloads.size, "persistAndProject must have exactly one overload") - assertEquals(1, persistAndProjectOverloads.single().parameterCount) assertEquals( - ChunkPropositionResult::class.java, - persistAndProjectOverloads.single().parameterTypes.single(), + listOf(ChunkPropositionResult::class.java, SourceAnalysisContext::class.java), + persistAndProjectOverloads.single().parameterTypes.toList(), + ) + + // Exactly one private method takes an ExtractionRunRef, and it is the lineage recorder. If a + // second one appears, a run has grown a second effect and the operator rule that a run adds + // a lineage write and nothing else no longer holds. + val runConsumers = IncrementalPropositionExtraction::class.java.declaredMethods + .filter { ExtractionRunRef::class.java in it.parameterTypes } + .map { it.name } + .toSet() + assertEquals( + setOf("recordRunLineage"), + runConsumers, + "a run drives the lineage write and nothing else", ) // Second: the run reaches the context by exactly one route, the request, and reaches it diff --git a/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/RunLineageBinaryCompatibilityTest.kt b/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/RunLineageBinaryCompatibilityTest.kt new file mode 100644 index 00000000..2ac59e76 --- /dev/null +++ b/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/RunLineageBinaryCompatibilityTest.kt @@ -0,0 +1,197 @@ +/* + * 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.DataDictionary +import com.embabel.agent.rag.model.NamedEntity +import com.embabel.agent.rag.service.NamedEntityDataRepository +import com.embabel.dice.common.EntityResolver +import com.embabel.dice.common.Relations +import com.embabel.dice.incremental.ChunkHistoryStore +import com.embabel.dice.pipeline.PropositionPipeline +import com.embabel.dice.projection.graph.GraphProjectionService +import com.embabel.dice.projection.grounding.GroundingWiringService +import com.embabel.dice.proposition.PropositionRepository +import kotlin.jvm.functions.Function1 +import kotlin.jvm.functions.Function2 +import com.embabel.dice.proposition.store.InMemoryPropositionRepository +import io.mockk.mockk +import org.junit.jupiter.api.Assertions.assertSame +import org.junit.jupiter.api.Assertions.assertThrows +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test +import java.util.function.Function + +/** + * The constructor ABI of [IncrementalPropositionExtraction], which run lineage must not move. + * + * **Why this test exists.** Kotlin compiles a constructor with default arguments into one synthetic + * `(...every parameter..., int mask, DefaultConstructorMarker)`, and *that* is the descriptor a + * precompiled Kotlin caller links against whenever it omits any argument. Appending a defaulted + * parameter — even a trailing one, even with `@JvmOverloads` faithfully republishing every Java + * overload — rewrites it, and every such caller fails with `NoSuchMethodError` at runtime. Java + * overload preservation does not cover this, because Java callers never touch the synthetic + * constructor and Kotlin callers using defaults never touch anything else. + * + * So the run link store is not a constructor parameter. It is bound by + * [IncrementalPropositionExtraction.withRunLineage] after construction, which adds a method instead + * of moving a descriptor. + * + * `SourceAnalysisContext` took the opposite trade in the profiles slice and said so: it is a data + * class whose `copy`/`componentN` ABI was explicitly not claimed. This class is not a data class and + * has no such carve-out, so the descriptor has to hold. + */ +class RunLineageBinaryCompatibilityTest { + + /** The 16 parameter types the primary constructor had before run lineage, in order. */ + private val slice8Parameters: List> = listOf( + PropositionPipeline::class.java, + ChunkHistoryStore::class.java, + DataDictionary::class.java, + Relations::class.java, + PropositionRepository::class.java, + NamedEntityDataRepository::class.java, + EntityResolver::class.java, + GraphProjectionService::class.java, + PropositionExtractionProperties::class.java, + Function::class.java, + Function::class.java, + Function2::class.java, + GroundingWiringService::class.java, + Function1::class.java, + Boolean::class.javaPrimitiveType!!, + Function1::class.java, + ) + + private val marker: Class<*> = Class.forName("kotlin.jvm.internal.DefaultConstructorMarker") + + @Test + fun `the synthetic default-argument constructor still has its slice 8 descriptor`() { + // The one a precompiled Kotlin caller links against when it omits any argument. If run + // lineage had been appended to the primary constructor this would carry a + // PropositionRunLinkStore before the mask, and every such caller would get NoSuchMethodError. + val synthetic = IncrementalPropositionExtraction::class.java.declaredConstructors + .map { it.parameterTypes.toList() } + .filter { it.size >= 2 && it[it.size - 1] == marker } + + assertTrue( + slice8Parameters + listOf(Int::class.javaPrimitiveType!!, marker) in synthetic, + "the synthetic default-argument constructor moved; a Kotlin caller compiled against " + + "slice 8 and using any constructor default would now fail with NoSuchMethodError. " + + "Published synthetics: $synthetic", + ) + } + + @Test + fun `every Java overload descriptor that existed before run lineage survives`() { + val published = IncrementalPropositionExtraction::class.java.constructors + .map { it.parameterTypes.toList() } + .toSet() + + // @JvmOverloads publishes one constructor per defaulted-parameter prefix. Nine required + // parameters, then one more overload for each of the seven defaults. + for (arity in 9..16) { + assertTrue( + slice8Parameters.take(arity) in published, + "constructor of $arity arguments no longer published", + ) + } + } + + @Test + fun `no published constructor mentions the run link store`() { + // The positive statement of the rule: run lineage is bound by a method, so it appears in no + // constructor descriptor at all — synthetic or published. + val everyConstructorParameter = IncrementalPropositionExtraction::class.java.declaredConstructors + .flatMap { it.parameterTypes.toList() } + + assertTrue( + everyConstructorParameter.none { it == PropositionRunLinkStore::class.java }, + "PropositionRunLinkStore reached a constructor descriptor; bind it with withRunLineage", + ) + } + + @Test + fun `run lineage binds once and a second attempt is rejected`() { + // The field is read when an analysis records lineage, not when it starts, so a later + // rebinding would redirect or silently erase the audit record of an extraction already in + // flight. An audit surface should not be swappable at a distance; a second call is a + // programming error and says so. Clearing it with null is the same call and the same answer. + val extraction = extraction() + val first = InMemoryPropositionRunLinkStore(InMemoryExtractionRunStore(), InMemoryPropositionRepository()) + val second = InMemoryPropositionRunLinkStore(InMemoryExtractionRunStore(), InMemoryPropositionRepository()) + + assertSame(extraction, extraction.withRunLineage(first)) + + assertThrows(IllegalStateException::class.java) { extraction.withRunLineage(second) } + assertThrows(IllegalStateException::class.java) { extraction.withRunLineage(null) } + } + + @Test + fun `binding no lineage is still a binding`() { + // `withRunLineage(null)` is how a host says "record none" explicitly. It is one binding, so + // it cannot later be upgraded into one that records — otherwise "bound once" would depend + // on which value was passed. + val extraction = extraction() + extraction.withRunLineage(null) + + assertThrows(IllegalStateException::class.java) { + extraction.withRunLineage( + InMemoryPropositionRunLinkStore(InMemoryExtractionRunStore(), InMemoryPropositionRepository()), + ) + } + } + + private fun extraction() = IncrementalPropositionExtraction( + propositionPipeline = mockk(relaxed = true), + chunkHistoryStore = mockk(relaxed = true), + dataDictionary = DataDictionary.fromClasses("binding"), + relations = Relations.empty(), + propositionRepository = mockk(relaxed = true), + entityRepository = mockk(relaxed = true), + entityResolver = mockk(relaxed = true), + graphProjectionService = mockk(relaxed = true), + properties = PropositionExtractionProperties(), + ) + + @Test + fun `withRunLineage is the binding point and returns the same instance`() { + val bindingPoints = IncrementalPropositionExtraction::class.java.methods + .filter { it.name == "withRunLineage" } + + // Two overloads and no more. The failure policy arrived as a second method, precisely so + // the original descriptor did not move: a default argument on the first would have rewritten + // it, which is the whole reason lineage binds by method and no constructor mentions it. A third entry here means someone added a binding shape + // without deciding what it does to callers of the other two. + assertEquals( + setOf( + listOf(PropositionRunLinkStore::class.java), + listOf(PropositionRunLinkStore::class.java, LineageFailurePolicy::class.java), + ), + bindingPoints.map { it.parameterTypes.toList() }.toSet(), + "the binding points are the one-argument form and the policy-carrying form", + ) + assertEquals(2, bindingPoints.size, "no synthetic or defaulted extra binding point") + bindingPoints.forEach { + assertEquals( + IncrementalPropositionExtraction::class.java, + it.returnType, + "returns the receiver so a bean method can bind it in one expression", + ) + } + } +} diff --git a/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/RunLineageWiringTest.kt b/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/RunLineageWiringTest.kt new file mode 100644 index 00000000..65adb932 --- /dev/null +++ b/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/RunLineageWiringTest.kt @@ -0,0 +1,720 @@ +/* + * 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 com.embabel.agent.core.DataDictionary +import com.embabel.agent.rag.model.NamedEntity +import com.embabel.agent.rag.service.NamedEntityDataRepository +import com.embabel.dice.common.EntityResolver +import com.embabel.dice.common.Relations +import com.embabel.dice.common.Resolutions +import com.embabel.dice.common.SuggestedEntityResolution +import com.embabel.dice.incremental.ChunkHistoryStore +import com.embabel.dice.pipeline.ChunkPropositionResult +import com.embabel.dice.pipeline.PropositionPipeline +import com.embabel.dice.projection.graph.GraphProjectionService +import com.embabel.dice.projection.graph.ProjectedRelationship +import com.embabel.dice.projection.graph.RelationshipPersistenceResult +import com.embabel.dice.projection.grounding.GroundingWiringService +import com.embabel.dice.projection.grounding.GroundingWiringService.GroundingReport +import com.embabel.dice.proposition.EntityMention +import com.embabel.dice.proposition.MentionRole +import com.embabel.dice.proposition.Proposition +import com.embabel.dice.proposition.PropositionPersistenceResult +import com.embabel.dice.proposition.PropositionRepository +import com.embabel.dice.proposition.ProjectionResults +import com.embabel.dice.proposition.SuggestedPropositions +import com.embabel.dice.proposition.store.InMemoryPropositionRepository +import io.mockk.every +import io.mockk.mockk +import io.mockk.slot +import io.mockk.verify +import org.assertj.core.api.Assertions.assertThat +import org.assertj.core.api.Assertions.assertThatThrownBy +import org.junit.jupiter.api.Test +import java.time.Instant +import com.embabel.dice.common.SourceAnalysisRequestEvent +import com.embabel.dice.incremental.IncrementalSource +import com.embabel.chat.Message +import java.util.concurrent.CountDownLatch +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicReference + +/** + * What `persistAndProject` hands to projection, grounding and lineage. + * + * The headline is that a run does not change any of it. Projection and grounding run over the + * propositions the repository returned on every call, so a projection of a deduplicated proposition + * targets the id that is stored whether or not anyone asked for an audit trail. A run adds the + * lineage write and nothing else, and the byte-identical case below is what holds that line: the + * same extraction with and without a run must reach the repository the same way. + * + * These tests deliberately pin the *opposite* of what the first cut of this slice did, where the run + * chose between canonical and pre-save ids. That made an audit setting decide whether the graph was + * written correctly. See `persistAndProject`. + * + * **The survival cases below run with no ambient transaction, and that is the shape they speak + * for.** The repository here commits each save as it makes it, so "the claim is still there after + * the pass failed" means what it says. A host that wraps extraction in its own `@Transactional` has + * a different shape: nothing has committed, the claims and their lineage share that transaction's + * fate, and a failure propagating out of `persistAndProject` rolls all of it back. What these tests + * pin for that host is narrower but still real — the *ordering*, so lineage is attempted before any + * fallible pass rather than after all of them. `DrivineRunLineageIntegrationTest` covers the ambient + * shape, including the server-side failure no catch can survive. + */ +class RunLineageWiringTest { + + private val tenant = ContextId("wiring-tenant") + private val runRef = ExtractionRunRef("wiring-run") + private val startedAt: Instant = Instant.parse("2026-08-31T10:15:30Z") + + /** `DrivinePropositionRepository`'s dedup rule, in memory. */ + private class DeduplicatingRepository( + private val delegate: PropositionRepository = InMemoryPropositionRepository(), + ) : PropositionRepository by delegate { + + override fun save(proposition: Proposition): Proposition { + val existing = delegate.findAll().firstOrNull { + it.contextId == proposition.contextId && + it.text == proposition.text && + it.id != proposition.id + } + val isUpdate = delegate.findById(proposition.id) != null + return if (existing != null && !isUpdate) existing else delegate.save(proposition) + } + + override fun saveAll(propositions: Collection) { + propositions.forEach { save(it) } + } + + override fun saveAllReturningCanonical( + propositions: Collection, + ): PropositionPersistenceResult { + val inputs = propositions.toList() + return PropositionPersistenceResult.of(inputs, inputs.map { save(it) }) + } + } + + private fun proposition(text: String, id: String) = Proposition( + id = id, + contextId = tenant, + text = text, + mentions = listOf( + EntityMention(span = "Alice", type = "Person", resolvedId = "e-alice", role = MentionRole.SUBJECT), + ), + confidence = 0.9, + grounding = listOf("chunk-1"), + ) + + private fun user(): NamedEntity = mockk(relaxed = true).also { + every { it.id } returns tenant.value + every { it.name } returns "Alice" + } + + /** + * A wired-up extractor plus the two capture points: what projection was asked to project, and + * what lineage was asked to link. + */ + private class Harness( + val repository: DeduplicatingRepository, + val extraction: IncrementalPropositionExtraction, + val pipeline: PropositionPipeline, + val chunkResult: ChunkPropositionResult, + val projected: MutableList>, + val grounded: MutableList>, + val linkStore: RecordingLinkStore?, + val entityRepository: NamedEntityDataRepository, + ) { + /** Whether structural wiring reached the entity repository. */ + val structurallyWired: Boolean + get() = runCatching { + verify { entityRepository.mergeRelationship(any(), any(), any()) } + }.isSuccess + } + + private class RecordingLinkStore( + private val runsPresent: Set, + private val failWith: RuntimeException? = null, + ) : PropositionRunLinkStore { + + val linked = mutableListOf>>() + + override fun link(key: ExtractionRunKey, propositionIds: Collection): Int { + failWith?.let { throw it } + if (key !in runsPresent) throw ExtractionRunNotFoundException(key) + linked += key to propositionIds.toList() + return propositionIds.distinct().size + } + + override fun runsOf(contextIdValue: String, propositionId: String, limit: Int) = + linked.filter { propositionId in it.second }.map { it.first.runRef } + + override fun propositionsOf(key: ExtractionRunKey, limit: Int) = + linked.filter { it.first == key }.flatMap { it.second } + } + + private fun harness( + stored: Proposition?, + extracted: Proposition, + linkStore: RecordingLinkStore?, + projectionFails: Boolean = false, + structuralWiringFails: Boolean = false, + groundingFails: Boolean = false, + policy: LineageFailurePolicy = LineageFailurePolicy.DEFAULT, + ): Harness { + val repository = DeduplicatingRepository() + stored?.let { repository.save(it) } + + val pipeline = mockk() + val result = ChunkPropositionResult.Success( + chunkId = "chunk-1", + suggestedPropositions = SuggestedPropositions("chunk-1", emptyList()), + entityResolutions = Resolutions(setOf("chunk-1"), emptyList()), + propositions = listOf(extracted), + ) + every { pipeline.processOnce(any(), any(), any(), any(), any(), any()) } returns result + // The event path reaches the pipeline through the incremental analyzer, which calls + // processChunk; both doors hand back the same extraction. + every { pipeline.processChunk(any(), any()) } returns result + + val projected = mutableListOf>() + val projection = mockk() + val captured = slot>() + every { projection.projectAndPersist(capture(captured)) } answers { + projected += captured.captured + if (projectionFails) throw IllegalStateException("projector is down") + ProjectionResults(emptyList()) to + RelationshipPersistenceResult(persistedCount = 0, failedCount = 0) + } + + // A real grounding service is wired in, so what it is handed is asserted rather than + // assumed. With none supplied the grounding half of "consumes the canonical results" would + // be untested — the call is null-safe and would simply not happen. + val grounded = mutableListOf>() + val grounding = mockk() + val groundingCaptured = slot>() + every { grounding.wire(capture(groundingCaptured)) } answers { + grounded += groundingCaptured.captured + if (groundingFails) throw IllegalStateException("grounding is down") + GroundingReport.EMPTY + } + + // Structural wiring runs through mergeRelationship, so this is where a failure in it comes + // from. It is the first fallible thing after the save, which is what makes it the test for + // where lineage sits. + val entityRepository = mockk(relaxed = true) + if (structuralWiringFails) { + every { entityRepository.mergeRelationship(any(), any(), any()) } throws + IllegalStateException("structural wiring is down") + } + + val extraction = IncrementalPropositionExtraction( + propositionPipeline = pipeline, + chunkHistoryStore = mockk(relaxed = true), + dataDictionary = DataDictionary.fromClasses("wiring"), + relations = Relations.empty(), + propositionRepository = repository, + entityRepository = entityRepository, + entityResolver = mockk(relaxed = true), + graphProjectionService = projection, + // One-message windows, so an event carrying a single message is enough to trigger the + // incremental analyzer on the event path; the direct path ignores these. + properties = PropositionExtractionProperties(windowSize = 1, overlapSize = 1, triggerInterval = 1), + groundingWiringService = grounding, + ).withRunLineage(linkStore, policy) + return Harness(repository, extraction, pipeline, result, projected, grounded, linkStore, entityRepository) + } + + private fun IncrementalPropositionExtraction.remember(currentRun: ExtractionRunRef?) = + rememberText( + "Alice likes coffee", + "source-1", + user(), + emptyList(), + null, + null, + ExtractionRequest(currentRun = currentRun), + ) + + private fun event(currentRun: ExtractionRunRef?, sourceId: String = "event-source"): SourceAnalysisRequestEvent { + val source = mockk>(relaxed = true) + every { source.id } returns sourceId + every { source.size } returns 1 + return object : SourceAnalysisRequestEvent(this, user()) { + override fun incrementalSource(): IncrementalSource = source + + override fun currentRun(): ExtractionRunRef? = currentRun + } + } + + // ---- the async event path is as loud as a direct call ---- + + @Test + fun `an event-driven extraction with a run and no link store fails under STRICT`() { + // Same inputs as the direct-call case above, published as an event. The listener used to + // swallow every exception, which made STRICT a promise the async path did not keep. + val harness = harness( + stored = null, + extracted = proposition("Alice likes coffee", id = "minted"), + linkStore = null, + policy = LineageFailurePolicy.STRICT, + ) + + assertThatThrownBy { harness.extraction.extractPropositions(event(currentRun = runRef)) } + .isInstanceOf(LineageNotRecordedException::class.java) + .hasMessageContaining("no PropositionRunLinkStore is bound") + + // The claim was saved before lineage was attempted and stands; what failed is attribution. + assertThat(harness.repository.findById("minted")).isNotNull() + } + + @Test + fun `a lineage failure on one queued event does not drop the next`() { + // The first event holds the drain open inside the pipeline while two more queue behind it. + // Its lineage failure must not cost those two their turn: the drain keeps going and the + // failure surfaces once, after the queue is empty. + val harness = harness( + stored = null, + extracted = proposition("Alice likes coffee", id = "minted"), + linkStore = null, + policy = LineageFailurePolicy.STRICT, + ) + val firstEntered = CountDownLatch(1) + val release = CountDownLatch(1) + var calls = 0 + every { harness.pipeline.processChunk(any(), any()) } answers { + if (calls++ == 0) { + firstEntered.countDown() + release.await(5, TimeUnit.SECONDS) + } + harness.chunkResult + } + + val surfaced = AtomicReference() + val drainer = Thread { + try { + harness.extraction.extractPropositions(event(currentRun = runRef, sourceId = "source-a")) + } catch (t: Throwable) { + surfaced.set(t) + } + } + drainer.start() + assertThat(firstEntered.await(5, TimeUnit.SECONDS)).isTrue() + // These two find the lock held and queue behind the first event. + harness.extraction.extractPropositions(event(currentRun = runRef, sourceId = "source-b")) + harness.extraction.extractPropositions(event(currentRun = runRef, sourceId = "source-c")) + release.countDown() + drainer.join(10_000) + + assertThat(surfaced.get()).isInstanceOf(LineageNotRecordedException::class.java) + verify(exactly = 3) { harness.pipeline.processChunk(any(), any()) } + assertThat(harness.extraction.isIdle).isTrue() + } + + @Test + fun `under LENIENT an event-driven extraction with no link store completes`() { + val harness = harness( + stored = null, + extracted = proposition("Alice likes coffee", id = "minted"), + linkStore = null, + policy = LineageFailurePolicy.LENIENT, + ) + + harness.extraction.extractPropositions(event(currentRun = runRef)) + + assertThat(harness.repository.findById("minted")).isNotNull() + } + + // ---- canonical persistence is the only path ---- + + @Test + fun `with no run the projection of a deduplicated proposition targets the canonical id`() { + // The discriminating case for "canonical persistence is the only path". Extraction minted + // "minted"; the deduplicating store answered with "canonical" and never stored "minted". + // Projection and grounding must target what the store holds, with no run anywhere in sight. + // + // This is the behavioural fix. Before it, a no-run extraction projected and grounded against + // "minted" — an id the store does not hold — so the edges pointed at a node that was never + // written. + val harness = harness( + stored = proposition("Alice likes coffee", id = "canonical"), + extracted = proposition("Alice likes coffee", id = "minted"), + linkStore = null, + ) + + harness.extraction.remember(currentRun = null) + + assertThat(harness.projected.single().map { it.id }) + .describedAs("projection targets the stored id, with no run involved") + .containsExactly("canonical") + assertThat(harness.grounded.single().map { it.id }) + .describedAs("grounding targets the stored id too") + .containsExactly("canonical") + assertThat(harness.repository.findById("minted")) + .describedAs("the minted id was never stored, which is why projecting it was wrong") + .isNull() + } + + @Test + fun `with no run and a link store present nothing is linked`() { + val links = RecordingLinkStore(runsPresent = setOf(ExtractionRunKey(tenant, runRef))) + val harness = harness( + stored = proposition("Alice likes coffee", id = "canonical"), + extracted = proposition("Alice likes coffee", id = "minted"), + linkStore = links, + ) + + harness.extraction.remember(currentRun = null) + + assertThat(links.linked) + .describedAs("no run, no lineage: the store is bound and never touched") + .isEmpty() + assertThat(harness.projected.single().map { it.id }).containsExactly("canonical") + } + + @Test + fun `a run adds the lineage write and changes nothing else about persistence`() { + // The other half of the operator rule. The no-run and run-present extractions are run over + // identical inputs, and everything except the lineage write has to match. + fun run(currentRun: ExtractionRunRef?): Harness { + val links = RecordingLinkStore(runsPresent = setOf(ExtractionRunKey(tenant, runRef))) + val harness = harness( + stored = proposition("Alice likes coffee", id = "canonical"), + extracted = proposition("Alice likes coffee", id = "minted"), + linkStore = links, + ) + harness.extraction.remember(currentRun = currentRun) + return harness + } + + val without = run(currentRun = null) + val with = run(currentRun = runRef) + + // Compared by id, because a `Proposition` carries creation and revision timestamps taken + // from the wall clock, so two runs of the same extraction differ in fields that have nothing + // to do with the run. The ids are what says which claims were + // wired, which is the thing a run must not change. + fun ids(passes: List>) = passes.map { pass -> pass.map { it.id } } + + assertThat(ids(with.projected)) + .describedAs("a run does not change what projection is handed") + .isEqualTo(ids(without.projected)) + assertThat(ids(with.grounded)) + .describedAs("a run does not change what grounding is handed") + .isEqualTo(ids(without.grounded)) + assertThat(with.repository.findAll().map { it.id }) + .describedAs("the same claims are stored either way") + .isEqualTo(without.repository.findAll().map { it.id }) + + // The one and only difference. + assertThat(without.linkStore!!.linked).isEmpty() + assertThat(with.linkStore!!.linked.single().second).containsExactly("canonical") + } + + // ---- the run-present path consumes canonical results ---- + + @Test + fun `the projection of a deduplicated proposition targets the canonical id`() { + // The same claim as the no-run case above, with a run present. Both hold, which is the point. + val harness = harness( + stored = proposition("Alice likes coffee", id = "canonical"), + extracted = proposition("Alice likes coffee", id = "minted"), + linkStore = RecordingLinkStore(runsPresent = setOf(ExtractionRunKey(tenant, runRef))), + ) + + harness.extraction.remember(currentRun = runRef) + + assertThat(harness.projected.single().map { it.id }).containsExactly("canonical") + assertThat(harness.grounded.single().map { it.id }) + .describedAs("grounding gets the canonical propositions too, not just projection") + .containsExactly("canonical") + assertThat(harness.repository.findById("canonical")).isNotNull() + assertThat(harness.repository.count()).isEqualTo(1) + } + + @Test + fun `a run-present flow links the canonical ids to its run`() { + val links = RecordingLinkStore(runsPresent = setOf(ExtractionRunKey(tenant, runRef))) + val harness = harness( + stored = proposition("Alice likes coffee", id = "canonical"), + extracted = proposition("Alice likes coffee", id = "minted"), + linkStore = links, + ) + + harness.extraction.remember(currentRun = runRef) + + assertThat(links.linked).hasSize(1) + val (key, ids) = links.linked.single() + assertThat(key).isEqualTo(ExtractionRunKey(tenant, runRef)) + assertThat(ids).containsExactly("canonical") + } + + // ---- attribution fails loud by policy ---- + + @Test + fun `a run with no link store bound fails the extraction under STRICT`() { + // The host asked for attribution and nothing can record it. That is a wiring mistake, true + // of every call this extractor will make, and the default policy says so out loud. A + // success with a silent gap in the audit is the outcome being prevented. + val harness = harness( + stored = proposition("Alice likes coffee", id = "canonical"), + extracted = proposition("Alice likes coffee", id = "minted"), + linkStore = null, + policy = LineageFailurePolicy.STRICT, + ) + + assertThatThrownBy { harness.extraction.remember(currentRun = runRef) } + .isInstanceOf(LineageNotRecordedException::class.java) + .hasMessageContaining("no PropositionRunLinkStore is bound") + .hasMessageContaining(runRef.runId) + } + + @Test + fun `STRICT is what an unqualified binding gets`() { + // The default is the whole point of the policy, so it is pinned here; the enum's + // declaration order does not get to decide it quietly. + assertThat(LineageFailurePolicy.DEFAULT).isEqualTo(LineageFailurePolicy.STRICT) + + val extraction = harness( + stored = null, + extracted = proposition("Alice likes coffee", id = "minted"), + linkStore = null, + ).extraction + + assertThatThrownBy { extraction.remember(currentRun = runRef) } + .describedAs("harness bound no policy, so the default had to be the strict one") + .isInstanceOf(LineageNotRecordedException::class.java) + } + + @Test + fun `a run with no link store records nothing and carries on under LENIENT`() { + val harness = harness( + stored = proposition("Alice likes coffee", id = "canonical"), + extracted = proposition("Alice likes coffee", id = "minted"), + linkStore = null, + policy = LineageFailurePolicy.LENIENT, + ) + + harness.extraction.remember(currentRun = runRef) + + assertThat(harness.projected.single().map { it.id }) + .describedAs("the extraction completed, on the canonical ids like every other call") + .containsExactly("canonical") + } + + @Test + fun `a link write that throws fails the extraction under STRICT`() { + val links = RecordingLinkStore( + runsPresent = emptySet(), + failWith = IllegalStateException("link store is down"), + ) + val harness = harness( + stored = null, + extracted = proposition("Alice likes coffee", id = "minted"), + linkStore = links, + policy = LineageFailurePolicy.STRICT, + ) + + assertThatThrownBy { harness.extraction.remember(currentRun = runRef) } + .isInstanceOf(LineageNotRecordedException::class.java) + .hasMessageContaining("could not attribute") + .hasRootCauseMessage("link store is down") + + // The claim was saved before lineage was attempted, and with no ambient transaction it + // stands. The caller learns that a stored claim is unattributed, which is the point. + assertThat(harness.repository.findById("minted")).isNotNull() + } + + @Test + fun `a run whose link store rejects the scope fails the extraction under STRICT`() { + // A scope rejection is a pipeline bug. It has to be at least as loud as an outage. + val links = RecordingLinkStore(runsPresent = emptySet()) + val harness = harness( + stored = null, + extracted = proposition("Alice likes coffee", id = "minted"), + linkStore = links, + policy = LineageFailurePolicy.STRICT, + ) + + assertThatThrownBy { harness.extraction.remember(currentRun = runRef) } + .isInstanceOf(LineageNotRecordedException::class.java) + .hasCauseInstanceOf(ExtractionRunNotFoundException::class.java) + } + + @Test + fun `a structural wiring throw leaves the saved claims attributed`() { + // Lineage now runs right behind the save, ahead of every fallible wiring pass, so a + // structural wiring throw happens after attribution has already succeeded. The claim is + // durable and linked to the run even though the graph around it is incomplete. + val links = RecordingLinkStore(runsPresent = setOf(ExtractionRunKey(tenant, runRef))) + val harness = harness( + stored = null, + extracted = proposition("Alice likes coffee", id = "minted"), + linkStore = links, + structuralWiringFails = true, + ) + + assertThatThrownBy { harness.extraction.remember(currentRun = runRef) } + .isInstanceOf(IllegalStateException::class.java) + .hasMessage("structural wiring is down") + + assertThat(harness.repository.findById("minted")).isNotNull() + assertThat(links.linked.single().second) + .describedAs("the claim was attributed before the failing pass ran") + .containsExactly("minted") + assertThat(harness.projected) + .describedAs("projection never ran, since structural wiring failed ahead of it") + .isEmpty() + } + + @Test + fun `a projector that throws leaves the saved claims attributed`() { + // Projection is the first fallible pass behind structural wiring, and lineage now sits + // ahead of both. A throwing projector reaches the caller with the claim already linked to + // its run. + val links = RecordingLinkStore(runsPresent = setOf(ExtractionRunKey(tenant, runRef))) + val harness = harness( + stored = null, + extracted = proposition("Alice likes coffee", id = "minted"), + linkStore = links, + projectionFails = true, + ) + + assertThatThrownBy { harness.extraction.remember(currentRun = runRef) } + .isInstanceOf(IllegalStateException::class.java) + .hasMessage("projector is down") + + assertThat(harness.repository.findById("minted")).isNotNull() + assertThat(links.linked.single().second) + .describedAs("the link store recorded the canonical ids before the projector ran") + .containsExactly("minted") + assertThat(harness.grounded) + .describedAs("grounding never ran") + .isEmpty() + } + + @Test + fun `a grounding failure leaves the saved claims attributed`() { + // Grounding is the last of the three wiring passes, and lineage now sits ahead of all of + // them. A grounding failure reaches the caller with the claim stored, attributed, structurally + // wired and projected, and no grounding edges. + val links = RecordingLinkStore(runsPresent = setOf(ExtractionRunKey(tenant, runRef))) + val harness = harness( + stored = null, + extracted = proposition("Alice likes coffee", id = "minted"), + linkStore = links, + groundingFails = true, + ) + + assertThatThrownBy { harness.extraction.remember(currentRun = runRef) } + .isInstanceOf(IllegalStateException::class.java) + .hasMessage("grounding is down") + + assertThat(harness.repository.findById("minted")).isNotNull() + assertThat(links.linked.single().second).containsExactly("minted") + assertThat(harness.projected.single().map { it.id }).containsExactly("minted") + assertThat(harness.grounded.single().map { it.id }) + .describedAs("grounding ran and threw, so it was reached") + .containsExactly("minted") + } + + // ---- the end state a lineage failure leaves behind ---- + + @Test + fun `a STRICT lineage failure leaves the claims saved with nothing built around them yet`() { + // The discriminating test for where lineage sits. Lineage runs first now, right behind the + // save, so when its own write is the thing that fails, none of structural wiring, + // projection or grounding has run at all. The state the caller is left with is the save and + // nothing past it, which is the honest report of "attribution failed before anything else + // was asked to run on this claim". + val links = RecordingLinkStore( + runsPresent = emptySet(), + failWith = IllegalStateException("link store is down"), + ) + val harness = harness( + stored = null, + extracted = proposition("Alice likes coffee", id = "minted"), + linkStore = links, + policy = LineageFailurePolicy.STRICT, + ) + + // The operation is reported as failed. + assertThatThrownBy { harness.extraction.remember(currentRun = runRef) } + .isInstanceOf(LineageNotRecordedException::class.java) + .hasRootCauseMessage("link store is down") + + // ...and nothing after the save ran. + assertThat(harness.repository.findById("minted")) + .describedAs("claims persisted") + .isNotNull() + assertThat(harness.structurallyWired) + .describedAs("structural edges were never wired") + .isFalse() + assertThat(harness.projected) + .describedAs("projection never ran") + .isEmpty() + assertThat(harness.grounded) + .describedAs("grounding never ran") + .isEmpty() + assertThat(links.linked) + .describedAs("and the link write is the one that failed") + .isEmpty() + } + + @Test + fun `a LENIENT lineage failure logs the gap and runs every later pass anyway`() { + val links = RecordingLinkStore( + runsPresent = emptySet(), + failWith = IllegalStateException("link store is down"), + ) + val harness = harness( + stored = null, + extracted = proposition("Alice likes coffee", id = "minted"), + linkStore = links, + policy = LineageFailurePolicy.LENIENT, + ) + + // Reported as success, and unlike the STRICT case, every later pass still runs: LENIENT + // logs the gap and carries on, without stopping the pipeline where lineage failed. + harness.extraction.remember(currentRun = runRef) + + assertThat(harness.repository.findById("minted")).isNotNull() + assertThat(harness.structurallyWired).isTrue() + assertThat(harness.projected.single().map { it.id }).containsExactly("minted") + assertThat(harness.grounded.single().map { it.id }).containsExactly("minted") + assertThat(links.linked).isEmpty() + } + + @Test + fun `lineage that cannot be written does not fail the extraction under LENIENT`() { + // The documented downgrade. A host that has decided the claims outweigh their audit trail + // says so in configuration and gets the old best-effort behaviour, explicitly. + val links = RecordingLinkStore(runsPresent = emptySet(), failWith = IllegalStateException("no store")) + val harness = harness( + stored = null, + extracted = proposition("Alice likes coffee", id = "minted"), + linkStore = links, + policy = LineageFailurePolicy.LENIENT, + ) + + harness.extraction.remember(currentRun = runRef) + + assertThat(harness.repository.findById("minted")).isNotNull() + assertThat(harness.projected.single().map { it.id }).containsExactly("minted") + assertThat(links.linked).isEmpty() + } +} diff --git a/docs/design/extraction-runs.md b/docs/design/extraction-runs.md index 00ae9803..7b1a4009 100644 --- a/docs/design/extraction-runs.md +++ b/docs/design/extraction-runs.md @@ -12,10 +12,11 @@ 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 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. +This note covers DICE #67's value model, its store contract, the Drivine implementation, and the +proposition-to-run relation — the types in `com.embabel.dice.proposition.extraction`, the lifecycle +state machine that governs a run's status, the reads a store owes, and how a stored claim gets +traced back to the runs that produced it. The run coordinator and its REST exposure are a separate +slice. ## What a run holds @@ -134,9 +135,9 @@ The two checks a record can fail on its own terms do apply: a finish cannot prec an `IN_FLIGHT` attempt has not finished. Completion order writes into identities that already exist. There is no factory that takes a -position in a result list, and the run stores records in whatever order they arrived while -`invocationsInPlanOrder()` reads the plan back out. A run rejects two records with the same -`(invocationIndex, attempt)`. +position in a result list, and a run holds its records in plan order however they were handed in — +arrival order is not a fact about the run, and `equals` compares the list. A run rejects two records +with the same `(invocationIndex, attempt)`. ## The root run reference, and why it is denormalized @@ -982,6 +983,269 @@ the list is empty leaves the run aliasing a list the caller still holds, and the afterwards; it fails later and stranger than the non-empty case. The copies are unmodifiable, so the list a caller reads back cannot be edited either. +## Lineage: which runs produced which claims + +The gap #67 opens with is that a stored claim cannot be traced to the run that produced it. This is +the part that closes it. + +```mermaid +flowchart LR + P["(:Proposition)
id, contextId"] + S["(:Source)
key"] + R1["(:ExtractionRun)
run-1"] + R2["(:ExtractionRun)
run-2"] + P -->|DERIVED_FROM| S + P -->|PRODUCED_BY_RUN| R1 + P -->|PRODUCED_BY_RUN| R2 +``` + +Two runs over identical content leave **one proposition, one source grounding, two run links**. Each +part of that is a separate way to get it wrong, and `DrivineRunLineageIntegrationTest` measures all +three on a real graph. + +### Only canonical ids go in + +`DrivinePropositionRepository.save` answers a fresh insert of text it already holds with the +proposition it already holds — a different id from the one extraction minted. That has always been +true and callers have never been able to see it, because `saveAll` returns `Unit`. Any edge written +afterwards against the minted id points at a node that was never stored. + +So a second save call keeps the answer: + +- `PropositionStore.saveAllReturningCanonical` — the same writes as `saveAll`, returning a + `PropositionPersistenceResult`: the stored proposition per input, in input order, plus the + input-id to stored-id map. `saveAll` keeps its `Unit` descriptor; changing it would break every + implementation and every compiled caller. +- `PersistablePropositions.persistReturningCanonical` — the same persistence `persist` does, with + structural relationships wired against what the repository returned. `persist` is untouched. + +`DrivinePropositionRepository` needed no change at all. Its `save` already returned the canonical +proposition; the id was being dropped one layer up, in `saveAll` and in `persist`. + +The result type has two views and they are not interchangeable. `canonicalPropositions` is +positional — one entry per input, so it can repeat when two inputs deduplicate onto one. +`canonicalIds` is the distinct set, which is what a write that should happen once per stored +proposition uses. When one batch names an id twice, every position reports the store's last answer +for it — a replace-by-id store overwrites the first, so the first object is stale the moment the +second save lands, and both answers carry the same id so comparing ids cannot catch it. Resolving +rather than rejecting, because this runs after the saves. + +The store is bound to the extractor with `IncrementalPropositionExtraction.withRunLineage(store)` +rather than a constructor parameter. Binding is **one-time** — a second call throws +`IllegalStateException`, null included — because the field is read when an analysis records lineage +rather than when it starts, so a later call would redirect or silently erase the audit record of an +extraction already in flight. Kotlin compiles a constructor with default arguments into one +synthetic `(..., int mask, DefaultConstructorMarker)`, and appending a defaulted parameter +rewrites the descriptor every precompiled Kotlin caller using a default links against. Adding a +method adds API; appending a defaulted parameter moves one. + +### The relation + +`(:Proposition {id, contextId})-[:PRODUCED_BY_RUN]->(:ExtractionRun {contextId, runId})`, behind +`PropositionRunLinkStore`. + +Many-to-many in both directions, and it has to be. One run produces many claims. One claim is +produced by many runs — not an edge case, but the normal outcome of re-extraction, where the second +run's insert deduplicates onto a proposition the first run created. Both runs are true answers to +"what produced this?", and a single-valued field would have to pick one silently. + +The edge carries no properties. A bare edge says one thing and has nothing for a replay to disagree +about, which is what lets the write be a plain `MERGE`. A timestamp would need `ON CREATE SET` to +stay idempotent and would duplicate the run header's `startedAt`. + +It is named for the run rather than left as a bare `PRODUCED_BY` because at least three things in +DICE produce a proposition — an extraction run, a collector run, and later a #68 commit. The target +label disambiguates a pattern; the name has to disambiguate a grep. + +**No new schema.** Both endpoint labels already carry the uniqueness constraints these statements +seek on, and a relationship has no key of its own: `MERGE` on a pattern between two matched nodes +creates at most one edge, whoever else is writing. `ExtractionRunSchema.specs()` is unchanged, so a +host that already declared it needs no migration. + +### Tenant-guarded on the write, fail-closed on the reads + +Every statement names `contextId` on both endpoints, so a cross-tenant edge is not expressible and +the reads fail closed for free. A write needs more than that, because "matched nothing" and "you +asked to link a neighbour's claim" are the same silence: `link` resolves the run and then every +proposition inside the run's tenant, and names what did not resolve +(`PropositionRunLinkScopeException`). One out-of-scope id rejects the whole batch and writes nothing. + +A proposition id that resolves in another tenant and one nobody holds are the same answer from +inside a tenant, and they mean the same thing: this run cannot claim to have produced it. + +**The preflight names; the write decides.** Validation and the `MERGE` are separate statements, so +under read-committed isolation a proposition deleted or re-tenanted between them passes the check +and is gone by the time the write runs. The Drivine statement therefore counts its own matches — +`WHERE size(ps) = $expected` — in the same snapshot it writes in, so either every proposition is +present and all the edges are written or none of them are; the caller then compares the returned +count against the batch size and rolls the transaction back on any mismatch. The preflight stays +because it is what can name the ids in the error. + +**Both reads resolve against live endpoint state**, not against a remembered link. Deleting a +proposition removes its lineage from both directions: on a graph because the edge is detached with +the node, and in the reference implementation because the reads filter through the proposition store +rather than answering from their own map. Without that the in-memory backend would keep reporting +lineage for claims the store no longer holds, and the two backends would disagree. + +The reference implementation also prunes as it reads: a link whose proposition the store no longer +holds at all is dropped when a read resolves it, and a run with no links left goes with it, +so the map does not keep growing with claims that no longer exist. Nothing reads on a host's +behalf, though, so the store also caps how many runs it keeps links for: a constructor parameter, +`maxRuns`, defaulting to 10,000 like the reference run store's. Past the cap a link for a new run +evicts the links of the runs linked earliest, oldest first, as long as the run store says the run +has ended; a run still `RUNNING` keeps its links, and a store where every linked run is running +grows past the cap and logs that once. A host wanting real retention uses the durable store, where +the edge lives and dies with its endpoints. + +Both reads are bounded by a positive limit and ordered by id ascending. Ordering runs newest-first +would mean reading each run's header for its start time; a caller who wants that has +`ExtractionRunStore` and the refs these reads return. + +### Run identity stays out of source provenance + +Nothing in the lineage touches `ProvenanceEntry` or `SourceLocator`. Two claims read from the same +source under two different runs have equal provenance, and that is asserted rather than assumed — +both behaviourally and structurally, by reading the two types' fields. + +Folding run identity into source identity would make evidence from two runs over one document look +like evidence from two documents, and it would change what `SourceLocator.key()` means, which is the +`:Source` node's key. "Where did this come from" and "which execution wrote it down" are different +questions with different answers, and the second one lives in the relation. + +### Canonical persistence is the only path + +`IncrementalPropositionExtraction.persistAndProject` wires everything downstream of the save against +what the repository returned, on every call: + +| | structural wiring | projection | grounding | run links | +|---|---|---|---|---| +| run present | canonical | canonical | canonical | canonical | +| no run | canonical | canonical | canonical | none | + +**A run adds a row to the last column and changes nothing else.** An earlier cut of this slice made +the run the switch: canonical ids with one, pre-save ids without. Both rows were defensible on their +own terms — the canonical one is correct, and leaving the other alone avoided changing behaviour for +hosts that never asked for extraction runs — but together they meant an audit setting decided whether +the graph was written correctly. Turning on lineage silently changed which nodes the edges pointed +at; leaving it off kept writing edges against ids the store does not hold whenever dedup or a merge +substituted a canonical proposition. + +Audit metadata never changes product behaviour. So the correct row became the only row, and the +no-run change is declared as the behavioural fix it is. A test runs the same extraction with and +without a run and compares everything except the lineage write. + +Lineage's write joins a caller's transaction rather than opening its own. `REQUIRES_NEW` would +guarantee a failure could never touch the caller, but it suspends that transaction, and a suspended +transaction's uncommitted propositions are invisible — so a host wrapping extraction in +`@Transactional` would get fail-closed lineage on every extraction. Joining means lineage resolves +the claims it is about and commits with them. + +The risk `REQUIRES_NEW` would have removed is Spring marking a participating transaction +rollback-only when an inner method throws. That marking *is* propagated here — Drivine overrides +`doSetRollbackOnly` and the shared transaction object's flag is set — but the flag is write-only: +`DrivineTransactionObject` does not implement `SmartTransactionObject`, so Spring's +`isGlobalRollbackOnly` cannot see it, and Drivine never reads it when committing. Propagated, then +dropped. Because that is behaviour rather than contract it is pinned by a test, which goes red if +Drivine implements `SmartTransactionObject` or starts reading the flag. + +**The guarantee covers application-level failures only, and this is the slice's known limitation.** +Everything `link` raises by itself is thrown after its statements succeeded, so the transaction is +healthy and a caller that catches can carry on. A statement that fails *at the server* — a deadlock +between two runs linking overlapping propositions is the realistic case, since nothing orders the +node locks two concurrent `MERGE` batches take over the same propositions — terminates the +transaction beneath Spring, and no catch can undo it. A test injects exactly that and measures the +cost: the wrapping host's later writes are lost. + +That window belongs to the ambient-transaction shape only; a host that does not wrap extraction is +unaffected, because each save has already committed. It closes when the run coordinator commits +claims before recording lineage (slice 10), which makes lineage a write that does not share a +caller's fate. + +Lineage is written **right after the save, before structural wiring, projection or grounding have +run**. `persistReturningCanonical` is split in two for this: `persistCanonicalPropositions` writes +the claims and `wireStructuralRelationships` writes the chunk/entity edges, and lineage runs between +them. A stored claim is attributed the moment it exists, not once the rest of the pipeline has also +succeeded on it. The claims are written at that point, committed with the caller's transaction where +one wraps the call, immediately otherwise, and attribution is a statement about a claim that exists, +checked before anything else is asked to run on it. + +### Attribution fails loud + +A link that cannot be written fails the extraction, under the default `LineageFailurePolicy.STRICT`. +Two cases: an analysis carrying a run with no `PropositionRunLinkStore` bound, and a link write that +throws. + +The first cut logged both and carried on, reasoning that a missing link is a truthful gap in the +audit where a throw would surface as an extraction that appears to have produced nothing. The +argument does not survive asking who reads the gap. A host binds lineage because it wants every +stored claim traceable to the run that produced it; the moment that property is worth something is +after an incident, reconstructing what a run wrote, and a gap discovered then is indistinguishable +from "no run produced this claim". A warning logged weeks earlier by an extraction that reported +success is not a control. The missing-store case is worse still: it is a wiring mistake, identical on +every call, and silence means a host can run for months believing it has an audit trail it never had. + +`LineageFailurePolicy.LENIENT` is the documented downgrade for a host that has weighed the claims +against their attribution and chosen the claims. It is chosen once, in configuration, in the open. + +#### The end state a lineage failure leaves behind + +This is the reason lineage sits right behind the save now, not at the end of `persistAndProject`. + +When a `STRICT` lineage failure raises, the claims it was attributing are saved and nothing past +that point has run: + +| | state after a STRICT lineage failure | +|---|---| +| canonical claims | persisted | +| structural edges | **not wired** | +| graph projection | **not run** | +| grounding | **not run** | +| `PRODUCED_BY_RUN` edge | **absent** | +| operation | **reported as failed** | + +The claims exist and the audit says so honestly: they are not attributed, and the rest of the +pipeline never touched them either. A caller that catches the exception knows precisely what it +has: claims saved with nothing built around them yet. + +`LENIENT` reaches a different end state and reports success: the link write is skipped and every +later pass still runs, because logging the gap and carrying on is what that policy is for. + +An earlier cut of this slice ran lineage last, after all three wiring passes, on the reasoning that +a `STRICT` failure should leave a complete extraction behind with only the audit edge missing. That +ordering meant a claim could sit fully wired, projected and grounded with nobody able to say which +run produced it, for as long as the pipeline kept running past the save. Attribution is checked +before any of that work happens now, so a lineage failure means the later passes never run, and the +honest report of that is a saved, unattributed claim, not a finished one with a gap. + +Under a host's ambient transaction the save and the lineage write share one fate, so a STRICT +failure rolls both back, which is what a host running strict attribution is asking for. + +The same rule holds on the async path. `extractPropositions`, the entry point the +`@Async @EventListener` calls, swallows and logs every other failure because nobody is waiting on +the result, but a `LineageNotRecordedException` under `STRICT` is logged at `error` with the run +key and rethrown to the publisher, so an event-published extraction fails exactly as loud as a +direct call with the same inputs. One failing event does not cost the rest of the queue: the drain +keeps going, remembers the first failure, and raises it once the queue it can see is empty. A host +dispatching those events through an async multicaster sees the failure in its executor's error +handler, which is where every uncaught listener exception goes. + +The policy binds with the store, `withRunLineage(store, policy)`, and failures raise +`LineageNotRecordedException` carrying the store's own exception as its cause, so a scope rejection +and a database outage stay distinguishable. An analysis that saved nothing records nothing and fails +under neither policy: there is no claim for the audit to be missing. + +### The invocation order fix that rides here + +`ExtractionRun.invocations` is normalized to plan order — `(invocationIndex, attempt)` — at +construction. It used to keep whatever order the caller supplied, which is the order calls came +back, which is not a fact about the run: the same four calls answered in a different sequence are +the same run. Since `equals` compares the list element by element, the old behaviour made those two +runs unequal, and it made the two backends disagree on one call sequence — a durable store keeps +identified rows and reads them back in plan order, while the in-memory one handed back the order it +was given. `invocationsInPlanOrder()` is now the identity. + +`sourceRevisions` is deliberately not normalized. The order sources were read in is data. + ## Status: EXPERIMENTAL Every type in this slice carries `@ApiStatus.Experimental`, the marker DICE already uses for API @@ -994,22 +1258,38 @@ public surface. ## What is not here yet -- **No auto-configuration.** `DrivineExtractionRunStore` is a bean a host declares itself, along - with the `SchemaCatalog` carrying `ExtractionRunSchema.specs()`. An `ExtractionRunAutoConfiguration` - arrives with the coordinator. +- **Auto-configured, and still inert.** `dice-storage-autoconfigure` registers + `DrivineExtractionRunStore`, `DrivinePropositionRunLinkStore` and the `SchemaCatalog` carrying + `ExtractionRunSchema.specs()` under the graph-backend condition, each behind + `@ConditionalOnMissingBean`. Having the beans changes nothing until a caller names a run. - **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 run coordinator behind the relation.** Attribution from a claim to the run that produced it + now exists — `PRODUCED_BY_RUN`, written on canonical saved ids — and run identity stays out of + source-provenance equality. What is still missing is the coordinator that mints and terminalizes + the run around it. - **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. 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. + needs the reference brings its implementation. Nothing on a run header holds one yet; attaching + replay material to the header is a later slice. +- **No REST exposure for lineage.** Nothing surfaces run lineage over HTTP. That arrives with the + coordinator. +- **No per-run policy override.** `LineageFailurePolicy` is bound once on the extractor, so a host + runs every extraction under one posture. Choosing strict attribution for one run and lenient for + the next needs the coordinator, which is where a run's own settings will live. +- **Nothing writes lineage for a run the host did not mint.** `persistAndProject` links what it + persisted when the analysis carries a `currentRun`, but nothing constructs or terminalizes the run + around it yet — the host supplies the ref and owns the run's lifecycle until the coordinator lands. +- **Dedup unions evidence but not grounding.** When a second extraction deduplicates onto an + existing proposition, the repository unions the incoming provenance into it and keeps the stored + proposition's `grounding` and `mentions`. So a re-extraction from a *different* chunk that produced + identical text contributes no new `HAS_PROPOSITION` edge, which shows up as a missing edge. + Unioning them is a repository change with its own test burden and is not this slice. - **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