From 325d6197dfa0dc882a999265e941b92d0ac2569c Mon Sep 17 00:00:00 2001 From: James Dunnam <7660553+jimador@users.noreply.github.com> Date: Mon, 31 Aug 2026 08:31:45 -0400 Subject: [PATCH 1/3] Carry versioned extraction profiles and a run ref through entry points MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ExtractionContentProfileRef gives source-independent extraction behavior a stable name-and-version identity, and ExtractionRunRef ships ahead of the run store that will key on it; both are opaque, and the host authorizes and binds what they name. The context carries them beside perspective, schema, and tenant as independent dimensions, proven by a constructional matrix test. Every legacy entry-point signature stays present, open, and reached: the pre-profile declarations delegate forward to the wide forms, the wide forms never route back, and subclass overrides of the old signatures still intercept file calls — pinned by descriptor, finality, compile-level override, and dispatch tests, because overridable and reached are different claims. Signed-off-by: James Dunnam <7660553+jimador@users.noreply.github.com> --- CHANGELOG.md | 83 +++ .../ConversationAnalysisRequestEvent.kt | 24 +- .../dice/common/SourceAnalysisContext.kt | 30 + .../dice/common/SourceAnalysisRequestEvent.kt | 18 + .../extraction/ExtractionContentProfileRef.kt | 87 +++ .../extraction/ExtractionRunRef.kt | 68 +++ .../IncrementalPropositionExtraction.kt | 192 +++++- .../ExtractionProfileJavaInteropTest.java | 405 ++++++++++++ .../ExtractionProfileCompatibilityTest.kt | 183 ++++++ .../ExtractionContextIndependenceTest.kt | 256 ++++++++ .../SourceAnalysisRequestEventProfileTest.kt | 134 ++++ .../ExtractionProfileContractTest.kt | 144 +++++ .../IncrementalPropositionExtractionTest.kt | 576 +++++++++++++++++- docs/design/INDEX.md | 4 + docs/design/extraction-profiles.md | 268 ++++++++ 15 files changed, 2442 insertions(+), 30 deletions(-) create mode 100644 dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionContentProfileRef.kt create mode 100644 dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunRef.kt create mode 100644 dice/src/test/java/com/embabel/dice/ExtractionProfileJavaInteropTest.java create mode 100644 dice/src/test/kotlin/com/embabel/dice/ExtractionProfileCompatibilityTest.kt create mode 100644 dice/src/test/kotlin/com/embabel/dice/common/ExtractionContextIndependenceTest.kt create mode 100644 dice/src/test/kotlin/com/embabel/dice/common/SourceAnalysisRequestEventProfileTest.kt create mode 100644 dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionProfileContractTest.kt create mode 100644 docs/design/extraction-profiles.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 0aba96c2..312b1907 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1078,3 +1078,86 @@ and the consumer PRs that deliver it). on its propositions — subclasses that do not override are unaffected, since both default to null. No stored data migrates: `sourceRevision` stays absent from the JSON of a revisionless entry, which is byte-identical to what was written before. + +- **EXPERIMENTAL.** Versioned extraction content profiles and an extraction run reference carried + through the extraction entry points (DICE #66). `ExtractionContentProfileRef(name, version)` names + a version of a host's content profile — the host's durable answer to what extraction of this kind + of material should do. DICE carries the two strings and nothing else: it never looks a profile up, + never reads policy out of it, and **selects no provider, model, or credential from it**. The host + owns the catalog, authorizes the reference, and binds it to whatever it means. Identity is name + and version together, so republishing a profile under a new version yields a distinct reference + and runs attributed to the older one stay attributed to it. `ExtractionRunRef(runId)` is identity + and nothing else — no timing, status, counts, or lineage — and ships ahead of the durable + extraction run it will key in DICE #67, so the entry points and the run model meet at an opaque + string rather than at a type one has to import from the other's release. Passing a reference for + a run is always legal to carry, and DICE does not check that the run exists — there is nowhere + yet to check against, and what a store does with a reference to a run it has never seen is #67's + decision rather than a promise made here. Both types validate + non-blank components and cap their lengths (256 for a profile name or a run id, 64 for a version), + because #67 stores and indexes these strings and a reference is an identifier rather than a place + to put a payload. Neither is an authorization token, and neither may carry a direct identifier or + a dereferenceable secret. `SourceAnalysisContext` gains optional `profile` and `currentRun`, both + defaulting to null, with `withProfile` and `withCurrentRun` copy helpers. Neither is checked + against any other field: a `sourceRevision` is coupled to its `sourceLocator` because it names a + version of a specific source, while a profile and a run reference are independent of everything + else, and the `init` block says so rather than inventing a relationship the contract does not + have. `rememberText`, `rememberTextFromSource`, `rememberFile` and `rememberFileFromSource` each + take `profile` and `currentRun` as trailing arguments — extra arguments rather than new method + names, which is the opposite of Wave A's `rememberTextFromSource` split, because a locator is + *required* by the source-aware calls and a profile is optional everywhere. Each of the four is + now two declarations: the pre-profile signature exactly as it was, delegating to a new + maximum-arity form that takes the two references. That split is deliberate — `@JvmOverloads` + emits every reduced-arity overload as `final` even on an `open` function, so folding the new + arguments into the existing declarations would have turned each method's pre-change maximum + arity into a final bridge and broken subclasses that override it. Dispatch follows the same + principle: a call that carries no profile and no run takes the chain it took before profiles + existed — the file entry points hand their text to the *pre-profile* text signature, so a + subclass overriding only that one still intercepts file ingestion — while a call that actually + carries a reference goes wide, because the legacy signature cannot express one. The wide forms + are terminal and never route back, so there is no cycle. Unintercepted, every call still ends + at the maximum-arity text form, so overriding that one sees all traffic. + `SourceAnalysisRequestEvent` gains `profile()` and `currentRun()`, both open and null-defaulted, + and `ConversationAnalysisRequestEvent` takes both on its longer constructor. Both paths feed one + `buildContext`, which is what makes the async path carry a profile identically; a test counts + each accessor being read exactly once. Nothing downstream consults either reference — a test + compares the whole context built with a profile against the one built without and asserts they + differ in exactly those two fields. Profile, perspective, schema and tenant stay four independent + dimensions: perspective describes conversational input, a profile is content policy, and a + 64-cell matrix test asserts every combination is constructible, that every ordered pair of + dimensions realises its whole cross product, and that varying one leaves the other three + identical. Design note: + [docs/design/extraction-profiles.md](docs/design/extraction-profiles.md). + **Compatibility: additive, with the same scoped ABI boundary as the Wave A slices.** Source and + Java constructor-descriptor compatibility are claimed. `@JvmOverloads` on `SourceAnalysisContext` + preserves every published constructor descriptor and adds two on the end; a test enumerates + arities 3 through 13 (each with the trailing `DefaultConstructorMarker` Kotlin emits because + `contextId` is a value class) and asserts all of them resolve. Every `rememberText`, + `rememberTextFromSource`, `rememberFile` and `rememberFileFromSource` descriptor survives, with + exactly one added per method name, on the end; a test pins the exact descriptor set of all four + names and that a profile and a run always arrive together as the last two parameters. + **Subclass-override compatibility is part of the claimed surface**: every signature that was + overridable before this slice still is — `rememberText` at six arguments, `rememberFile` at + three, `rememberTextFromSource` at eight, `rememberFileFromSource` at five — and each method's + new maximum-arity form is overridable too. It is proven twice: a reflection test asserts + `Modifier.isFinal` is false on all eight and true on the reduced arities that were already final + bridges, and a Java subclass in the compat suite overrides all four pre-profile signatures, so + the suite compiling is the second proof (`javac` rejects `@Override` on a final method). A + Kotlin test additionally constructs a subclass overriding the six-argument `rememberText` and + the three-argument `rememberFile` and asserts a three-argument call still reaches the override. + Being overridable is not the whole guarantee — the override also has to be reached — so two + further tests pin the dispatch rule: a subclass overriding only the pre-profile text methods + still sees both file entry points, and a file call carrying a profile goes wide instead. + `ConversationAnalysisRequestEvent` keeps its five-argument constructor and gains + six- and seven-argument forms; its `sourceLocator` parameter relaxes from non-null to nullable, + so a publisher can name a profile for material it has no typed source for, and every call that + compiled before still compiles. Full Kotlin synthetic `copy` and `componentN` ABI is **not** + claimed for `SourceAnalysisContext`: two more fields rewrite `copy`, add two `componentN` + methods, and change the synthetic `$default` constructor, so Kotlin code compiled against an + earlier jar must be recompiled rather than swapped in — the same half of the boundary #64 + declined, pinned here by a test asserting exactly one `copy` remains and that it takes thirteen + arguments. No stored data changes and no migration is required: nothing serializes a profile or a + run reference yet. Extraction, resolution, and revision ordering are behaviour-identical; a + profile changes what a run is attributed to, not what it does. `ExtractionContentProfileRef` and + `ExtractionRunRef` both carry `@ApiStatus.Experimental` and their shapes may still move while + #67 lands. A Kotlin `@RequiresOptIn` marker would make that enforceable at the call site rather + than advisory; DICE defines none today and the design note records it as an open question. diff --git a/dice/src/main/kotlin/com/embabel/dice/common/ConversationAnalysisRequestEvent.kt b/dice/src/main/kotlin/com/embabel/dice/common/ConversationAnalysisRequestEvent.kt index 9b57f89f..1d985d62 100644 --- a/dice/src/main/kotlin/com/embabel/dice/common/ConversationAnalysisRequestEvent.kt +++ b/dice/src/main/kotlin/com/embabel/dice/common/ConversationAnalysisRequestEvent.kt @@ -20,6 +20,8 @@ import com.embabel.chat.Conversation import com.embabel.chat.Message import com.embabel.dice.incremental.ConversationSource import com.embabel.dice.incremental.IncrementalSource +import com.embabel.dice.proposition.extraction.ExtractionContentProfileRef +import com.embabel.dice.proposition.extraction.ExtractionRunRef import com.embabel.dice.provenance.SourceLocator import com.embabel.dice.provenance.SourceRevisionRef @@ -29,7 +31,12 @@ import com.embabel.dice.provenance.SourceRevisionRef * * The three-argument constructor is the one that has always existed and carries no * provenance. A publisher that has a typed source for the conversation — a thread in a - * chat system, a transcript file — uses the longer constructor to say so. + * chat system, a transcript file — uses the longer constructor to say so, and the same + * constructor takes an extraction content profile and a run reference. + * + * [sourceLocator] is nullable there because the four things are independent: a publisher can + * name a profile for a conversation it has no typed source for. Only the revision is coupled, + * and to the locator alone — it names a version of that source, so it needs one. */ class ConversationAnalysisRequestEvent( source: Any, @@ -41,15 +48,24 @@ class ConversationAnalysisRequestEvent( private var eventSourceRevision: SourceRevisionRef? = null + private var eventProfile: ExtractionContentProfileRef? = null + + private var eventCurrentRun: ExtractionRunRef? = null + + @JvmOverloads constructor( source: Any, user: NamedEntity, conversation: Conversation, - sourceLocator: SourceLocator, + sourceLocator: SourceLocator?, sourceRevision: SourceRevisionRef? = null, + profile: ExtractionContentProfileRef? = null, + currentRun: ExtractionRunRef? = null, ) : this(source, user, conversation) { eventSourceLocator = sourceLocator eventSourceRevision = sourceRevision + eventProfile = profile + eventCurrentRun = currentRun } override fun incrementalSource(): IncrementalSource = @@ -58,4 +74,8 @@ class ConversationAnalysisRequestEvent( override fun sourceLocator(): SourceLocator? = eventSourceLocator override fun sourceRevision(): SourceRevisionRef? = eventSourceRevision + + override fun profile(): ExtractionContentProfileRef? = eventProfile + + override fun currentRun(): ExtractionRunRef? = eventCurrentRun } 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 c754edf0..bb9174a3 100644 --- a/dice/src/main/kotlin/com/embabel/dice/common/SourceAnalysisContext.kt +++ b/dice/src/main/kotlin/com/embabel/dice/common/SourceAnalysisContext.kt @@ -19,7 +19,9 @@ import com.embabel.agent.core.ContextId import com.embabel.agent.core.DataDictionary import com.embabel.dice.provenance.SourceLocator import com.embabel.dice.provenance.SourceRevisionRef +import com.embabel.dice.proposition.extraction.ExtractionContentProfileRef import com.embabel.dice.proposition.extraction.ExtractionPerspective +import com.embabel.dice.proposition.extraction.ExtractionRunRef /** * Base context for analyzing sources. @@ -39,6 +41,14 @@ import com.embabel.dice.proposition.extraction.ExtractionPerspective * @param sourceRevision optional revision of [sourceLocator] — the provider's own identifier for * the version of that source this run reads. Setting it requires a [sourceLocator] whose key it * matches, so a revision can never name a source the run is not actually reading. + * @param profile optional extraction content profile the host wants this analysis attributed to. + * EXPERIMENTAL. DICE carries the reference and nothing else — it selects no provider, model, or + * credential, and no DICE code reads policy out of it. The host authorizes the profile and binds + * 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. * @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 @@ -65,6 +75,8 @@ data class SourceAnalysisContext @JvmOverloads constructor( */ val mintedEntityProperties: Map = emptyMap(), val sourceRevision: SourceRevisionRef? = null, + val profile: ExtractionContentProfileRef? = null, + val currentRun: ExtractionRunRef? = null, ) { init { @@ -76,6 +88,10 @@ data class SourceAnalysisContext @JvmOverloads constructor( "sourceRevision source key must match sourceLocator source key" } } + // [profile] and [currentRun] are checked against nothing else here, deliberately. A + // revision has to name the source the run is reading, which is why it is coupled to + // [sourceLocator]. A profile and a run reference are independent of every other field, + // and validating them against one would invent a relationship the contract doesn't have. } companion object { @@ -150,6 +166,20 @@ data class SourceAnalysisContext @JvmOverloads constructor( fun withSourceRevision(sourceRevision: SourceRevisionRef): SourceAnalysisContext = copy(sourceRevision = sourceRevision) + /** + * Returns a copy attributed to the given extraction content [profile]. EXPERIMENTAL. + * Changes no other field and no extraction behaviour — see [profile]. + */ + fun withProfile(profile: ExtractionContentProfileRef): SourceAnalysisContext = + copy(profile = profile) + + /** + * Returns a copy that says this analysis belongs to the given extraction run. EXPERIMENTAL. + * Changes no other field and no extraction behaviour — see [currentRun]. + */ + fun withCurrentRun(currentRun: ExtractionRunRef): SourceAnalysisContext = + copy(currentRun = currentRun) + /** * Returns a copy allowing (or forbidding) this analysis to persist NEW entities * for mentions the resolver could not match. See [mintNewEntities]. 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 729929ac..e29e16e3 100644 --- a/dice/src/main/kotlin/com/embabel/dice/common/SourceAnalysisRequestEvent.kt +++ b/dice/src/main/kotlin/com/embabel/dice/common/SourceAnalysisRequestEvent.kt @@ -18,6 +18,8 @@ package com.embabel.dice.common import com.embabel.agent.rag.model.NamedEntity import com.embabel.chat.Message import com.embabel.dice.incremental.IncrementalSource +import com.embabel.dice.proposition.extraction.ExtractionContentProfileRef +import com.embabel.dice.proposition.extraction.ExtractionRunRef import com.embabel.dice.provenance.SourceLocator import com.embabel.dice.provenance.SourceRevisionRef import org.springframework.context.ApplicationEvent @@ -32,6 +34,10 @@ import org.springframework.context.ApplicationEvent * `SourceAnalysisContext` it builds, so the async path grounds propositions exactly * the way a direct `rememberTextFromSource` call does. Both default to null, so an * existing subclass carries no provenance and behaves as it always did. + * + * [profile] and [currentRun] work the same way and reach the same context through the same + * call, so an async publisher can attribute its extraction to a content profile and a run + * without the listener growing a second code path. Both also default to null. */ abstract class SourceAnalysisRequestEvent( source: Any, @@ -51,4 +57,16 @@ abstract class SourceAnalysisRequestEvent( * builds the context. */ open fun sourceRevision(): SourceRevisionRef? = null + + /** + * The extraction content profile this event's analysis should be attributed to, when the + * publisher has one. EXPERIMENTAL. DICE carries it and routes nothing on it. + */ + open fun profile(): ExtractionContentProfileRef? = null + + /** + * 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. + */ + open fun currentRun(): ExtractionRunRef? = null } diff --git a/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionContentProfileRef.kt b/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionContentProfileRef.kt new file mode 100644 index 00000000..160e2d7a --- /dev/null +++ b/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionContentProfileRef.kt @@ -0,0 +1,87 @@ +/* + * 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 + +/** + * Names a version of a host's extraction content profile. + * + * A profile is the host's durable answer to "what should extraction of this kind of material + * do?" — the tone, the coverage, the house rules a product wants applied. DICE holds the + * name and version and nothing else. It never looks the profile up, never reads policy out + * of it, and never branches on it. The host owns the catalog, decides who may use which + * profile, and binds it to whatever it actually means at call time. + * + * That split is the point. If DICE resolved profiles it would need a catalog, an + * authorization model, and a connector for every host that has one. Instead a profile + * reference is two opaque strings that ride along with the analysis so the run that produced + * a claim can be attributed to the policy it ran under. + * + * **A profile selects no provider, no model, and no credential.** DICE routes nothing on it. + * A host that wants a particular model for a particular profile makes that decision on its + * own side, before it calls DICE. + * + * Profile is independent of every other dimension on + * [com.embabel.dice.common.SourceAnalysisContext]. Perspective says whose statements to + * mine out of conversational input; schema says what types exist; the context id says which + * tenant owns the result. Setting one never constrains another — see + * `docs/design/extraction-profiles.md`. + * + * Both strings are opaque to DICE: it compares them and carries them, and it parses neither. + * Two profiles are the same profile when name and version both match, so a host that + * republishes a profile under a new version gets a distinct reference and stays attributable + * to the older one. + * + * EXPERIMENTAL. The shape may still change while extraction runs (DICE #67) land. + * + * @property name Host-defined profile name, stable across versions of the same profile + * @property version Host-defined version of that profile + */ +@ApiStatus.Experimental +data class ExtractionContentProfileRef( + val name: String, + val version: String, +) { + + init { + require(name.isNotBlank()) { "name must not be blank" } + require(name.length <= MAX_NAME_LENGTH) { + "name must be at most $MAX_NAME_LENGTH characters, was ${name.length}" + } + require(version.isNotBlank()) { "version must not be blank" } + require(version.length <= MAX_VERSION_LENGTH) { + "version must be at most $MAX_VERSION_LENGTH characters, was ${version.length}" + } + } + + companion object { + + /** + * Longest profile name DICE accepts. A reference is an identifier the host mints, not + * a place to smuggle a payload, and extraction runs will store these — so the bound + * exists to keep a stored run header from growing without limit. Any real profile name + * is far shorter. + */ + const val MAX_NAME_LENGTH: Int = 256 + + /** + * Longest profile version DICE accepts. Same reasoning as [MAX_NAME_LENGTH]; versions + * are short by nature (`v3`, `2026-08-01`, a commit sha). + */ + const val MAX_VERSION_LENGTH: Int = 64 + } +} diff --git a/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunRef.kt b/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunRef.kt new file mode 100644 index 00000000..37ebb426 --- /dev/null +++ b/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunRef.kt @@ -0,0 +1,68 @@ +/* + * 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 + +/** + * Names one extraction run. + * + * This is identity and nothing else. It holds no timing, no status, no counts, no lineage — + * just the id, so a caller can say "this analysis belongs to that run" without DICE having + * anywhere to store a run yet. Durable extraction runs arrive with DICE #67 and will be keyed + * by ([com.embabel.agent.core.ContextId], `ExtractionRunRef`); shipping the reference first + * means the entry points and the run model meet at an opaque string rather than at a type one + * of them has to import from the other's release. + * + * The id is opaque. DICE compares it and carries it and parses nothing out of it. It also never + * mints one: a run is something the host (or, later, DICE's own run coordinator) starts. Nothing + * here checks that the run exists, because there is nowhere yet to check against — carrying a + * reference is always allowed. What a store does with a reference to a run it has never seen is + * DICE #67's to decide, and this type makes no promise about it either way. + * + * Run identity is deliberately not part of source-provenance equality. Two runs over the same + * material still produce one piece of source evidence; what differs is which runs are + * attributed to it. + * + * A run reference is not an authorization token and must not carry a secret, a direct + * identifier, or anything a reader could dereference into personal data. Hosts mint it. + * + * EXPERIMENTAL. The shape may still change while extraction runs (DICE #67) land. + * + * @property runId Host-minted opaque identifier for the run + */ +@ApiStatus.Experimental +data class ExtractionRunRef( + val runId: String, +) { + + init { + require(runId.isNotBlank()) { "runId must not be blank" } + require(runId.length <= MAX_RUN_ID_LENGTH) { + "runId must be at most $MAX_RUN_ID_LENGTH characters, was ${runId.length}" + } + } + + companion object { + + /** + * Longest run id DICE accepts. #67 keys stored runs on this string and indexes it, so + * an unbounded id would become an unbounded key. A uuid, a ULID, or a host's own + * correlation id all fit with room to spare. + */ + const val MAX_RUN_ID_LENGTH: Int = 256 + } +} 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 7a6300fa..bafd7165 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 @@ -163,15 +163,52 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( /** * Extract propositions from a file via Tika and persist them. * Requires `embabel-agent-rag-tika` on the classpath. + * + * This is the signature that existed before profiles, kept as its own declaration so it + * stays overridable — see the note on [rememberText]. */ - open fun rememberFile(inputStream: InputStream, filename: String, user: NamedEntity) = + open fun rememberFile( + inputStream: InputStream, + filename: String, + user: NamedEntity, + ) = withRememberedFileText(inputStream, filename) { text -> + // Deliberately the pre-profile text signature, not the wide one. This is the dispatch + // this method had before profiles existed, so a subclass that overrides only that + // signature still intercepts file ingestion the way it always did. + rememberText(text, "remember:$filename", user, emptyList(), null, null) + } + + /** + * Extract propositions from a file via Tika, attributed to an extraction content profile + * and a run. EXPERIMENTAL; see [rememberText] for what the two references mean. + * + * With no references to carry this hands straight back to the three-argument form, so a + * call that looks like a pre-profile call also dispatches like one. + */ + open fun rememberFile( + inputStream: InputStream, + filename: String, + user: NamedEntity, + profile: ExtractionContentProfileRef?, + currentRun: ExtractionRunRef?, + ) { + if (profile == null && currentRun == null) { + rememberFile(inputStream, filename, user) + return + } withRememberedFileText(inputStream, filename) { text -> rememberText( - text = text, - sourceId = "remember:$filename", - user = user, + text, + "remember:$filename", + user, + emptyList(), + null, + null, + profile, + currentRun, ) } + } /** * Extract propositions from a file and ground them in the caller's typed source. @@ -179,6 +216,9 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( * Passing a [sourceRevision] asserts that the locator's revision covers the whole file, all * of it, as extracted here. DICE reads the file as one aggregate and has no way to work out * whether a provider's revision really spans it, so the host has to know that and say so. + * + * This is the signature that existed before profiles, kept as its own declaration so it + * stays overridable — see the note on [rememberText]. */ @JvmOverloads open fun rememberFileFromSource( @@ -188,16 +228,60 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( sourceLocator: SourceLocator, sourceRevision: SourceRevisionRef? = null, ) { + require(sourceRevision == null || sourceRevision.sourceKey == sourceLocator.key()) { + "sourceRevision source key must match sourceLocator source key" + } + withRememberedFileText(inputStream, filename) { text -> + // The pre-profile text signature, for the same reason as [rememberFile]. + rememberTextFromSource( + text, + "remember:$filename", + user, + sourceLocator, + sourceRevision, + emptyList(), + null, + null, + ) + } + } + + /** + * Extract propositions from a file, ground them in the caller's typed source, and attribute + * them to an extraction content profile and a run. EXPERIMENTAL; see [rememberText] for what + * the two references mean. The profile is independent of the locator and the revision. + * + * With no references to carry this hands straight back to the five-argument form, so a call + * that looks like a pre-profile call also dispatches like one. + */ + open fun rememberFileFromSource( + inputStream: InputStream, + filename: String, + user: NamedEntity, + sourceLocator: SourceLocator, + sourceRevision: SourceRevisionRef?, + profile: ExtractionContentProfileRef?, + currentRun: ExtractionRunRef?, + ) { + if (profile == null && currentRun == null) { + rememberFileFromSource(inputStream, filename, user, sourceLocator, sourceRevision) + return + } require(sourceRevision == null || sourceRevision.sourceKey == sourceLocator.key()) { "sourceRevision source key must match sourceLocator source key" } withRememberedFileText(inputStream, filename) { text -> rememberTextFromSource( - text = text, - sourceId = "remember:$filename", - user = user, - sourceLocator = sourceLocator, - sourceRevision = sourceRevision, + text, + "remember:$filename", + user, + sourceLocator, + sourceRevision, + emptyList(), + null, + null, + profile, + currentRun, ) } } @@ -239,6 +323,15 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( * @param mintNewEntities per-call override for whether unmatched mentions may * be persisted as NEW entities. `null` (default) uses the extractor * instance's default; see [SourceAnalysisContext.mintNewEntities]. + * + * This is the signature that existed before profiles, and it stays its own declaration + * rather than growing two defaulted parameters. `@JvmOverloads` emits every reduced-arity + * overload as `final`, so folding the new arguments into this method would have turned the + * six-argument form — the one a subclass overrides — into a final bridge. Declaring the two + * shapes separately keeps both open. Unintercepted, every call lands on the eight-argument + * form below, so overriding that one sees everything; overriding this one sees everything a + * pre-profile subclass used to see, including file ingestion, which still routes through + * here rather than jumping to the wide form. */ @JvmOverloads open fun rememberText( @@ -248,6 +341,31 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( additionalGrounding: List = emptyList(), perspective: ExtractionPerspective? = null, mintNewEntities: Boolean? = null, + ) = rememberText(text, sourceId, user, additionalGrounding, perspective, mintNewEntities, null, null) + + /** + * Extract propositions from raw text, attributed to an extraction content profile and a run. + * + * Every other text entry point funnels here, so this is the one method to override to see + * every call. + * + * @param profile extraction content profile this call should be attributed to, or null. + * EXPERIMENTAL. DICE puts it on the context and does nothing else with it: no provider, + * model, or credential is selected from it, and extraction runs exactly as it would + * without one. + * @param currentRun reference to the extraction run this call belongs to, or null. + * EXPERIMENTAL. Identity only; nothing is stored under it until DICE #67 lands, and DICE + * does not check that the run exists. + */ + open fun rememberText( + text: String, + sourceId: String, + user: NamedEntity, + additionalGrounding: List, + perspective: ExtractionPerspective?, + mintNewEntities: Boolean?, + profile: ExtractionContentProfileRef?, + currentRun: ExtractionRunRef?, ) = rememberTextInternal( text = text, @@ -256,6 +374,8 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( additionalGrounding = additionalGrounding, perspective = perspective, mintNewEntities = mintNewEntities, + profile = profile, + currentRun = currentRun, ) /** @@ -269,6 +389,9 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( * identifier. Passing a [sourceRevision] asserts that [sourceLocator]'s revision covers the * whole text. DICE cannot read revision coverage out of an untyped [sourceId] or out of * [additionalGrounding], so the host has to know that and say so. + * + * This is the signature that existed before profiles, kept as its own declaration so it + * stays overridable — see the note on [rememberText]. */ @JvmOverloads open fun rememberTextFromSource( @@ -280,6 +403,36 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( additionalGrounding: List = emptyList(), perspective: ExtractionPerspective? = null, mintNewEntities: Boolean? = null, + ) = rememberTextFromSource( + text, + sourceId, + user, + sourceLocator, + sourceRevision, + additionalGrounding, + perspective, + mintNewEntities, + null, + null, + ) + + /** + * Extract propositions from raw text, ground them in the caller's typed source, and + * attribute them to an extraction content profile and a run. EXPERIMENTAL; see + * [rememberText] for what the two references mean. A profile is independent of the locator + * and the revision — it says what extraction should do, they say what it is reading. + */ + open fun rememberTextFromSource( + text: String, + sourceId: String, + user: NamedEntity, + sourceLocator: SourceLocator, + sourceRevision: SourceRevisionRef?, + additionalGrounding: List, + perspective: ExtractionPerspective?, + mintNewEntities: Boolean?, + profile: ExtractionContentProfileRef?, + currentRun: ExtractionRunRef?, ) = rememberTextInternal( text = text, @@ -290,6 +443,8 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( additionalGrounding = additionalGrounding, perspective = perspective, mintNewEntities = mintNewEntities, + profile = profile, + currentRun = currentRun, ) private fun rememberTextInternal( @@ -301,6 +456,8 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( additionalGrounding: List = emptyList(), perspective: ExtractionPerspective? = null, mintNewEntities: Boolean? = null, + profile: ExtractionContentProfileRef? = null, + currentRun: ExtractionRunRef? = null, ) { val context = buildContext( user = user, @@ -309,6 +466,8 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( mintNewEntities = mintNewEntities, sourceLocator = sourceLocator, sourceRevision = sourceRevision, + profile = profile, + currentRun = currentRun, ) val result = propositionPipeline.processOnce( text, sourceId, context, additionalGrounding = additionalGrounding, @@ -357,12 +516,15 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( } // The async path grounds propositions exactly the way rememberTextFromSource does: - // whatever provenance the event carries goes through the same buildContext call. + // whatever provenance, profile and run the event carries goes through the same + // buildContext call. val context = buildContext( user = event.user, sourceId = source.id, sourceLocator = event.sourceLocator(), sourceRevision = event.sourceRevision(), + profile = event.profile(), + currentRun = event.currentRun(), ) logger.info( "Context relations count: {}, injected relations count: {}", @@ -396,6 +558,8 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( mintNewEntities: Boolean? = null, sourceLocator: SourceLocator? = null, sourceRevision: SourceRevisionRef? = null, + profile: ExtractionContentProfileRef? = null, + currentRun: ExtractionRunRef? = null, ): SourceAnalysisContext { val aliases = try { currentUserAliasesProvider(user) @@ -461,6 +625,14 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( if (sourceRevision != null) { ctx = ctx.withSourceRevision(sourceRevision) } + // 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. + if (profile != null) { + ctx = ctx.withProfile(profile) + } + if (currentRun != null) { + ctx = ctx.withCurrentRun(currentRun) + } return ctx } diff --git a/dice/src/test/java/com/embabel/dice/ExtractionProfileJavaInteropTest.java b/dice/src/test/java/com/embabel/dice/ExtractionProfileJavaInteropTest.java new file mode 100644 index 00000000..56a7099b --- /dev/null +++ b/dice/src/test/java/com/embabel/dice/ExtractionProfileJavaInteropTest.java @@ -0,0 +1,405 @@ +/* + * 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; + +import com.embabel.agent.core.DataDictionary; +import com.embabel.agent.rag.model.NamedEntity; +import com.embabel.chat.Conversation; +import com.embabel.chat.Message; +import com.embabel.dice.common.ConversationAnalysisRequestEvent; +import com.embabel.dice.common.SourceAnalysisContext; +import com.embabel.dice.common.SourceAnalysisRequestEvent; +import com.embabel.dice.common.resolver.AlwaysCreateEntityResolver; +import com.embabel.dice.incremental.IncrementalSource; +import com.embabel.dice.proposition.extraction.ExtractionContentProfileRef; +import com.embabel.dice.proposition.extraction.ExtractionPerspective; +import com.embabel.dice.proposition.extraction.ExtractionRunRef; +import com.embabel.dice.proposition.extraction.IncrementalPropositionExtraction; +import com.embabel.dice.provenance.ContentAddressedLocator; +import com.embabel.dice.provenance.SourceLocator; +import com.embabel.dice.provenance.SourceRevisionRef; +import org.junit.jupiter.api.Test; + +import java.io.InputStream; +import java.lang.reflect.Modifier; +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; + +/** + * Java's view of the profile contract, with and without a profile present. + * {@code @JvmOverloads} means the remember entry points keep every descriptor a Java caller + * could already have compiled against, and the profile and run arguments only ever add + * descriptors on the end. + */ +class ExtractionProfileJavaInteropTest { + + private SourceAnalysisContext context() { + return SourceAnalysisContext + .withContextId("java-profile") + .withEntityResolver(AlwaysCreateEntityResolver.INSTANCE) + .withSchema(DataDictionary.fromClasses("java-profile")); + } + + @Test + void javaCallersBuildAndReadProfileAndRunValues() { + ExtractionContentProfileRef profile = new ExtractionContentProfileRef("house-style", "v1"); + assertEquals("house-style", profile.getName()); + assertEquals("v1", profile.getVersion()); + assertEquals(profile, new ExtractionContentProfileRef("house-style", "v1")); + assertNotEquals(profile, new ExtractionContentProfileRef("house-style", "v2")); + assertThrows( + IllegalArgumentException.class, + () -> new ExtractionContentProfileRef(" ", "v1") + ); + + ExtractionRunRef run = new ExtractionRunRef("run-1"); + assertEquals("run-1", run.getRunId()); + assertEquals(run, new ExtractionRunRef("run-1")); + assertThrows(IllegalArgumentException.class, () -> new ExtractionRunRef("")); + } + + @Test + void javaBuiltContextsCarryNoProfileUnlessAsked() { + SourceAnalysisContext absent = context(); + assertNull(absent.getProfile()); + assertNull(absent.getCurrentRun()); + + ExtractionContentProfileRef profile = new ExtractionContentProfileRef("house-style", "v1"); + ExtractionRunRef run = new ExtractionRunRef("run-1"); + SourceAnalysisContext present = absent.withProfile(profile).withCurrentRun(run); + + assertSame(profile, present.getProfile()); + assertSame(run, present.getCurrentRun()); + // The copy is a copy: the original is untouched. + assertNull(absent.getProfile()); + assertNull(absent.getCurrentRun()); + } + + @Test + void retainsEveryLegacyRememberDescriptorAndAddsProfileAwareOnes() throws Exception { + Class[][] legacyTextParameters = { + {String.class, String.class, NamedEntity.class}, + {String.class, String.class, NamedEntity.class, List.class}, + {String.class, String.class, NamedEntity.class, List.class, ExtractionPerspective.class}, + {String.class, String.class, NamedEntity.class, List.class, ExtractionPerspective.class, + Boolean.class}, + }; + for (Class[] parameters : legacyTextParameters) { + IncrementalPropositionExtraction.class.getMethod("rememberText", parameters); + } + IncrementalPropositionExtraction.class.getMethod( + "rememberText", + String.class, String.class, NamedEntity.class, List.class, + ExtractionPerspective.class, Boolean.class, ExtractionContentProfileRef.class, + ExtractionRunRef.class + ); + // A profile and a run always arrive together; there is no profile-only descriptor. + assertThrows( + NoSuchMethodException.class, + () -> IncrementalPropositionExtraction.class.getMethod( + "rememberText", + String.class, String.class, NamedEntity.class, List.class, + ExtractionPerspective.class, Boolean.class, + ExtractionContentProfileRef.class + ) + ); + + IncrementalPropositionExtraction.class.getMethod( + "rememberTextFromSource", + String.class, String.class, NamedEntity.class, SourceLocator.class, + SourceRevisionRef.class, List.class, ExtractionPerspective.class, Boolean.class, + ExtractionContentProfileRef.class, ExtractionRunRef.class + ); + + IncrementalPropositionExtraction.class.getMethod( + "rememberFile", InputStream.class, String.class, NamedEntity.class + ); + IncrementalPropositionExtraction.class.getMethod( + "rememberFile", + InputStream.class, String.class, NamedEntity.class, + ExtractionContentProfileRef.class, ExtractionRunRef.class + ); + IncrementalPropositionExtraction.class.getMethod( + "rememberFileFromSource", + InputStream.class, String.class, NamedEntity.class, SourceLocator.class, + SourceRevisionRef.class, ExtractionContentProfileRef.class, ExtractionRunRef.class + ); + + // A profile never lands where a legacy caller already puts something else. + assertThrows( + NoSuchMethodException.class, + () -> IncrementalPropositionExtraction.class.getMethod( + "rememberText", + String.class, String.class, NamedEntity.class, + ExtractionContentProfileRef.class + ) + ); + assertThrows( + NoSuchMethodException.class, + () -> IncrementalPropositionExtraction.class.getMethod( + "rememberFile", + InputStream.class, String.class, NamedEntity.class, + ExtractionRunRef.class + ) + ); + } + + @Test + void legacyAndProfileAwareJavaEventSubclassesUseTheBaseConstructor() { + NamedEntity user = org.mockito.Mockito.mock(NamedEntity.class); + + LegacyJavaEvent legacy = new LegacyJavaEvent(this, user); + assertNull(legacy.profile()); + assertNull(legacy.currentRun()); + + ExtractionContentProfileRef profile = new ExtractionContentProfileRef("house-style", "v1"); + ExtractionRunRef run = new ExtractionRunRef("run-1"); + ProfileAwareJavaEvent profileAware = new ProfileAwareJavaEvent(this, user, profile, run); + assertSame(profile, profileAware.profile()); + assertSame(run, profileAware.currentRun()); + // A subclass that only knows about profiles still carries no source provenance. + assertNull(profileAware.sourceLocator()); + assertNull(profileAware.sourceRevision()); + } + + @Test + void conversationEventKeepsItsFiveArgumentConstructorAndGainsProfileAwareOnes() throws Exception { + ConversationAnalysisRequestEvent.class.getConstructor( + Object.class, NamedEntity.class, Conversation.class + ); + ConversationAnalysisRequestEvent.class.getConstructor( + Object.class, NamedEntity.class, Conversation.class, SourceLocator.class, + SourceRevisionRef.class + ); + ConversationAnalysisRequestEvent.class.getConstructor( + Object.class, NamedEntity.class, Conversation.class, SourceLocator.class, + SourceRevisionRef.class, ExtractionContentProfileRef.class + ); + ConversationAnalysisRequestEvent.class.getConstructor( + Object.class, NamedEntity.class, Conversation.class, SourceLocator.class, + SourceRevisionRef.class, ExtractionContentProfileRef.class, ExtractionRunRef.class + ); + + NamedEntity user = org.mockito.Mockito.mock(NamedEntity.class); + Conversation conversation = org.mockito.Mockito.mock(Conversation.class); + SourceLocator locator = new ContentAddressedLocator("java-conversation"); + SourceRevisionRef revision = new SourceRevisionRef(locator.key(), "r1"); + ExtractionContentProfileRef profile = new ExtractionContentProfileRef("house-style", "v1"); + ExtractionRunRef run = new ExtractionRunRef("run-1"); + + ConversationAnalysisRequestEvent legacy = + new ConversationAnalysisRequestEvent(this, user, conversation, locator, revision); + assertSame(locator, legacy.sourceLocator()); + assertSame(revision, legacy.sourceRevision()); + assertNull(legacy.profile()); + assertNull(legacy.currentRun()); + + ConversationAnalysisRequestEvent profiled = new ConversationAnalysisRequestEvent( + this, user, conversation, locator, revision, profile, run + ); + assertSame(profile, profiled.profile()); + assertSame(run, profiled.currentRun()); + + // A profile without a typed source: the locator argument is nullable because the two + // dimensions are independent. + ConversationAnalysisRequestEvent profileOnly = new ConversationAnalysisRequestEvent( + this, user, conversation, null, null, profile, run + ); + assertNull(profileOnly.sourceLocator()); + assertSame(profile, profileOnly.profile()); + } + + @Test + void javaSubclassesStillOverrideThePreProfileSignatures() throws Exception { + // The proof is that this file compiles: javac rejects @Override on a final method, so + // LegacyOverridingJavaExtraction would not build if adding the profile arguments had + // turned the six-argument rememberText or the three-argument rememberFile into the + // final bridges @JvmOverloads emits for reduced arities. + assertFalse( + Modifier.isFinal( + IncrementalPropositionExtraction.class.getMethod( + "rememberText", + String.class, String.class, NamedEntity.class, List.class, + ExtractionPerspective.class, Boolean.class + ).getModifiers() + ) + ); + assertFalse( + Modifier.isFinal( + IncrementalPropositionExtraction.class.getMethod( + "rememberFile", + InputStream.class, String.class, NamedEntity.class + ).getModifiers() + ) + ); + LegacyOverridingJavaExtraction.class.getDeclaredMethod( + "rememberText", + String.class, String.class, NamedEntity.class, List.class, + ExtractionPerspective.class, Boolean.class + ); + LegacyOverridingJavaExtraction.class.getDeclaredMethod( + "rememberFile", InputStream.class, String.class, NamedEntity.class + ); + } + + /** + * A Java subclass written before profiles existed, overriding the entry-point signatures + * that were open then. It is never instantiated; compiling it is the assertion. + */ + @SuppressWarnings("unused") + private static final class LegacyOverridingJavaExtraction extends IncrementalPropositionExtraction { + + private LegacyOverridingJavaExtraction( + com.embabel.dice.pipeline.PropositionPipeline propositionPipeline, + com.embabel.dice.incremental.ChunkHistoryStore chunkHistoryStore, + DataDictionary dataDictionary, + com.embabel.dice.common.Relations relations, + com.embabel.dice.proposition.PropositionRepository propositionRepository, + com.embabel.agent.rag.service.NamedEntityDataRepository entityRepository, + com.embabel.dice.common.EntityResolver entityResolver, + com.embabel.dice.projection.graph.GraphProjectionService graphProjectionService, + com.embabel.dice.proposition.extraction.PropositionExtractionProperties properties + ) { + super( + propositionPipeline, chunkHistoryStore, dataDictionary, relations, + propositionRepository, entityRepository, entityResolver, + graphProjectionService, properties + ); + } + + @Override + public void rememberText( + String text, + String sourceId, + NamedEntity user, + List additionalGrounding, + ExtractionPerspective perspective, + Boolean mintNewEntities + ) { + // A host's interception point; deliberately does nothing. + } + + @Override + public void rememberFile(InputStream inputStream, String filename, NamedEntity user) { + // A host's interception point; deliberately does nothing. + } + + @Override + public void rememberFileFromSource( + InputStream inputStream, + String filename, + NamedEntity user, + SourceLocator sourceLocator, + SourceRevisionRef sourceRevision + ) { + // A host's interception point; deliberately does nothing. + } + + @Override + public void rememberTextFromSource( + String text, + String sourceId, + NamedEntity user, + SourceLocator sourceLocator, + SourceRevisionRef sourceRevision, + List additionalGrounding, + ExtractionPerspective perspective, + Boolean mintNewEntities + ) { + // A host's interception point; deliberately does nothing. + } + } + + /** + * Compiling this body proves the legacy and additive Java source entry points remain + * callable. It is intentionally never executed because extraction has observable side + * effects. + */ + @SuppressWarnings({"unused", "DataFlowIssue"}) + private static void compileJavaSourceCalls( + IncrementalPropositionExtraction extraction, + InputStream input, + NamedEntity user, + SourceLocator locator, + SourceRevisionRef revision, + ExtractionContentProfileRef profile, + ExtractionRunRef run + ) { + extraction.rememberText("legacy", "legacy-id", user); + extraction.rememberText("legacy", "legacy-id", user, List.of(), null, null); + extraction.rememberText("profiled", "profiled-id", user, List.of(), null, null, profile, run); + extraction.rememberFile(input, "legacy.txt", user); + extraction.rememberFile(input, "profiled.txt", user, profile, run); + extraction.rememberTextFromSource("source", "source-id", user, locator); + extraction.rememberTextFromSource( + "source", "source-id", user, locator, revision, List.of(), null, null, profile, run + ); + extraction.rememberFileFromSource(input, "source.txt", user, locator); + extraction.rememberFileFromSource( + input, "source.txt", user, locator, revision, profile, run + ); + } + + private static final class LegacyJavaEvent extends SourceAnalysisRequestEvent { + + private LegacyJavaEvent(Object source, NamedEntity user) { + super(source, user); + } + + @Override + public IncrementalSource incrementalSource() { + throw new UnsupportedOperationException("Not needed by this compatibility test"); + } + } + + private static final class ProfileAwareJavaEvent extends SourceAnalysisRequestEvent { + + private final ExtractionContentProfileRef profile; + private final ExtractionRunRef run; + + private ProfileAwareJavaEvent( + Object source, + NamedEntity user, + ExtractionContentProfileRef profile, + ExtractionRunRef run + ) { + super(source, user); + this.profile = profile; + this.run = run; + } + + @Override + public IncrementalSource incrementalSource() { + throw new UnsupportedOperationException("Not needed by this compatibility test"); + } + + @Override + public ExtractionContentProfileRef profile() { + return profile; + } + + @Override + public ExtractionRunRef currentRun() { + return run; + } + } +} diff --git a/dice/src/test/kotlin/com/embabel/dice/ExtractionProfileCompatibilityTest.kt b/dice/src/test/kotlin/com/embabel/dice/ExtractionProfileCompatibilityTest.kt new file mode 100644 index 00000000..815ae2e3 --- /dev/null +++ b/dice/src/test/kotlin/com/embabel/dice/ExtractionProfileCompatibilityTest.kt @@ -0,0 +1,183 @@ +/* + * 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 + +import com.embabel.agent.core.ContextId +import com.embabel.agent.core.DataDictionary +import com.embabel.dice.common.EntityResolver +import com.embabel.dice.common.KnownEntity +import com.embabel.dice.common.Relations +import com.embabel.dice.common.SourceAnalysisContext +import com.embabel.dice.common.resolver.AlwaysCreateEntityResolver +import com.embabel.dice.proposition.extraction.ExtractionContentProfileRef +import com.embabel.dice.proposition.extraction.ExtractionPerspective +import com.embabel.dice.proposition.extraction.ExtractionRunRef +import com.embabel.dice.provenance.ContentAddressedLocator +import com.embabel.dice.provenance.SourceLocator +import com.embabel.dice.provenance.SourceRevisionRef +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertNull +import org.junit.jupiter.api.Assertions.assertSame +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test + +/** + * The compatibility boundary this slice claims for `SourceAnalysisContext`, from Kotlin: + * source-level constructor and `copy` calls written before profiles keep compiling and keep + * seeing null, every Java-visible constructor descriptor that existed survives, and the old + * `copy` descriptor does not — the same split Wave A declared for `sourceRevision`. + */ +class ExtractionProfileCompatibilityTest { + + private fun context( + profile: ExtractionContentProfileRef? = null, + currentRun: ExtractionRunRef? = null, + ) = SourceAnalysisContext( + schema = DataDictionary.fromClasses("profile-compatibility"), + entityResolver = AlwaysCreateEntityResolver, + contextId = ContextId("profile-compatibility"), + profile = profile, + currentRun = currentRun, + ) + + @Test + fun `legacy Kotlin source constructors and copy calls see no profile and no run`() { + val legacy = SourceAnalysisContext( + schema = DataDictionary.fromClasses("profile-compatibility"), + entityResolver = AlwaysCreateEntityResolver, + contextId = ContextId("profile-compatibility"), + ).copy(promptVariables = mapOf("legacy" to true)) + + assertEquals(true, legacy.promptVariables["legacy"]) + assertNull(legacy.profile) + assertNull(legacy.currentRun) + + // The Java-facing builder is unchanged too, and its result carries neither. + val built = SourceAnalysisContext + .withContextId("profile-compatibility") + .withEntityResolver(AlwaysCreateEntityResolver) + .withSchema(DataDictionary.fromClasses("profile-compatibility")) + assertNull(built.profile) + assertNull(built.currentRun) + } + + @Test + fun `new Kotlin source constructors and copy calls carry a profile and a run`() { + val profile = ExtractionContentProfileRef("house-style", "v3") + val run = ExtractionRunRef("run-42") + + val fromConstructor = context(profile = profile, currentRun = run) + .copy(promptVariables = mapOf("profiled" to true)) + assertSame(profile, fromConstructor.profile) + assertSame(run, fromConstructor.currentRun) + assertEquals(true, fromConstructor.promptVariables["profiled"]) + + val fromHelpers = context().withProfile(profile).withCurrentRun(run) + assertSame(profile, fromHelpers.profile) + assertSame(run, fromHelpers.currentRun) + } + + @Test + fun `a profile rides alongside a locator and a revision without disturbing either`() { + val locator = ContentAddressedLocator("profile-compatibility-source") + val revision = SourceRevisionRef(locator.key(), "r4") + + val context = SourceAnalysisContext( + schema = DataDictionary.fromClasses("profile-compatibility"), + entityResolver = AlwaysCreateEntityResolver, + contextId = ContextId("profile-compatibility"), + sourceLocator = locator, + perspective = ExtractionPerspective.USER, + sourceRevision = revision, + profile = ExtractionContentProfileRef("house-style", "v3"), + currentRun = ExtractionRunRef("run-42"), + ) + + assertSame(locator, context.sourceLocator) + assertSame(revision, context.sourceRevision) + assertSame(ExtractionPerspective.USER, context.perspective) + } + + @Test + fun `every constructor descriptor that existed before profiles survives`() { + val marker = Class.forName("kotlin.jvm.internal.DefaultConstructorMarker") + // Kotlin publishes the @JvmOverloads constructors with a trailing DefaultConstructorMarker + // because contextId is a value class; the marker is part of the descriptor a compiled + // caller links against. + val declared = listOf( + DataDictionary::class.java, + EntityResolver::class.java, + String::class.java, + List::class.java, + Relations::class.java, + Map::class.java, + SourceLocator::class.java, + ExtractionPerspective::class.java, + Boolean::class.javaPrimitiveType!!, + Map::class.java, + SourceRevisionRef::class.java, + ExtractionContentProfileRef::class.java, + ExtractionRunRef::class.java, + ) + val published = SourceAnalysisContext::class.java.constructors + .map { it.parameterTypes.toList() } + .toSet() + + // 3..11 are the descriptors Wave A left behind. + for (arity in 3..11) { + assertTrue( + declared.take(arity) + marker in published, + "constructor of $arity arguments no longer published", + ) + } + // 12 and 13 are what this slice adds, on the end. + for (arity in 12..13) { + assertTrue( + declared.take(arity) + marker in published, + "constructor of $arity arguments was not published", + ) + } + } + + @Test + fun `the old copy descriptor is outside the approved boundary`() { + // Adding a field to a data class rewrites copy and componentN. That half of the ABI is + // not claimed, here or in Wave A; only source and Java-constructor compatibility are. + val copyArities = SourceAnalysisContext::class.java.declaredMethods + .filter { it.name.startsWith("copy") && !it.name.endsWith("\$default") } + .map { it.parameterCount } + assertEquals(listOf(13), copyArities) + + val componentCount = SourceAnalysisContext::class.java.declaredMethods + .count { it.name.startsWith("component") } + assertEquals(13, componentCount) + } + + @Test + fun `known entity and relation helpers are unaffected by a profile`() { + val profile = ExtractionContentProfileRef("house-style", "v3") + val base = context(profile = profile) + + val widened = base + .withRelations(Relations.empty()) + .withKnownEntities(*emptyArray()) + .withMintNewEntities(true) + .withMintedEntityProperties(mapOf("owner" to "tenant")) + + assertSame(profile, widened.profile) + assertEquals(true, widened.mintNewEntities) + } +} diff --git a/dice/src/test/kotlin/com/embabel/dice/common/ExtractionContextIndependenceTest.kt b/dice/src/test/kotlin/com/embabel/dice/common/ExtractionContextIndependenceTest.kt new file mode 100644 index 00000000..5568bedd --- /dev/null +++ b/dice/src/test/kotlin/com/embabel/dice/common/ExtractionContextIndependenceTest.kt @@ -0,0 +1,256 @@ +/* + * 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.common + +import com.embabel.agent.core.ContextId +import com.embabel.agent.core.DataDictionary +import com.embabel.dice.common.resolver.AlwaysCreateEntityResolver +import com.embabel.dice.proposition.extraction.ExtractionContentProfileRef +import com.embabel.dice.proposition.extraction.ExtractionPerspective +import com.embabel.dice.proposition.extraction.ExtractionRunRef +import com.embabel.dice.provenance.ContentAddressedLocator +import com.embabel.dice.provenance.SourceRevisionRef +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertNotNull +import org.junit.jupiter.api.Assertions.assertNull +import org.junit.jupiter.api.Assertions.assertSame +import org.junit.jupiter.api.Assertions.assertThrows +import org.junit.jupiter.api.Test + +/** + * Profile, perspective, schema and tenant are four independent dimensions of an analysis. + * Setting one never constrains, coerces or perturbs another — this file is that claim, run as + * a matrix rather than argued in a comment. + * + * Why it needs stating: perspective is the dimension a profile is most easily confused with. + * Perspective describes conversational input (whose statements to mine); a profile is the + * host's content policy. They answer different questions and a caller may combine them freely. + */ +class ExtractionContextIndependenceTest { + + private val profiles = listOf( + null, + ExtractionContentProfileRef("house-style", "v1"), + ExtractionContentProfileRef("house-style", "v2"), + ExtractionContentProfileRef("legal-review", "v1"), + ) + private val perspectives = listOf( + null, + ExtractionPerspective.ALL, + ExtractionPerspective.USER, + ExtractionPerspective.NON_USER_RELATIONSHIPS, + ) + private val schemas = listOf( + DataDictionary.fromClasses("schema-one"), + DataDictionary.fromClasses("schema-two"), + ) + private val tenants = listOf(ContextId("tenant-one"), ContextId("tenant-two")) + + /** One matrix cell, named by index so the test never depends on how a dimension compares. */ + private data class Cell( + val profile: Int, + val perspective: Int, + val schema: Int, + val tenant: Int, + ) + + private fun matrix(): Map = + profiles.indices.flatMap { p -> + perspectives.indices.flatMap { e -> + schemas.indices.flatMap { s -> + tenants.indices.map { t -> + Cell(p, e, s, t) to SourceAnalysisContext( + schema = schemas[s], + entityResolver = AlwaysCreateEntityResolver, + contextId = tenants[t], + perspective = perspectives[e], + profile = profiles[p], + ) + } + } + } + }.toMap() + + @Test + fun `every combination of profile perspective schema and tenant is constructible`() { + val matrix = matrix() + + assertEquals(64, matrix.size, "4 profiles x 4 perspectives x 2 schemas x 2 tenants") + matrix.forEach { (cell, context) -> + assertSame(profiles[cell.profile], context.profile, "profile at $cell") + assertSame(perspectives[cell.perspective], context.perspective, "perspective at $cell") + assertSame(schemas[cell.schema], context.schema, "schema at $cell") + assertEquals(tenants[cell.tenant], context.contextId, "tenant at $cell") + } + } + + @Test + fun `each pair of dimensions realises its whole cross product`() { + val matrix = matrix() + val observed = matrix.values.map { + listOf(it.profile, it.perspective, it.schema, it.contextId) + } + val domains = listOf(profiles, perspectives, schemas, tenants) + + // For every ordered pair of dimensions, every combination of their values occurs. A + // dimension that quietly disabled, defaulted or rejected another would leave a hole here. + for (first in domains.indices) { + for (second in domains.indices) { + if (first == second) continue + val expected = domains[first].flatMap { a -> domains[second].map { b -> a to b } } + .toSet() + val actual = observed.map { it[first] to it[second] }.toSet() + assertEquals(expected, actual, "cross product of dimensions $first and $second") + } + } + } + + @Test + fun `varying one dimension leaves the other three untouched`() { + val baseline = SourceAnalysisContext( + schema = schemas[0], + entityResolver = AlwaysCreateEntityResolver, + contextId = tenants[0], + perspective = perspectives[1], + profile = profiles[1], + ) + + for (profile in profiles) { + val varied = baseline.copy(profile = profile) + assertSame(profile, varied.profile) + assertSame(baseline.perspective, varied.perspective) + assertSame(baseline.schema, varied.schema) + assertEquals(baseline.contextId, varied.contextId) + } + for (perspective in perspectives) { + val varied = baseline.copy(perspective = perspective) + assertSame(perspective, varied.perspective) + assertSame(baseline.profile, varied.profile) + assertSame(baseline.schema, varied.schema) + assertEquals(baseline.contextId, varied.contextId) + } + for (schema in schemas) { + val varied = baseline.copy(schema = schema) + assertSame(schema, varied.schema) + assertSame(baseline.profile, varied.profile) + assertSame(baseline.perspective, varied.perspective) + assertEquals(baseline.contextId, varied.contextId) + } + for (tenant in tenants) { + val varied = baseline.copy(contextId = tenant) + assertEquals(tenant, varied.contextId) + assertSame(baseline.profile, varied.profile) + assertSame(baseline.perspective, varied.perspective) + assertSame(baseline.schema, varied.schema) + } + } + + @Test + fun `the copy helpers move one field and no other`() { + val locator = ContentAddressedLocator("independence-source") + val base = SourceAnalysisContext( + schema = schemas[0], + entityResolver = AlwaysCreateEntityResolver, + contextId = tenants[0], + knownEntities = emptyList(), + relations = Relations.empty(), + promptVariables = mapOf("k" to "v"), + sourceLocator = locator, + perspective = ExtractionPerspective.USER, + mintNewEntities = true, + mintedEntityProperties = mapOf("owner" to "tenant-one"), + sourceRevision = SourceRevisionRef(locator.key(), "r1"), + profile = profiles[1], + currentRun = ExtractionRunRef("run-before"), + ) + + // Comparing against copy(...) is a statement about every component at once: the helper + // differs from the receiver in exactly the one field it names. + val profile = ExtractionContentProfileRef("legal-review", "v9") + assertEquals(base.copy(profile = profile), base.withProfile(profile)) + + val run = ExtractionRunRef("run-after") + assertEquals(base.copy(currentRun = run), base.withCurrentRun(run)) + + assertEquals( + base.copy(perspective = ExtractionPerspective.ALL), + base.withPerspective(ExtractionPerspective.ALL), + ) + } + + @Test + fun `a profile needs no locator and a locator needs no profile`() { + val profile = profiles[1]!! + val locator = ContentAddressedLocator("independence-source") + + val profileOnly = SourceAnalysisContext( + schema = schemas[0], + entityResolver = AlwaysCreateEntityResolver, + contextId = tenants[0], + profile = profile, + ) + assertSame(profile, profileOnly.profile) + assertNull(profileOnly.sourceLocator) + + val both = SourceAnalysisContext( + schema = schemas[0], + entityResolver = AlwaysCreateEntityResolver, + contextId = tenants[0], + sourceLocator = locator, + sourceRevision = SourceRevisionRef(locator.key(), "r1"), + profile = profile, + ) + assertSame(profile, both.profile) + assertNotNull(both.sourceRevision) + + // The one coupling that does exist is unaffected by a profile: a revision still has to + // name the source being read. + assertThrows(IllegalArgumentException::class.java) { + SourceAnalysisContext( + schema = schemas[0], + entityResolver = AlwaysCreateEntityResolver, + contextId = tenants[0], + sourceLocator = locator, + sourceRevision = SourceRevisionRef("content:something-else", "r1"), + profile = profile, + ) + } + } + + @Test + fun `profile and current run are independent of each other`() { + val profile = profiles[1]!! + val run = ExtractionRunRef("run-7") + + val combinations = listOf( + null to null, + profile to null, + null to run, + profile to run, + ).map { (p, r) -> + SourceAnalysisContext( + schema = schemas[0], + entityResolver = AlwaysCreateEntityResolver, + contextId = tenants[0], + profile = p, + currentRun = r, + ) + } + + assertEquals(listOf(null, profile, null, profile), combinations.map { it.profile }) + assertEquals(listOf(null, null, run, run), combinations.map { it.currentRun }) + } +} diff --git a/dice/src/test/kotlin/com/embabel/dice/common/SourceAnalysisRequestEventProfileTest.kt b/dice/src/test/kotlin/com/embabel/dice/common/SourceAnalysisRequestEventProfileTest.kt new file mode 100644 index 00000000..989a7fe9 --- /dev/null +++ b/dice/src/test/kotlin/com/embabel/dice/common/SourceAnalysisRequestEventProfileTest.kt @@ -0,0 +1,134 @@ +/* + * 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.common + +import com.embabel.agent.rag.model.NamedEntity +import com.embabel.chat.Conversation +import com.embabel.chat.Message +import com.embabel.dice.incremental.IncrementalSource +import com.embabel.dice.proposition.extraction.ExtractionContentProfileRef +import com.embabel.dice.proposition.extraction.ExtractionRunRef +import com.embabel.dice.provenance.ContentAddressedLocator +import com.embabel.dice.provenance.SourceRevisionRef +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertNull +import org.junit.jupiter.api.Assertions.assertSame +import org.junit.jupiter.api.Test +import org.mockito.Mockito.mock + +/** + * The async publisher's half of the profile contract: both accessors default to null, an + * existing subclass is unaffected, and the shipped conversation event carries a profile and a + * run when its longer constructor is used. + */ +class SourceAnalysisRequestEventProfileTest { + + private val profile = ExtractionContentProfileRef("house-style", "v1") + private val run = ExtractionRunRef("run-1") + + @Test + fun `a subclass written before profiles carries neither`() { + val legacy = LegacyEvent(this, mock(NamedEntity::class.java)) + + assertNull(legacy.profile()) + assertNull(legacy.currentRun()) + assertNull(legacy.sourceLocator()) + assertNull(legacy.sourceRevision()) + } + + @Test + fun `the shipped conversation event defaults to no profile and no run`() { + val event = ConversationAnalysisRequestEvent( + source = this, + user = mock(NamedEntity::class.java), + conversation = mock(Conversation::class.java), + ) + + assertNull(event.profile()) + assertNull(event.currentRun()) + } + + @Test + fun `the conversation event carries the exact profile and run it was given`() { + val locator = ContentAddressedLocator("event-source") + val revision = SourceRevisionRef(locator.key(), "r1") + + val event = ConversationAnalysisRequestEvent( + source = this, + user = mock(NamedEntity::class.java), + conversation = mock(Conversation::class.java), + sourceLocator = locator, + sourceRevision = revision, + profile = profile, + currentRun = run, + ) + + assertSame(locator, event.sourceLocator()) + assertSame(revision, event.sourceRevision()) + assertSame(profile, event.profile()) + assertSame(run, event.currentRun()) + } + + @Test + fun `a publisher can name a profile for material it has no typed source for`() { + val event = ConversationAnalysisRequestEvent( + source = this, + user = mock(NamedEntity::class.java), + conversation = mock(Conversation::class.java), + sourceLocator = null, + profile = profile, + ) + + assertNull(event.sourceLocator()) + assertNull(event.sourceRevision()) + assertSame(profile, event.profile()) + assertNull(event.currentRun()) + } + + @Test + fun `overriding one accessor leaves the others at their defaults`() { + val profileOnly = object : SourceAnalysisRequestEvent(this, mock(NamedEntity::class.java)) { + override fun incrementalSource(): IncrementalSource = + throw UnsupportedOperationException("not needed by this compatibility test") + + override fun profile(): ExtractionContentProfileRef = profile + } + + assertSame(profile, profileOnly.profile()) + assertNull(profileOnly.currentRun()) + assertNull(profileOnly.sourceLocator()) + } + + @Test + fun `the base constructor descriptor is still source and user only`() { + val constructor = SourceAnalysisRequestEvent::class.java.declaredConstructors.single() + + assertEquals(2, constructor.parameterCount) + assertEquals( + listOf(Any::class.java, NamedEntity::class.java), + constructor.parameterTypes.toList(), + ) + } + + private class LegacyEvent( + source: Any, + user: NamedEntity, + ) : SourceAnalysisRequestEvent(source, user) { + + override fun incrementalSource(): IncrementalSource = + throw UnsupportedOperationException("not needed by this compatibility test") + } +} diff --git a/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionProfileContractTest.kt b/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionProfileContractTest.kt new file mode 100644 index 00000000..f2d19b1b --- /dev/null +++ b/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionProfileContractTest.kt @@ -0,0 +1,144 @@ +/* + * 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.fasterxml.jackson.module.kotlin.jacksonObjectMapper +import com.fasterxml.jackson.module.kotlin.readValue +import org.assertj.core.api.Assertions.assertThat +import org.assertj.core.api.Assertions.assertThatIllegalArgumentException +import org.junit.jupiter.api.Test + +/** + * What the two new reference types promise: stable name-and-version identity for a profile, + * an opaque id for a run, bounded strings, and no interpretation of either. + */ +class ExtractionProfileContractTest { + + private val objectMapper = jacksonObjectMapper() + + @Test + fun `profile ref preserves opaque name and version`() { + // Values a host might plausibly mint, including punctuation and non-ASCII. DICE never + // parses these, so nothing here is a reserved character. + val ref = ExtractionContentProfileRef( + name = "house/style:with spaces & 記号", + version = "2026-08-31+build.7", + ) + + assertThat(ref.name).isEqualTo("house/style:with spaces & 記号") + assertThat(ref.version).isEqualTo("2026-08-31+build.7") + assertThat( + objectMapper.readValue(objectMapper.writeValueAsString(ref)), + ).isEqualTo(ref) + } + + @Test + fun `profile identity is name and version together`() { + val v1 = ExtractionContentProfileRef("house-style", "v1") + val sameV1 = ExtractionContentProfileRef("house-style", "v1") + val v2 = ExtractionContentProfileRef("house-style", "v2") + val otherName = ExtractionContentProfileRef("legal-review", "v1") + + assertThat(v1).isEqualTo(sameV1) + assertThat(v1.hashCode()).isEqualTo(sameV1.hashCode()) + // Republishing a profile under a new version yields a distinct reference, so runs + // attributed to the old version stay attributed to it. + assertThat(v1).isNotEqualTo(v2) + assertThat(v1).isNotEqualTo(otherName) + assertThat(listOf(v1, sameV1, v2, otherName).distinct()).containsExactly(v1, v2, otherName) + } + + @Test + fun `profile ref rejects blank components`() { + assertThatIllegalArgumentException() + .isThrownBy { ExtractionContentProfileRef(" ", "v1") } + .withMessageContaining("name") + assertThatIllegalArgumentException() + .isThrownBy { ExtractionContentProfileRef("house-style", "\t") } + .withMessageContaining("version") + } + + @Test + fun `profile ref accepts its length caps and rejects one character more`() { + val name = "n".repeat(ExtractionContentProfileRef.MAX_NAME_LENGTH) + val version = "v".repeat(ExtractionContentProfileRef.MAX_VERSION_LENGTH) + + val atCap = ExtractionContentProfileRef(name, version) + assertThat(atCap.name).hasSize(ExtractionContentProfileRef.MAX_NAME_LENGTH) + assertThat(atCap.version).hasSize(ExtractionContentProfileRef.MAX_VERSION_LENGTH) + + assertThatIllegalArgumentException() + .isThrownBy { ExtractionContentProfileRef(name + "n", version) } + .withMessageContaining("name") + assertThatIllegalArgumentException() + .isThrownBy { ExtractionContentProfileRef(name, version + "v") } + .withMessageContaining("version") + } + + @Test + fun `run ref preserves an opaque id`() { + val ref = ExtractionRunRef("01J9Z0V1XQ:host/7#a") + + assertThat(ref.runId).isEqualTo("01J9Z0V1XQ:host/7#a") + assertThat(objectMapper.readValue(objectMapper.writeValueAsString(ref))) + .isEqualTo(ref) + } + + @Test + fun `run identity is the id`() { + val run = ExtractionRunRef("run-1") + val same = ExtractionRunRef("run-1") + val other = ExtractionRunRef("run-2") + + assertThat(run).isEqualTo(same) + assertThat(run.hashCode()).isEqualTo(same.hashCode()) + assertThat(run).isNotEqualTo(other) + } + + @Test + fun `run ref rejects a blank id`() { + assertThatIllegalArgumentException() + .isThrownBy { ExtractionRunRef("") } + .withMessageContaining("runId") + assertThatIllegalArgumentException() + .isThrownBy { ExtractionRunRef(" ") } + .withMessageContaining("runId") + } + + @Test + fun `run ref accepts its length cap and rejects one character more`() { + val runId = "r".repeat(ExtractionRunRef.MAX_RUN_ID_LENGTH) + + assertThat(ExtractionRunRef(runId).runId).hasSize(ExtractionRunRef.MAX_RUN_ID_LENGTH) + assertThatIllegalArgumentException() + .isThrownBy { ExtractionRunRef(runId + "r") } + .withMessageContaining("runId") + } + + @Test + fun `the two references are unrelated types`() { + // Nothing converts one into the other and neither derives from the other. A profile says + // what extraction should do; a run says which execution this was. + assertThat( + ExtractionContentProfileRef::class.java.isAssignableFrom(ExtractionRunRef::class.java), + ).isFalse() + assertThat( + ExtractionRunRef::class.java.isAssignableFrom(ExtractionContentProfileRef::class.java), + ).isFalse() + assertThat(ExtractionContentProfileRef("run-1", "run-1")) + .isNotEqualTo(ExtractionRunRef("run-1")) + } +} 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 7967a23c..08436a7b 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 @@ -39,6 +39,7 @@ import com.embabel.dice.provenance.SourceRevisionRef import com.embabel.dice.provenance.UriLocator import org.slf4j.LoggerFactory import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertFalse import org.junit.jupiter.api.Assertions.assertInstanceOf import org.junit.jupiter.api.Assertions.assertNull import org.junit.jupiter.api.Assertions.assertSame @@ -52,11 +53,13 @@ import org.mockito.kotlin.doNothing import org.mockito.kotlin.eq import org.mockito.kotlin.mock import org.mockito.kotlin.spy +import org.mockito.kotlin.times import org.mockito.kotlin.verify import org.mockito.kotlin.verifyNoInteractions import org.mockito.kotlin.whenever import java.io.ByteArrayInputStream import java.io.InputStream +import java.lang.reflect.Modifier import java.util.concurrent.atomic.AtomicInteger class IncrementalPropositionExtractionTest { @@ -80,26 +83,40 @@ class IncrementalPropositionExtractionTest { .map { it.parameterTypes.toList() } .toSet() + // Every descriptor that existed before profiles is still here, and the profile and run + // arguments only ever add one descriptor on the end of each name. val legacyTextPrefix = listOf( String::class.java, String::class.java, NamedEntity::class.java, ) + val legacyTextFull = legacyTextPrefix + listOf( + List::class.java, + ExtractionPerspective::class.java, + Boolean::class.javaObjectType, + ) + val profileAndRun = listOf( + ExtractionContentProfileRef::class.java, + ExtractionRunRef::class.java, + ) assertEquals( setOf( legacyTextPrefix, legacyTextPrefix + List::class.java, legacyTextPrefix + listOf(List::class.java, ExtractionPerspective::class.java), - legacyTextPrefix + listOf( - List::class.java, - ExtractionPerspective::class.java, - Boolean::class.javaObjectType, - ), + legacyTextFull, + legacyTextFull + profileAndRun, ), rememberTextParameters, ) val sourceTextPrefix = legacyTextPrefix + SourceLocator::class.java + val sourceTextFull = sourceTextPrefix + listOf( + SourceRevisionRef::class.java, + List::class.java, + ExtractionPerspective::class.java, + Boolean::class.javaObjectType, + ) assertEquals( setOf( sourceTextPrefix, @@ -110,12 +127,8 @@ class IncrementalPropositionExtractionTest { List::class.java, ExtractionPerspective::class.java, ), - sourceTextPrefix + listOf( - SourceRevisionRef::class.java, - List::class.java, - ExtractionPerspective::class.java, - Boolean::class.javaObjectType, - ), + sourceTextFull, + sourceTextFull + profileAndRun, ), rememberTextFromSourceParameters, ) @@ -125,14 +138,321 @@ class IncrementalPropositionExtractionTest { String::class.java, NamedEntity::class.java, ) - assertEquals(setOf(legacyFile), rememberFileParameters) + assertEquals( + setOf(legacyFile, legacyFile + profileAndRun), + rememberFileParameters, + ) + val sourceFileFull = + legacyFile + listOf(SourceLocator::class.java, SourceRevisionRef::class.java) assertEquals( setOf( legacyFile + SourceLocator::class.java, - legacyFile + listOf(SourceLocator::class.java, SourceRevisionRef::class.java), + sourceFileFull, + sourceFileFull + profileAndRun, ), rememberFileFromSourceParameters, ) + + // A profile-aware call can never collapse onto a legacy one: a profile and a run always + // arrive together, on the end, and never at an arity a legacy caller already fills. + val everyRememberParameterList = rememberTextParameters + rememberFileParameters + + rememberTextFromSourceParameters + rememberFileFromSourceParameters + everyRememberParameterList.forEach { parameters -> + val hasProfile = ExtractionContentProfileRef::class.java in parameters + val hasRun = ExtractionRunRef::class.java in parameters + assertEquals(hasProfile, hasRun, "profile and run must arrive together: $parameters") + if (hasProfile) { + assertEquals( + profileAndRun, + parameters.takeLast(2), + "profile and run must be the last two parameters: $parameters", + ) + } + } + } + + @Test + fun `the entry point signatures that were overridable before profiles still are`() { + // @JvmOverloads emits every reduced-arity overload as final. Folding the two new + // arguments into the existing declarations would therefore have turned each method's + // pre-change maximum arity — the signature a subclass overrides — into a final bridge. + // Callers would not have noticed; a subclass would have stopped compiling, and one + // already compiled could fail verification at load. Each shape is its own declaration + // instead, and this is the assertion that keeps it that way. + val stillOpen = mapOf( + "rememberText" to listOf( + String::class.java, + String::class.java, + NamedEntity::class.java, + List::class.java, + ExtractionPerspective::class.java, + Boolean::class.javaObjectType, + ), + "rememberFile" to listOf( + InputStream::class.java, + String::class.java, + NamedEntity::class.java, + ), + "rememberTextFromSource" to listOf( + String::class.java, + String::class.java, + NamedEntity::class.java, + SourceLocator::class.java, + SourceRevisionRef::class.java, + List::class.java, + ExtractionPerspective::class.java, + Boolean::class.javaObjectType, + ), + "rememberFileFromSource" to listOf( + InputStream::class.java, + String::class.java, + NamedEntity::class.java, + SourceLocator::class.java, + SourceRevisionRef::class.java, + ), + ) + stillOpen.forEach { (name, parameters) -> + val method = IncrementalPropositionExtraction::class.java + .getMethod(name, *parameters.toTypedArray()) + assertFalse( + Modifier.isFinal(method.modifiers), + "$name${parameters.map { it.simpleName }} must stay overridable", + ) + } + + // The new maximum-arity forms are the single override point every call funnels through, + // so they have to be open too. + val profileAndRun = arrayOf( + ExtractionContentProfileRef::class.java, + ExtractionRunRef::class.java, + ) + stillOpen.forEach { (name, parameters) -> + val method = IncrementalPropositionExtraction::class.java + .getMethod(name, *(parameters.toTypedArray() + profileAndRun)) + assertFalse( + Modifier.isFinal(method.modifiers), + "the profile-aware $name must be overridable", + ) + } + + // The reduced-arity overloads @JvmOverloads generates were final before this slice and + // still are. Stating it pins that the fix restored the previous surface exactly rather + // than widening it. + val generatedBridges = listOf( + "rememberText" to arrayOf>( + String::class.java, + String::class.java, + NamedEntity::class.java, + ), + "rememberTextFromSource" to arrayOf>( + String::class.java, + String::class.java, + NamedEntity::class.java, + SourceLocator::class.java, + ), + "rememberFileFromSource" to arrayOf>( + InputStream::class.java, + String::class.java, + NamedEntity::class.java, + SourceLocator::class.java, + ), + ) + generatedBridges.forEach { (name, parameters) -> + val method = IncrementalPropositionExtraction::class.java.getMethod(name, *parameters) + assertTrue( + Modifier.isFinal(method.modifiers), + "$name at ${parameters.size} arguments was a final bridge before this slice", + ) + } + } + + @Test + fun `a subclass overriding the pre-profile signatures still intercepts every call`() { + val pipeline = pipelineReturningNoResult() + val seen = mutableListOf() + val extraction = object : IncrementalPropositionExtraction( + propositionPipeline = pipeline, + chunkHistoryStore = mock(), + dataDictionary = mock(), + relations = Relations.empty(), + propositionRepository = mock(), + entityRepository = mock(), + entityResolver = mock(), + graphProjectionService = mock(), + properties = extractionProperties(), + ) { + // Written exactly as it would have been before this slice existed. + override fun rememberText( + text: String, + sourceId: String, + user: NamedEntity, + additionalGrounding: List, + perspective: ExtractionPerspective?, + mintNewEntities: Boolean?, + ) { + seen += "text:$sourceId" + } + + override fun rememberFile( + inputStream: InputStream, + filename: String, + user: NamedEntity, + ) { + seen += "file:$filename" + } + } + + extraction.rememberText("t", "three-args", user()) + extraction.rememberText("t", "six-args", user(), emptyList(), null, null) + extraction.rememberFile(ByteArrayInputStream(byteArrayOf()), "legacy.txt", user()) + + // The three-argument call goes through a final generated bridge, which dispatches + // virtually to the six-argument method the subclass overrode. + assertEquals(listOf("text:three-args", "text:six-args", "file:legacy.txt"), seen) + verifyNoInteractions(pipeline) + } + + @Test + fun `a subclass overriding only the pre-profile text methods still intercepts file ingestion`() { + // Before profiles, the file entry points read the file and handed the text to the + // pre-profile text methods, so a subclass overriding only those intercepted file + // ingestion too. Routing the file paths to the wide text methods would have quietly + // taken that away: the override would still compile, still be called for direct text + // calls, and silently stop seeing files. + val pipeline = pipelineReturningNoResult() + val seen = mutableListOf() + val extraction = object : IncrementalPropositionExtraction( + propositionPipeline = pipeline, + chunkHistoryStore = mock(), + dataDictionary = mock(), + relations = Relations.empty(), + propositionRepository = mock(), + entityRepository = mock(), + entityResolver = mock(), + graphProjectionService = mock(), + properties = extractionProperties(), + ) { + override fun rememberText( + text: String, + sourceId: String, + user: NamedEntity, + additionalGrounding: List, + perspective: ExtractionPerspective?, + mintNewEntities: Boolean?, + ) { + seen += "text:$sourceId" + } + + override fun rememberTextFromSource( + text: String, + sourceId: String, + user: NamedEntity, + sourceLocator: SourceLocator, + sourceRevision: SourceRevisionRef?, + additionalGrounding: List, + perspective: ExtractionPerspective?, + mintNewEntities: Boolean?, + ) { + seen += "textFromSource:$sourceId" + } + } + val locator = UriLocator("file:///notes/dispatch.txt") + val revision = SourceRevisionRef(locator.key(), "r1") + + extraction.rememberFile( + ByteArrayInputStream("legacy file text".toByteArray()), + "legacy.txt", + user(), + ) + // A wide call carrying no references dispatches like the pre-profile call it resembles. + extraction.rememberFile( + ByteArrayInputStream("legacy file text".toByteArray()), + "wide-null.txt", + user(), + null, + null, + ) + extraction.rememberFileFromSource( + ByteArrayInputStream("source file text".toByteArray()), + "source.txt", + user(), + locator, + revision, + ) + extraction.rememberFileFromSource( + ByteArrayInputStream("source file text".toByteArray()), + "wide-null-source.txt", + user(), + locator, + revision, + null, + null, + ) + + assertEquals( + listOf( + "text:remember:legacy.txt", + "text:remember:wide-null.txt", + "textFromSource:remember:source.txt", + "textFromSource:remember:wide-null-source.txt", + ), + seen, + ) + verifyNoInteractions(pipeline) + } + + @Test + fun `a file call that carries a profile goes to the wide text entry point`() { + // The other half of the routing rule: once there is a reference to carry, the legacy + // text signature cannot express it, so the call has to go wide. + val pipeline = pipelineReturningNoResult() + val seen = mutableListOf() + val extraction = object : IncrementalPropositionExtraction( + propositionPipeline = pipeline, + chunkHistoryStore = mock(), + dataDictionary = mock(), + relations = Relations.empty(), + propositionRepository = mock(), + entityRepository = mock(), + entityResolver = mock(), + graphProjectionService = mock(), + properties = extractionProperties(), + ) { + override fun rememberText( + text: String, + sourceId: String, + user: NamedEntity, + additionalGrounding: List, + perspective: ExtractionPerspective?, + mintNewEntities: Boolean?, + ) { + seen += "legacy:$sourceId" + } + + override fun rememberText( + text: String, + sourceId: String, + user: NamedEntity, + additionalGrounding: List, + perspective: ExtractionPerspective?, + mintNewEntities: Boolean?, + profile: ExtractionContentProfileRef?, + currentRun: ExtractionRunRef?, + ) { + seen += "wide:$sourceId:${profile?.name}:${currentRun?.runId}" + } + } + + extraction.rememberFile( + ByteArrayInputStream("legacy file text".toByteArray()), + "profiled.txt", + user(), + ExtractionContentProfileRef("house-style", "v1"), + ExtractionRunRef("run-1"), + ) + + assertEquals(listOf("wide:remember:profiled.txt:house-style:run-1"), seen) + verifyNoInteractions(pipeline) } @Test @@ -363,6 +683,8 @@ class IncrementalPropositionExtractionTest { any(), anyOrNull(), anyOrNull(), + anyOrNull(), + anyOrNull(), ) doNothing().whenever(extraction).rememberTextFromSource( any(), @@ -373,6 +695,8 @@ class IncrementalPropositionExtractionTest { any(), anyOrNull(), anyOrNull(), + anyOrNull(), + anyOrNull(), ) extraction.rememberFile( @@ -388,7 +712,18 @@ class IncrementalPropositionExtractionTest { revision, ) - verify(extraction).rememberText("legacy dispatch", "remember:legacy-dispatch.txt", user) + // Both file entry points land on the maximum-arity text method, which is the single + // override point every call funnels through. + verify(extraction).rememberText( + "legacy dispatch", + "remember:legacy-dispatch.txt", + user, + emptyList(), + null, + null, + null, + null, + ) verify(extraction).rememberTextFromSource( "source dispatch", "remember:source-dispatch.txt", @@ -398,6 +733,8 @@ class IncrementalPropositionExtractionTest { emptyList(), null, null, + null, + null, ) verifyNoInteractions(pipeline) } @@ -484,6 +821,205 @@ class IncrementalPropositionExtractionTest { assertTrue(extraction.isIdle, "a rejected event must not leave the extractor busy") } + @Test + fun `profile and run reach the context through both text entry points`() { + val pipeline = pipelineReturningNoResult() + val extraction = extraction(pipeline) + val user = user() + val locator = UriLocator("https://example.com/profiled") + val revision = SourceRevisionRef(locator.key(), "r1") + val profile = ExtractionContentProfileRef("house-style", "v1") + val run = ExtractionRunRef("run-1") + + extraction.rememberText( + text = "legacy text", + sourceId = "legacy:profiled", + user = user, + additionalGrounding = emptyList(), + perspective = null, + mintNewEntities = null, + profile = profile, + currentRun = run, + ) + extraction.rememberTextFromSource( + text = "source text", + sourceId = "source:profiled", + user = user, + sourceLocator = locator, + sourceRevision = revision, + additionalGrounding = emptyList(), + perspective = null, + mintNewEntities = null, + profile = profile, + currentRun = run, + ) + + val fromLegacy = capturedContext(pipeline, "legacy text", "legacy:profiled", emptyList()) + assertSame(profile, fromLegacy.profile) + assertSame(run, fromLegacy.currentRun) + assertNull(fromLegacy.sourceLocator) + + val fromSource = capturedContext(pipeline, "source text", "source:profiled", emptyList()) + assertSame(profile, fromSource.profile) + assertSame(run, fromSource.currentRun) + assertSame(revision, fromSource.sourceRevision) + } + + @Test + fun `profile and run reach the context through both file entry points`() { + val pipeline = pipelineReturningNoResult() + val extraction = extraction(pipeline) + val user = user() + val locator = UriLocator("file:///notes/profiled.txt") + val profile = ExtractionContentProfileRef("house-style", "v1") + val run = ExtractionRunRef("run-1") + + extraction.rememberFile( + inputStream = ByteArrayInputStream("legacy file text".toByteArray()), + filename = "legacy-profiled.txt", + user = user, + profile = profile, + currentRun = run, + ) + extraction.rememberFileFromSource( + inputStream = ByteArrayInputStream("source file text".toByteArray()), + filename = "source-profiled.txt", + user = user, + sourceLocator = locator, + sourceRevision = SourceRevisionRef(locator.key(), "r1"), + profile = profile, + currentRun = run, + ) + // Both file calls land on the profile-aware text entry point, so the two contexts the + // pipeline sees are the proof that carriage survives the file hop. + + val contextCaptor = argumentCaptor() + verify(pipeline, times(2)).processOnce( + any(), + any(), + contextCaptor.capture(), + anyOrNull(), + any(), + eq(emptyList()), + ) + contextCaptor.allValues.forEach { context -> + assertSame(profile, context.profile) + assertSame(run, context.currentRun) + } + } + + @Test + fun `legacy calls carry no profile and no run`() { + val pipeline = pipelineReturningNoResult() + val extraction = extraction(pipeline) + val user = user() + + extraction.rememberText(text = "legacy text", sourceId = "legacy:plain", user = user) + extraction.rememberFile( + inputStream = ByteArrayInputStream("legacy file text".toByteArray()), + filename = "legacy-plain.txt", + user = user, + ) + + val contextCaptor = argumentCaptor() + verify(pipeline, times(2)).processOnce( + any(), + any(), + contextCaptor.capture(), + anyOrNull(), + any(), + eq(emptyList()), + ) + contextCaptor.allValues.forEach { context -> + assertNull(context.profile) + assertNull(context.currentRun) + } + } + + @Test + fun `a profile changes nothing else about what the pipeline is asked to do`() { + val pipeline = pipelineReturningNoResult() + val extraction = extraction(pipeline) + val user = user() + val grounding = listOf("record:one") + val profile = ExtractionContentProfileRef("house-style", "v1") + val run = ExtractionRunRef("run-1") + + extraction.rememberText( + text = "same text", + sourceId = "same:id", + user = user, + additionalGrounding = grounding, + perspective = ExtractionPerspective.USER, + mintNewEntities = true, + ) + extraction.rememberText( + text = "same text", + sourceId = "same:id", + user = user, + additionalGrounding = grounding, + perspective = ExtractionPerspective.USER, + mintNewEntities = true, + profile = profile, + currentRun = run, + ) + + val contextCaptor = argumentCaptor() + verify(pipeline, times(2)).processOnce( + eq("same text"), + eq("same:id"), + contextCaptor.capture(), + anyOrNull(), + any(), + eq(grounding), + ) + val (plain, profiled) = contextCaptor.allValues + + // Comparing whole contexts is the point: they agree on every component but the two the + // second call set. The resolver is substituted because buildContext constructs a fresh + // one per call by design, so it is never the same instance twice. + assertEquals( + plain, + profiled.copy(profile = null, currentRun = null, entityResolver = plain.entityResolver), + ) + assertSame(profile, profiled.profile) + assertSame(run, profiled.currentRun) + } + + @Test + fun `event profile and run reach the context observed by the pipeline`() { + val pipeline = pipelineReturningNoResult() + val extraction = extraction(pipeline) + val source = mock>() + whenever(source.id).thenReturn("event-source") + whenever(source.size).thenReturn(1) + val profile = ExtractionContentProfileRef("house-style", "v1") + val run = ExtractionRunRef("run-1") + val profileCalls = AtomicInteger() + val runCalls = AtomicInteger() + val event = object : SourceAnalysisRequestEvent(this, user()) { + override fun incrementalSource(): IncrementalSource = source + + override fun profile(): ExtractionContentProfileRef = + profile.also { profileCalls.incrementAndGet() } + + override fun currentRun(): ExtractionRunRef = + run.also { runCalls.incrementAndGet() } + } + + extraction.extractPropositions(event) + + // One read each: the async path builds one context through the same buildContext the + // direct calls use, so there is nowhere else for a second read to happen. + assertEquals(1, profileCalls.get()) + assertEquals(1, runCalls.get()) + val contextCaptor = argumentCaptor() + verify(pipeline).processChunk(any(), contextCaptor.capture()) + assertSame(profile, contextCaptor.firstValue.profile) + assertSame(run, contextCaptor.firstValue.currentRun) + assertNull(contextCaptor.firstValue.sourceLocator) + } + private fun event( sourceId: String, locator: SourceLocator, @@ -518,11 +1054,15 @@ class IncrementalPropositionExtractionTest { ) } + private fun extractionProperties(): PropositionExtractionProperties = + mock().also { properties -> + whenever(properties.windowSize).thenReturn(1) + whenever(properties.overlapSize).thenReturn(1) + whenever(properties.triggerInterval).thenReturn(1) + } + private fun extraction(pipeline: PropositionPipeline): IncrementalPropositionExtraction { - val properties = mock() - whenever(properties.windowSize).thenReturn(1) - whenever(properties.overlapSize).thenReturn(1) - whenever(properties.triggerInterval).thenReturn(1) + val properties = extractionProperties() return IncrementalPropositionExtraction( propositionPipeline = pipeline, chunkHistoryStore = mock(), diff --git a/docs/design/INDEX.md b/docs/design/INDEX.md index c8f62f55..9478abe8 100644 --- a/docs/design/INDEX.md +++ b/docs/design/INDEX.md @@ -21,6 +21,10 @@ you need. - [entity-resolution-and-text2graph.md](entity-resolution-and-text2graph.md) — how mentions get matched to existing entities (or minted as new ones) without blowing the LLM budget or fragmenting the graph with near-duplicates. +- [extraction-profiles.md](extraction-profiles.md) — carrying a host's content-policy identity + and a run reference through extraction without DICE resolving either: why profile identity is + opaque, why the run reference ships ahead of the run, and why profile, perspective, schema and + tenant stay independent. EXPERIMENTAL. ## Propositions & lifecycle diff --git a/docs/design/extraction-profiles.md b/docs/design/extraction-profiles.md new file mode 100644 index 00000000..b165af76 --- /dev/null +++ b/docs/design/extraction-profiles.md @@ -0,0 +1,268 @@ +# Extraction profiles: opaque policy identity, carried and never resolved + +An extraction content profile is the host's durable answer to "what should extraction of this +kind of material do?" DICE carries its name and version and nothing else. It never looks a +profile up, never reads policy out of it, and never routes on it. The host owns the catalog, +authorizes who may use which profile, and binds it to whatever it actually means. + +This note covers DICE #66: the two reference types (`ExtractionContentProfileRef` and +`ExtractionRunRef`), where they sit on `SourceAnalysisContext` and the extraction entry points, +and why profile, perspective, schema and tenant are four independent dimensions rather than one +knob with four names. + +## Why the identity is opaque + +A profile reference is two strings: + +```kotlin +data class ExtractionContentProfileRef( + val name: String, + val version: String, +) +``` + +Neither is parsed. DICE compares them, carries them, and stores them (from #67 onward). The +alternative — DICE resolving a profile into an actual configuration — costs three things it has +no business owning: + +- **A catalog.** Someone would have to hold the mapping from profile to policy, and it would have + to be reachable from inside extraction. Every host already has that mapping; DICE would be + duplicating it, and the two copies would drift. +- **An authorization model.** "May this caller use the `legal-review` profile?" is a product + question with a product's answer. DICE has no user model to answer it with, and a reference it + cannot authorize is a reference it must not dereference. +- **A connector per host.** The moment a profile selects behaviour, DICE branches on which host it + is talking to. That is the connector-branching-inside-DICE outcome #66 exists to avoid. + +So the split is: **DICE carries profile identity; the host authorizes and binds it.** A host that +wants a particular prompt, a particular temperature, or a particular model for a given profile +makes that decision on its own side, before it calls DICE, and passes the reference along so the +resulting claims can be attributed to the policy they were extracted under. + +**A profile selects no provider, no model, and no credential.** That is a contract, not an +oversight, and it is stated in the KDoc of every surface that accepts one. Nothing downstream of +`buildContext` reads either reference; a test compares the whole context built with a profile +against the one built without and asserts they differ in exactly that field. + +Identity is name *and* version together. A host that republishes `house-style` as `v2` gets a +distinct reference, so runs attributed to `v1` stay attributed to `v1` and a later comparison of +the two is a comparison of two identities rather than an archaeology problem. + +Both strings are bounded — 256 characters for a name, 64 for a version — because #67 stores them +in run headers and indexes them. A reference is an identifier the host mints, not a place to put +a payload. + +## Why the run reference ships before the run + +`ExtractionRunRef` is a single opaque id and nothing else: no timing, no status, no counts, no +lineage. + +```kotlin +data class ExtractionRunRef( + val runId: String, +) +``` + +Durable extraction runs are DICE #67, a whole wave behind this one. The run model will be keyed +by (`ContextId`, `ExtractionRunRef`). Shipping the reference now means the entry points and the +run model meet at an opaque string rather than at a type one of them has to import from the +other's release: this slice can carry a run through extraction while #67 is still being designed, +and #67 can key its store on a type that already exists and is already tested. + +Carrying a reference is always allowed, and DICE does not check that the run exists — there is +nowhere yet to check against. What a store does with a reference to a run it has never seen is +#67's decision; this slice makes no promise about it in either direction, so #67 stays free to +require referential integrity when it wires proposition-to-run attribution. DICE mints no run ids +here either: a run is something the host starts, or, from #67's wiring slice, something DICE's own +run coordinator starts. + +Two properties the reference commits to now, because they are cheaper to hold than to retrofit: + +- **It is not part of source-provenance equality.** Two runs over the same material still produce + one piece of source evidence. What differs is which runs are attributed to it, which is a + separate relation in #67 rather than a field on the evidence. +- **It is a pseudonymous token.** Not an authorization token, not a direct identifier, and not + something a reader can dereference into personal data. Length-capped at 256 characters for the + same reason the profile strings are. + +## Four independent dimensions + +Perspective is the dimension a profile is most easily confused with, so the difference is worth +stating plainly. **Perspective describes conversational input** — whose statements to mine out of +a transcript. **A profile is content policy** — what the host wants done with material of this +kind. They answer different questions, and a caller combines them freely. + +```mermaid +flowchart TD + CTX["SourceAnalysisContext"] + CTX --> P["profile
ExtractionContentProfileRef?
what policy this ran under"] + CTX --> E["perspective
ExtractionPerspective?
whose statements to mine"] + CTX --> S["schema
DataDictionary
what types exist"] + CTX --> T["contextId
ContextId
which tenant owns the result"] + CTX --> R["currentRun
ExtractionRunRef?
which execution this was"] + CTX -.-> L["sourceLocator + sourceRevision
what is being read"] + N["the one coupling on the context:
a revision requires a locator
whose key it matches"] + L --- N +``` + +The dotted branch is the only coupling on the context: a `sourceRevision` requires a +`sourceLocator` whose key it matches, because a revision names a version of a specific source. +Profile and run are checked against nothing, and the `init` block says so — validating them +against another dimension would invent a relationship the contract does not have. + +Independence is pinned as a matrix rather than argued. `ExtractionContextIndependenceTest` builds +every combination of 4 profiles (including absent) × 4 perspectives (including absent) × 2 schemas +× 2 tenants — 64 contexts — and asserts three things: + +1. every cell is constructible and reads back exactly the values it was given; +2. for every ordered pair of dimensions, the observed pairs are the whole cross product, so a + dimension that quietly disabled, defaulted or rejected another would leave a hole; +3. varying one dimension leaves the other three identical. + +A fourth case pins the copy helpers by comparing `withProfile(p)` against `copy(profile = p)`, +which is a statement about all thirteen components at once rather than about the one field the +helper names. + +## Carrying it in + +Everything that reaches extraction builds a `SourceAnalysisContext`, and both entry paths build it +through one `buildContext` call — the same structural argument Wave A used for source revisions, +for the same reason: parallel code in two places is where two paths start behaving differently. + +```mermaid +flowchart TD + A["rememberText / rememberTextFromSource
rememberFile / rememberFileFromSource"] --> C + B["SourceAnalysisRequestEvent.profile() / currentRun()"] --> C + C["IncrementalPropositionExtraction.buildContext"] --> CTX + CTX["SourceAnalysisContext.profile
SourceAnalysisContext.currentRun"] --> X["carried, never consulted"] +``` + +| Path | How a profile arrives | +| --- | --- | +| `rememberText`, `rememberTextFromSource` | trailing optional `profile` and `currentRun` arguments | +| `rememberFile`, `rememberFileFromSource` | the same two arguments, forwarded to the text call | +| async `SourceAnalysisRequestEvent` | `profile()` / `currentRun()`, both open and null-defaulted | +| `ConversationAnalysisRequestEvent` | its longer constructor, whose `sourceLocator` is nullable so a publisher can name a profile for material it has no typed source for | + +The direct calls take extra arguments rather than getting their own method names, which is the +opposite of what Wave A did for `rememberTextFromSource`. The reason is the difference in the +contracts: a locator is *required* by the source-aware calls, so those are genuinely different +methods and separate names keep every call site unambiguous. A profile is optional everywhere, so +an extra name would buy nothing and double the surface. + +### Why each entry point is two declarations + +Growing the existing declarations with two defaulted parameters would have been the obvious +shape, and it is wrong. `@JvmOverloads` emits every reduced-arity overload as `final`, even on an +`open` function — only the declared maximum arity stays open. So adding `profile` and `currentRun` +to `rememberText` would have moved the open declaration from six arguments to eight and re-emitted +the six-argument form as a final bridge. Callers would not have noticed. A subclass overriding the +six-argument form would have stopped compiling, and one already compiled could fail verification +at class load. + +Each entry point is therefore two declarations: the pre-profile signature exactly as it was +(`@JvmOverloads` where it already had it), delegating to a new maximum-arity form that takes the +two references and carries no defaults. The defaults are what force the split — two overloads that +both supply defaults for the same arity are ambiguous at a Kotlin call site, so the wide form +spells every argument out. + +| Method | Overridable before | Overridable now | +| --- | --- | --- | +| `rememberText` | 6 args | 6 args, and 8 | +| `rememberTextFromSource` | 8 args | 8 args, and 10 | +| `rememberFile` | 3 args | 3 args, and 5 | +| `rememberFileFromSource` | 5 args | 5 args, and 7 | + +The reduced arities `@JvmOverloads` generates were final before and still are — the fix restores +the previous surface rather than widening it. + +### Which chain a call takes + +Keeping the old signatures overridable is only half of it. They also have to still be *reached*. +Before profiles, `rememberFile` read the file and handed the text to the six-argument +`rememberText`, so a subclass overriding only that one intercepted file ingestion as well. +Routing the file paths straight to the wide text methods would have taken that away silently: the +override would still compile, still fire for direct text calls, and stop seeing files. + +So the rule is that a call dispatches like the call it resembles. A file call carrying no +references takes the pre-profile chain; one carrying a reference has to go wide, because the +legacy text signature cannot express a profile. + +```mermaid +flowchart LR + F3["rememberFile(3)"] --> T6 + F5["rememberFile(5)"] -->|"both refs null"| F3 + F5 -->|"a ref is present"| T8 + T6["rememberText(6)"] --> T8["rememberText(8)"] --> I["rememberTextInternal"] +``` + +`rememberFileFromSource` and `rememberTextFromSource` mirror it at their own arities. The wide +text forms are terminal and never route back to the legacy ones — that would be a cycle, since +the legacy forms delegate forwards. + +Two consequences worth stating. A subclass overriding only a pre-profile text method sees +everything it used to, files included. And unintercepted, every call still ends at the wide text +form, so a host that wants one place to see all traffic overrides that. + +The async path reads each accessor exactly once, which a test asserts by counting: there is one +`buildContext` call and nowhere else for a second read to happen. + +## Compatibility + +**Additive, with the same scoped ABI boundary Wave A declared.** + +- Every Java-visible `SourceAnalysisContext` constructor descriptor that existed before this slice + is still published. `@JvmOverloads` adds the two new ones on the end. A test enumerates arities + 3 through 13 (each with the trailing `DefaultConstructorMarker` Kotlin emits because `contextId` + is a value class) and asserts every one resolves. +- Every `rememberText`, `rememberTextFromSource`, `rememberFile` and `rememberFileFromSource` + descriptor survives, and the new arguments add exactly one descriptor per method name, on the + end. A test asserts the exact descriptor set of all four names, that a profile and a run always + arrive together, and that they are always the last two parameters. +- **Subclass-override compatibility is claimed, and covers being reached as well as being + overridable.** Every signature that was overridable before this slice still is, and every + pre-profile call still dispatches through it. A reflection test asserts `Modifier.isFinal` is + false on all four pre-profile signatures and on the four new maximum-arity forms, and true on + the reduced arities that were final bridges already. A Java subclass in the compat suite + overrides all four pre-profile signatures; `javac` rejects `@Override` on a final method, so the + suite compiling is the second proof. A Kotlin test constructs a subclass overriding the + six-argument `rememberText` and the three-argument `rememberFile` and asserts a three-argument + call still reaches the override. Overridability alone is not enough, so two further tests pin + the dispatch: a subclass overriding only the pre-profile *text* methods still sees both file + entry points, and a file call that carries a profile goes to the wide text form instead. +- `ConversationAnalysisRequestEvent` keeps its five-argument constructor and gains six- and + seven-argument forms. Its `sourceLocator` parameter relaxes from non-null to nullable, which + accepts strictly more calls than before. +- **Full Kotlin synthetic `copy` and `componentN` ABI is not claimed.** Adding two fields to a + data class rewrites `copy` and adds two `componentN` methods, so Kotlin code compiled against an + earlier jar must be recompiled rather than swapped in. This is the same half of the boundary + #64 declined, for the same mechanical reason, and a test pins it: exactly one `copy` remains and + it takes thirteen arguments. +- No stored data changes. Nothing serializes a profile or a run reference yet; #67 is where they + reach a store. + +## Status: EXPERIMENTAL + +`ExtractionContentProfileRef` and `ExtractionRunRef` both carry `@ApiStatus.Experimental`, which +is the marker DICE already uses for API that may still move (`PrologProjector`, +`PropositionStatus.STALE`). The KDoc on both types and on every new parameter says the same in +words, and the CHANGELOG entry is labelled. + +The shape most likely to move is `ExtractionRunRef`, because #67 is where a run acquires +everything else about itself and may want the reference to carry more than one string. + +A Kotlin `@RequiresOptIn` annotation would make the experimental status enforceable at the call +site rather than advisory. Nothing in DICE defines one today, and inventing an opt-in marker is a +policy decision about the whole public surface, not about this slice. It is recorded here as an +open question rather than answered. + +## What this slice does not do + +- **No host profile catalog.** DICE holds no mapping from a reference to a policy and offers no + place to put one. +- **No durable run.** `ExtractionRunRef` names a run; nothing stores one until #67. +- **No REST surface.** `POST /extract` takes no profile. The REST request carries a source locator + and a revision from Wave A and is otherwise unchanged; adding profile there is a separate + decision about the authorized boundary, not a mechanical extension of this slice. +- **No behaviour.** Extraction, resolution and revision ordering are behaviour-identical. A + profile changes what a run is *attributed to*, not what it does. From a72c9aed21061f377e064bac199f2bb1e6716890 Mon Sep 17 00:00:00 2001 From: James Dunnam <7660553+jimador@users.noreply.github.com> Date: Mon, 31 Aug 2026 17:44:39 -0400 Subject: [PATCH 2/3] Carry profile identity without promising run attribution ExtractionRunRef had no consumer on this branch: nothing read it, and no write stamped a run with it. It comes back on the run-lineage branch together with the lineage write that gives it a reason to exist. The prose is pulled back to what ships. DICE carries an ExtractionContentProfileRef to the extractor and persists no run or claim association, so the documents now claim only that identity is name and version together and that a republished profile yields a reference the host can tell apart from the old one. A pin test guards the removal: it asserts ExtractionRunRef is absent, fixes the persistAndProject overload count, and sweeps generic field, return, and parameter signatures across the carrier types so a run reference cannot re-enter unnoticed. Signed-off-by: James Dunnam <7660553+jimador@users.noreply.github.com> --- CHANGELOG.md | 125 +++++----- .../ConversationAnalysisRequestEvent.kt | 17 +- .../dice/common/SourceAnalysisContext.kt | 20 +- .../dice/common/SourceAnalysisRequestEvent.kt | 13 +- .../extraction/ExtractionRunRef.kt | 68 ------ .../IncrementalPropositionExtraction.kt | 59 ++--- .../ExtractionProfileJavaInteropTest.java | 96 +++----- .../ExtractionProfileCompatibilityTest.kt | 36 +-- .../ExtractionContextIndependenceTest.kt | 29 --- .../SourceAnalysisRequestEventProfileTest.kt | 20 +- .../ExtractionProfileContractTest.kt | 58 +---- .../IncrementalPropositionExtractionTest.kt | 171 +++++++++----- docs/design/INDEX.md | 6 +- docs/design/extraction-profiles.md | 218 +++++++++--------- 14 files changed, 375 insertions(+), 561 deletions(-) delete mode 100644 dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunRef.kt diff --git a/CHANGELOG.md b/CHANGELOG.md index 312b1907..e3239cd8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1079,49 +1079,41 @@ and the consumer PRs that deliver it). No stored data migrates: `sourceRevision` stays absent from the JSON of a revisionless entry, which is byte-identical to what was written before. -- **EXPERIMENTAL.** Versioned extraction content profiles and an extraction run reference carried - through the extraction entry points (DICE #66). `ExtractionContentProfileRef(name, version)` names - a version of a host's content profile — the host's durable answer to what extraction of this kind - of material should do. DICE carries the two strings and nothing else: it never looks a profile up, - never reads policy out of it, and **selects no provider, model, or credential from it**. The host - owns the catalog, authorizes the reference, and binds it to whatever it means. Identity is name - and version together, so republishing a profile under a new version yields a distinct reference - and runs attributed to the older one stay attributed to it. `ExtractionRunRef(runId)` is identity - and nothing else — no timing, status, counts, or lineage — and ships ahead of the durable - extraction run it will key in DICE #67, so the entry points and the run model meet at an opaque - string rather than at a type one has to import from the other's release. Passing a reference for - a run is always legal to carry, and DICE does not check that the run exists — there is nowhere - yet to check against, and what a store does with a reference to a run it has never seen is #67's - decision rather than a promise made here. Both types validate - non-blank components and cap their lengths (256 for a profile name or a run id, 64 for a version), - because #67 stores and indexes these strings and a reference is an identifier rather than a place - to put a payload. Neither is an authorization token, and neither may carry a direct identifier or - a dereferenceable secret. `SourceAnalysisContext` gains optional `profile` and `currentRun`, both - defaulting to null, with `withProfile` and `withCurrentRun` copy helpers. Neither is checked - against any other field: a `sourceRevision` is coupled to its `sourceLocator` because it names a - version of a specific source, while a profile and a run reference are independent of everything - else, and the `init` block says so rather than inventing a relationship the contract does not - have. `rememberText`, `rememberTextFromSource`, `rememberFile` and `rememberFileFromSource` each - take `profile` and `currentRun` as trailing arguments — extra arguments rather than new method - names, which is the opposite of Wave A's `rememberTextFromSource` split, because a locator is - *required* by the source-aware calls and a profile is optional everywhere. Each of the four is - now two declarations: the pre-profile signature exactly as it was, delegating to a new - maximum-arity form that takes the two references. That split is deliberate — `@JvmOverloads` - emits every reduced-arity overload as `final` even on an `open` function, so folding the new - arguments into the existing declarations would have turned each method's pre-change maximum - arity into a final bridge and broken subclasses that override it. Dispatch follows the same - principle: a call that carries no profile and no run takes the chain it took before profiles - existed — the file entry points hand their text to the *pre-profile* text signature, so a - subclass overriding only that one still intercepts file ingestion — while a call that actually - carries a reference goes wide, because the legacy signature cannot express one. The wide forms - are terminal and never route back, so there is no cycle. Unintercepted, every call still ends - at the maximum-arity text form, so overriding that one sees all traffic. - `SourceAnalysisRequestEvent` gains `profile()` and `currentRun()`, both open and null-defaulted, - and `ConversationAnalysisRequestEvent` takes both on its longer constructor. Both paths feed one +- **EXPERIMENTAL.** Versioned extraction content profiles carried through the extraction entry + points (DICE #66). `ExtractionContentProfileRef(name, version)` names a version of a host's + content profile — the host's durable answer to what extraction of this kind of material should + do. DICE carries the two strings and nothing else: it never looks a profile up, never reads + policy out of it, and **selects no provider, model, or credential from it**. The host owns the + catalog, authorizes the reference, and binds it to whatever it means. Identity is name and + version together, so republishing a profile under a new version yields a reference the host can + tell apart from the old one. The type validates non-blank components + and caps their lengths (256 for a name, 64 for a version), because a reference is an identifier + rather than a place to put a payload. It is not an authorization token, and may not carry a + direct identifier or a dereferenceable secret. `SourceAnalysisContext` gains an optional + `profile`, defaulting to null, with a `withProfile` copy helper. It is checked against no other + field: a `sourceRevision` is coupled to its `sourceLocator` because it names a version of a + specific source, while a profile is independent of everything else, and the `init` block says + so rather than inventing a relationship the contract does not have. `rememberText`, + `rememberTextFromSource`, `rememberFile` and `rememberFileFromSource` each take `profile` as a + trailing argument — an extra argument rather than a new method name, which is the opposite of + Wave A's `rememberTextFromSource` split, because a locator is *required* by the source-aware + calls and a profile is optional everywhere. Each of the four is now two declarations: the + pre-profile signature exactly as it was, delegating to a new maximum-arity form that takes the + reference. That split is deliberate — `@JvmOverloads` emits every reduced-arity overload as + `final` even on an `open` function, so folding the new argument into the existing declarations + would have turned each method's pre-change maximum arity into a final bridge and broken + subclasses that override it. Dispatch follows the same principle: a call that carries no + profile takes the chain it took before profiles existed — the file entry points hand their text + to the *pre-profile* text signature, so a subclass overriding only that one still intercepts + file ingestion — while a call that actually carries a profile goes wide, because the legacy + signature cannot express one. The wide forms are terminal and never route back, so there is no + cycle. Unintercepted, every call still ends at the maximum-arity text form, so overriding that + one sees all traffic. `SourceAnalysisRequestEvent` gains `profile()`, open and null-defaulted, + and `ConversationAnalysisRequestEvent` takes it on its longer constructor. Both paths feed one `buildContext`, which is what makes the async path carry a profile identically; a test counts - each accessor being read exactly once. Nothing downstream consults either reference — a test + the accessor being read exactly once. Nothing downstream consults the reference — a test compares the whole context built with a profile against the one built without and asserts they - differ in exactly those two fields. Profile, perspective, schema and tenant stay four independent + differ in exactly that field. Profile, perspective, schema and tenant stay four independent dimensions: perspective describes conversational input, a profile is content policy, and a 64-cell matrix test asserts every combination is constructible, that every ordered pair of dimensions realises its whole cross product, and that varying one leaves the other three @@ -1129,12 +1121,12 @@ and the consumer PRs that deliver it). [docs/design/extraction-profiles.md](docs/design/extraction-profiles.md). **Compatibility: additive, with the same scoped ABI boundary as the Wave A slices.** Source and Java constructor-descriptor compatibility are claimed. `@JvmOverloads` on `SourceAnalysisContext` - preserves every published constructor descriptor and adds two on the end; a test enumerates - arities 3 through 13 (each with the trailing `DefaultConstructorMarker` Kotlin emits because + preserves every published constructor descriptor and adds one on the end; a test enumerates + arities 3 through 12 (each with the trailing `DefaultConstructorMarker` Kotlin emits because `contextId` is a value class) and asserts all of them resolve. Every `rememberText`, `rememberTextFromSource`, `rememberFile` and `rememberFileFromSource` descriptor survives, with exactly one added per method name, on the end; a test pins the exact descriptor set of all four - names and that a profile and a run always arrive together as the last two parameters. + names and that the profile is always the last parameter. **Subclass-override compatibility is part of the claimed surface**: every signature that was overridable before this slice still is — `rememberText` at six arguments, `rememberFile` at three, `rememberTextFromSource` at eight, `rememberFileFromSource` at five — and each method's @@ -1147,17 +1139,36 @@ and the consumer PRs that deliver it). Being overridable is not the whole guarantee — the override also has to be reached — so two further tests pin the dispatch rule: a subclass overriding only the pre-profile text methods still sees both file entry points, and a file call carrying a profile goes wide instead. - `ConversationAnalysisRequestEvent` keeps its five-argument constructor and gains - six- and seven-argument forms; its `sourceLocator` parameter relaxes from non-null to nullable, - so a publisher can name a profile for material it has no typed source for, and every call that + `ConversationAnalysisRequestEvent` keeps its five-argument constructor and gains a + six-argument form; its `sourceLocator` parameter relaxes from non-null to nullable, so a + publisher can name a profile for material it has no typed source for, and every call that compiled before still compiles. Full Kotlin synthetic `copy` and `componentN` ABI is **not** - claimed for `SourceAnalysisContext`: two more fields rewrite `copy`, add two `componentN` - methods, and change the synthetic `$default` constructor, so Kotlin code compiled against an - earlier jar must be recompiled rather than swapped in — the same half of the boundary #64 - declined, pinned here by a test asserting exactly one `copy` remains and that it takes thirteen - arguments. No stored data changes and no migration is required: nothing serializes a profile or a - run reference yet. Extraction, resolution, and revision ordering are behaviour-identical; a - profile changes what a run is attributed to, not what it does. `ExtractionContentProfileRef` and - `ExtractionRunRef` both carry `@ApiStatus.Experimental` and their shapes may still move while - #67 lands. A Kotlin `@RequiresOptIn` marker would make that enforceable at the call site rather - than advisory; DICE defines none today and the design note records it as an open question. + claimed for `SourceAnalysisContext`: an added field rewrites `copy`, adds a `componentN` method, + and changes the synthetic `$default` constructor, so Kotlin code compiled against an earlier jar + must be recompiled rather than swapped in — the same half of the boundary #64 declined, pinned + here by a test asserting exactly one `copy` remains and that it takes twelve arguments. No + stored data changes and no migration is required: nothing serializes a profile yet, and no + extractor DICE ships reads `context.profile` either. What exists is the means to build one: a + host-supplied `PropositionExtractor` — the pluggable interface every consumer of + `IncrementalPropositionExtraction` already wires in — receives the whole context on every + `extract()` call, `profile` included, so a host can build a reader for its own content-policy + identity today. DICE carries the profile to the extractor and stops there: every extractor DICE + ships is behaviour-identical with or without one, and a host extractor that chooses to read + `context.profile` is free to act on it however it defines. `ExtractionContentProfileRef` carries + `@ApiStatus.Experimental` and its shape may still move. A Kotlin `@RequiresOptIn` marker would + make that enforceable at the call site rather than advisory; DICE defines none today and the + design note records it as an open question. + + **A run reference travelled with this slice for one round and was pulled back out** (PR #94 + review). `ExtractionRunRef` shipped identity-only, ahead of the durable run store that would key + on it. Nothing on this branch consumes it: `persistAndProject`, the method that actually saves + extraction's output, takes only the pipeline's result and never sees the context that would have + carried a run reference, so a caller passing one got it silently accepted and then dropped. + `currentRun` reached the same `PropositionExtractor.extract` extension point `profile` does, so + the two were equally reachable; the difference was that a run id had no store to resolve against, + so a reader would have had nothing to act on even if one existed. Because that store does not + exist on this branch, `currentRun`/`ExtractionRunRef` are removed from this slice entirely — + `SourceAnalysisContext`, every `remember*` entry point, `SourceAnalysisRequestEvent`, and + `ConversationAnalysisRequestEvent` — and return together with the write that consumes them once + the durable run store lands (DICE #67 and the run-model slices above it). No caller outside this + slice's own code and tests used the parameter for anything, so there is nothing to migrate. diff --git a/dice/src/main/kotlin/com/embabel/dice/common/ConversationAnalysisRequestEvent.kt b/dice/src/main/kotlin/com/embabel/dice/common/ConversationAnalysisRequestEvent.kt index 1d985d62..fc3d9c21 100644 --- a/dice/src/main/kotlin/com/embabel/dice/common/ConversationAnalysisRequestEvent.kt +++ b/dice/src/main/kotlin/com/embabel/dice/common/ConversationAnalysisRequestEvent.kt @@ -21,7 +21,6 @@ import com.embabel.chat.Message import com.embabel.dice.incremental.ConversationSource import com.embabel.dice.incremental.IncrementalSource import com.embabel.dice.proposition.extraction.ExtractionContentProfileRef -import com.embabel.dice.proposition.extraction.ExtractionRunRef import com.embabel.dice.provenance.SourceLocator import com.embabel.dice.provenance.SourceRevisionRef @@ -32,11 +31,13 @@ import com.embabel.dice.provenance.SourceRevisionRef * The three-argument constructor is the one that has always existed and carries no * provenance. A publisher that has a typed source for the conversation — a thread in a * chat system, a transcript file — uses the longer constructor to say so, and the same - * constructor takes an extraction content profile and a run reference. + * constructor takes an extraction content [profile]. EXPERIMENTAL; see [ExtractionContentProfileRef] + * for what carrying it means and does not mean. * - * [sourceLocator] is nullable there because the four things are independent: a publisher can - * name a profile for a conversation it has no typed source for. Only the revision is coupled, - * and to the locator alone — it names a version of that source, so it needs one. + * [sourceLocator] is nullable there because profile and source provenance are two independent + * dimensions: a publisher can name a profile for a conversation it has no typed source for. + * Within source provenance, [sourceRevision] rides on [sourceLocator] — it names a version of + * that source, so it needs one. */ class ConversationAnalysisRequestEvent( source: Any, @@ -50,8 +51,6 @@ class ConversationAnalysisRequestEvent( private var eventProfile: ExtractionContentProfileRef? = null - private var eventCurrentRun: ExtractionRunRef? = null - @JvmOverloads constructor( source: Any, @@ -60,12 +59,10 @@ class ConversationAnalysisRequestEvent( sourceLocator: SourceLocator?, sourceRevision: SourceRevisionRef? = null, profile: ExtractionContentProfileRef? = null, - currentRun: ExtractionRunRef? = null, ) : this(source, user, conversation) { eventSourceLocator = sourceLocator eventSourceRevision = sourceRevision eventProfile = profile - eventCurrentRun = currentRun } override fun incrementalSource(): IncrementalSource = @@ -76,6 +73,4 @@ class ConversationAnalysisRequestEvent( override fun sourceRevision(): SourceRevisionRef? = eventSourceRevision override fun profile(): ExtractionContentProfileRef? = eventProfile - - override fun currentRun(): ExtractionRunRef? = eventCurrentRun } 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..322ad240 100644 --- a/dice/src/main/kotlin/com/embabel/dice/common/SourceAnalysisContext.kt +++ b/dice/src/main/kotlin/com/embabel/dice/common/SourceAnalysisContext.kt @@ -21,7 +21,6 @@ import com.embabel.dice.provenance.SourceLocator import com.embabel.dice.provenance.SourceRevisionRef import com.embabel.dice.proposition.extraction.ExtractionContentProfileRef import com.embabel.dice.proposition.extraction.ExtractionPerspective -import com.embabel.dice.proposition.extraction.ExtractionRunRef /** * Base context for analyzing sources. @@ -46,9 +45,6 @@ import com.embabel.dice.proposition.extraction.ExtractionRunRef * credential, and no DICE code reads policy out of it. The host authorizes the profile and binds * 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. * @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 @@ -76,7 +72,6 @@ data class SourceAnalysisContext @JvmOverloads constructor( val mintedEntityProperties: Map = emptyMap(), val sourceRevision: SourceRevisionRef? = null, val profile: ExtractionContentProfileRef? = null, - val currentRun: ExtractionRunRef? = null, ) { init { @@ -88,10 +83,10 @@ data class SourceAnalysisContext @JvmOverloads constructor( "sourceRevision source key must match sourceLocator source key" } } - // [profile] and [currentRun] are checked against nothing else here, deliberately. A - // revision has to name the source the run is reading, which is why it is coupled to - // [sourceLocator]. A profile and a run reference are independent of every other field, - // and validating them against one would invent a relationship the contract doesn't have. + // [profile] is checked against nothing else here, deliberately. A revision has to name + // the source it was read from, which is why it is coupled to [sourceLocator]. A profile + // is independent of every other field, and validating it against one would invent a + // relationship the contract doesn't have. } companion object { @@ -173,13 +168,6 @@ data class SourceAnalysisContext @JvmOverloads constructor( fun withProfile(profile: ExtractionContentProfileRef): SourceAnalysisContext = copy(profile = profile) - /** - * Returns a copy that says this analysis belongs to the given extraction run. EXPERIMENTAL. - * Changes no other field and no extraction behaviour — see [currentRun]. - */ - fun withCurrentRun(currentRun: ExtractionRunRef): SourceAnalysisContext = - copy(currentRun = currentRun) - /** * Returns a copy allowing (or forbidding) this analysis to persist NEW entities * for mentions the resolver could not match. See [mintNewEntities]. 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 e29e16e3..a10cd4f3 100644 --- a/dice/src/main/kotlin/com/embabel/dice/common/SourceAnalysisRequestEvent.kt +++ b/dice/src/main/kotlin/com/embabel/dice/common/SourceAnalysisRequestEvent.kt @@ -19,7 +19,6 @@ import com.embabel.agent.rag.model.NamedEntity import com.embabel.chat.Message import com.embabel.dice.incremental.IncrementalSource import com.embabel.dice.proposition.extraction.ExtractionContentProfileRef -import com.embabel.dice.proposition.extraction.ExtractionRunRef import com.embabel.dice.provenance.SourceLocator import com.embabel.dice.provenance.SourceRevisionRef import org.springframework.context.ApplicationEvent @@ -35,9 +34,9 @@ import org.springframework.context.ApplicationEvent * the way a direct `rememberTextFromSource` call does. Both default to null, so an * existing subclass carries no provenance and behaves as it always did. * - * [profile] and [currentRun] work the same way and reach the same context through the same - * call, so an async publisher can attribute its extraction to a content profile and a run - * without the listener growing a second code path. Both also default to null. + * [profile] works the same way and reaches the same context through the same call, so an async + * publisher can attribute its extraction to a content profile without the listener growing a + * second code path. It also defaults to null. */ abstract class SourceAnalysisRequestEvent( source: Any, @@ -63,10 +62,4 @@ abstract class SourceAnalysisRequestEvent( * publisher has one. EXPERIMENTAL. DICE carries it and routes nothing on it. */ open fun profile(): ExtractionContentProfileRef? = null - - /** - * 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. - */ - open fun currentRun(): ExtractionRunRef? = null } diff --git a/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunRef.kt b/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunRef.kt deleted file mode 100644 index 37ebb426..00000000 --- a/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRunRef.kt +++ /dev/null @@ -1,68 +0,0 @@ -/* - * 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 - -/** - * Names one extraction run. - * - * This is identity and nothing else. It holds no timing, no status, no counts, no lineage — - * just the id, so a caller can say "this analysis belongs to that run" without DICE having - * anywhere to store a run yet. Durable extraction runs arrive with DICE #67 and will be keyed - * by ([com.embabel.agent.core.ContextId], `ExtractionRunRef`); shipping the reference first - * means the entry points and the run model meet at an opaque string rather than at a type one - * of them has to import from the other's release. - * - * The id is opaque. DICE compares it and carries it and parses nothing out of it. It also never - * mints one: a run is something the host (or, later, DICE's own run coordinator) starts. Nothing - * here checks that the run exists, because there is nowhere yet to check against — carrying a - * reference is always allowed. What a store does with a reference to a run it has never seen is - * DICE #67's to decide, and this type makes no promise about it either way. - * - * Run identity is deliberately not part of source-provenance equality. Two runs over the same - * material still produce one piece of source evidence; what differs is which runs are - * attributed to it. - * - * A run reference is not an authorization token and must not carry a secret, a direct - * identifier, or anything a reader could dereference into personal data. Hosts mint it. - * - * EXPERIMENTAL. The shape may still change while extraction runs (DICE #67) land. - * - * @property runId Host-minted opaque identifier for the run - */ -@ApiStatus.Experimental -data class ExtractionRunRef( - val runId: String, -) { - - init { - require(runId.isNotBlank()) { "runId must not be blank" } - require(runId.length <= MAX_RUN_ID_LENGTH) { - "runId must be at most $MAX_RUN_ID_LENGTH characters, was ${runId.length}" - } - } - - companion object { - - /** - * Longest run id DICE accepts. #67 keys stored runs on this string and indexes it, so - * an unbounded id would become an unbounded key. A uuid, a ULID, or a host's own - * correlation id all fit with room to spare. - */ - const val MAX_RUN_ID_LENGTH: Int = 256 - } -} 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 bafd7165..2cf60c3a 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 @@ -179,20 +179,19 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( } /** - * Extract propositions from a file via Tika, attributed to an extraction content profile - * and a run. EXPERIMENTAL; see [rememberText] for what the two references mean. + * Extract propositions from a file via Tika, attributed to an extraction content profile. + * EXPERIMENTAL; see [rememberText] for what the reference means. * - * With no references to carry this hands straight back to the three-argument form, so a - * call that looks like a pre-profile call also dispatches like one. + * With no profile to carry this hands straight back to the three-argument form, so a call + * that looks like a pre-profile call also dispatches like one. */ open fun rememberFile( inputStream: InputStream, filename: String, user: NamedEntity, profile: ExtractionContentProfileRef?, - currentRun: ExtractionRunRef?, ) { - if (profile == null && currentRun == null) { + if (profile == null) { rememberFile(inputStream, filename, user) return } @@ -205,7 +204,6 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( null, null, profile, - currentRun, ) } } @@ -248,10 +246,10 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( /** * Extract propositions from a file, ground them in the caller's typed source, and attribute - * them to an extraction content profile and a run. EXPERIMENTAL; see [rememberText] for what - * the two references mean. The profile is independent of the locator and the revision. + * them to an extraction content profile. EXPERIMENTAL; see [rememberText] for what the + * reference means. The profile is independent of the locator and the revision. * - * With no references to carry this hands straight back to the five-argument form, so a call + * With no profile to carry this hands straight back to the five-argument form, so a call * that looks like a pre-profile call also dispatches like one. */ open fun rememberFileFromSource( @@ -261,9 +259,8 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( sourceLocator: SourceLocator, sourceRevision: SourceRevisionRef?, profile: ExtractionContentProfileRef?, - currentRun: ExtractionRunRef?, ) { - if (profile == null && currentRun == null) { + if (profile == null) { rememberFileFromSource(inputStream, filename, user, sourceLocator, sourceRevision) return } @@ -281,7 +278,6 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( null, null, profile, - currentRun, ) } } @@ -325,10 +321,10 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( * instance's default; see [SourceAnalysisContext.mintNewEntities]. * * This is the signature that existed before profiles, and it stays its own declaration - * rather than growing two defaulted parameters. `@JvmOverloads` emits every reduced-arity - * overload as `final`, so folding the new arguments into this method would have turned the + * rather than growing a defaulted parameter. `@JvmOverloads` emits every reduced-arity + * overload as `final`, so folding the new argument into this method would have turned the * six-argument form — the one a subclass overrides — into a final bridge. Declaring the two - * shapes separately keeps both open. Unintercepted, every call lands on the eight-argument + * shapes separately keeps both open. Unintercepted, every call lands on the seven-argument * form below, so overriding that one sees everything; overriding this one sees everything a * pre-profile subclass used to see, including file ingestion, which still routes through * here rather than jumping to the wide form. @@ -341,10 +337,10 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( additionalGrounding: List = emptyList(), perspective: ExtractionPerspective? = null, mintNewEntities: Boolean? = null, - ) = rememberText(text, sourceId, user, additionalGrounding, perspective, mintNewEntities, null, null) + ) = rememberText(text, sourceId, user, additionalGrounding, perspective, mintNewEntities, null) /** - * Extract propositions from raw text, attributed to an extraction content profile and a run. + * Extract propositions from raw text, attributed to an extraction content profile. * * Every other text entry point funnels here, so this is the one method to override to see * every call. @@ -353,9 +349,6 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( * EXPERIMENTAL. DICE puts it on the context and does nothing else with it: no provider, * model, or credential is selected from it, and extraction runs exactly as it would * without one. - * @param currentRun reference to the extraction run this call belongs to, or null. - * EXPERIMENTAL. Identity only; nothing is stored under it until DICE #67 lands, and DICE - * does not check that the run exists. */ open fun rememberText( text: String, @@ -365,7 +358,6 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( perspective: ExtractionPerspective?, mintNewEntities: Boolean?, profile: ExtractionContentProfileRef?, - currentRun: ExtractionRunRef?, ) = rememberTextInternal( text = text, @@ -375,7 +367,6 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( perspective = perspective, mintNewEntities = mintNewEntities, profile = profile, - currentRun = currentRun, ) /** @@ -413,14 +404,13 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( perspective, mintNewEntities, null, - null, ) /** * Extract propositions from raw text, ground them in the caller's typed source, and - * attribute them to an extraction content profile and a run. EXPERIMENTAL; see - * [rememberText] for what the two references mean. A profile is independent of the locator - * and the revision — it says what extraction should do, they say what it is reading. + * attribute them to an extraction content profile. EXPERIMENTAL; see [rememberText] for what + * the reference means. A profile is independent of the locator and the revision — it says + * what extraction should do, they say what it is reading. */ open fun rememberTextFromSource( text: String, @@ -432,7 +422,6 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( perspective: ExtractionPerspective?, mintNewEntities: Boolean?, profile: ExtractionContentProfileRef?, - currentRun: ExtractionRunRef?, ) = rememberTextInternal( text = text, @@ -444,7 +433,6 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( perspective = perspective, mintNewEntities = mintNewEntities, profile = profile, - currentRun = currentRun, ) private fun rememberTextInternal( @@ -457,7 +445,6 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( perspective: ExtractionPerspective? = null, mintNewEntities: Boolean? = null, profile: ExtractionContentProfileRef? = null, - currentRun: ExtractionRunRef? = null, ) { val context = buildContext( user = user, @@ -467,7 +454,6 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( sourceLocator = sourceLocator, sourceRevision = sourceRevision, profile = profile, - currentRun = currentRun, ) val result = propositionPipeline.processOnce( text, sourceId, context, additionalGrounding = additionalGrounding, @@ -516,7 +502,7 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( } // The async path grounds propositions exactly the way rememberTextFromSource does: - // whatever provenance, profile and run the event carries goes through the same + // whatever provenance and profile the event carries goes through the same // buildContext call. val context = buildContext( user = event.user, @@ -524,7 +510,6 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( sourceLocator = event.sourceLocator(), sourceRevision = event.sourceRevision(), profile = event.profile(), - currentRun = event.currentRun(), ) logger.info( "Context relations count: {}, injected relations count: {}", @@ -559,7 +544,6 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( sourceLocator: SourceLocator? = null, sourceRevision: SourceRevisionRef? = null, profile: ExtractionContentProfileRef? = null, - currentRun: ExtractionRunRef? = null, ): SourceAnalysisContext { val aliases = try { currentUserAliasesProvider(user) @@ -625,14 +609,11 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( if (sourceRevision != null) { ctx = ctx.withSourceRevision(sourceRevision) } - // 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. + // Carried, never consulted. Nothing downstream of here reads it — that is what "DICE + // holds profile identity and the host binds policy" means in code. if (profile != null) { ctx = ctx.withProfile(profile) } - if (currentRun != null) { - ctx = ctx.withCurrentRun(currentRun) - } return ctx } diff --git a/dice/src/test/java/com/embabel/dice/ExtractionProfileJavaInteropTest.java b/dice/src/test/java/com/embabel/dice/ExtractionProfileJavaInteropTest.java index 56a7099b..4c948c03 100644 --- a/dice/src/test/java/com/embabel/dice/ExtractionProfileJavaInteropTest.java +++ b/dice/src/test/java/com/embabel/dice/ExtractionProfileJavaInteropTest.java @@ -26,7 +26,6 @@ import com.embabel.dice.incremental.IncrementalSource; import com.embabel.dice.proposition.extraction.ExtractionContentProfileRef; import com.embabel.dice.proposition.extraction.ExtractionPerspective; -import com.embabel.dice.proposition.extraction.ExtractionRunRef; import com.embabel.dice.proposition.extraction.IncrementalPropositionExtraction; import com.embabel.dice.provenance.ContentAddressedLocator; import com.embabel.dice.provenance.SourceLocator; @@ -47,8 +46,8 @@ /** * Java's view of the profile contract, with and without a profile present. * {@code @JvmOverloads} means the remember entry points keep every descriptor a Java caller - * could already have compiled against, and the profile and run arguments only ever add - * descriptors on the end. + * could already have compiled against, and the profile argument only ever adds a descriptor + * on the end. */ class ExtractionProfileJavaInteropTest { @@ -60,7 +59,7 @@ private SourceAnalysisContext context() { } @Test - void javaCallersBuildAndReadProfileAndRunValues() { + void javaCallersBuildAndReadProfileValues() { ExtractionContentProfileRef profile = new ExtractionContentProfileRef("house-style", "v1"); assertEquals("house-style", profile.getName()); assertEquals("v1", profile.getVersion()); @@ -70,28 +69,19 @@ void javaCallersBuildAndReadProfileAndRunValues() { IllegalArgumentException.class, () -> new ExtractionContentProfileRef(" ", "v1") ); - - ExtractionRunRef run = new ExtractionRunRef("run-1"); - assertEquals("run-1", run.getRunId()); - assertEquals(run, new ExtractionRunRef("run-1")); - assertThrows(IllegalArgumentException.class, () -> new ExtractionRunRef("")); } @Test void javaBuiltContextsCarryNoProfileUnlessAsked() { SourceAnalysisContext absent = context(); assertNull(absent.getProfile()); - assertNull(absent.getCurrentRun()); ExtractionContentProfileRef profile = new ExtractionContentProfileRef("house-style", "v1"); - ExtractionRunRef run = new ExtractionRunRef("run-1"); - SourceAnalysisContext present = absent.withProfile(profile).withCurrentRun(run); + SourceAnalysisContext present = absent.withProfile(profile); assertSame(profile, present.getProfile()); - assertSame(run, present.getCurrentRun()); // The copy is a copy: the original is untouched. assertNull(absent.getProfile()); - assertNull(absent.getCurrentRun()); } @Test @@ -109,25 +99,14 @@ void retainsEveryLegacyRememberDescriptorAndAddsProfileAwareOnes() throws Except IncrementalPropositionExtraction.class.getMethod( "rememberText", String.class, String.class, NamedEntity.class, List.class, - ExtractionPerspective.class, Boolean.class, ExtractionContentProfileRef.class, - ExtractionRunRef.class - ); - // A profile and a run always arrive together; there is no profile-only descriptor. - assertThrows( - NoSuchMethodException.class, - () -> IncrementalPropositionExtraction.class.getMethod( - "rememberText", - String.class, String.class, NamedEntity.class, List.class, - ExtractionPerspective.class, Boolean.class, - ExtractionContentProfileRef.class - ) + ExtractionPerspective.class, Boolean.class, ExtractionContentProfileRef.class ); IncrementalPropositionExtraction.class.getMethod( "rememberTextFromSource", String.class, String.class, NamedEntity.class, SourceLocator.class, SourceRevisionRef.class, List.class, ExtractionPerspective.class, Boolean.class, - ExtractionContentProfileRef.class, ExtractionRunRef.class + ExtractionContentProfileRef.class ); IncrementalPropositionExtraction.class.getMethod( @@ -136,15 +115,15 @@ void retainsEveryLegacyRememberDescriptorAndAddsProfileAwareOnes() throws Except IncrementalPropositionExtraction.class.getMethod( "rememberFile", InputStream.class, String.class, NamedEntity.class, - ExtractionContentProfileRef.class, ExtractionRunRef.class + ExtractionContentProfileRef.class ); IncrementalPropositionExtraction.class.getMethod( "rememberFileFromSource", InputStream.class, String.class, NamedEntity.class, SourceLocator.class, - SourceRevisionRef.class, ExtractionContentProfileRef.class, ExtractionRunRef.class + SourceRevisionRef.class, ExtractionContentProfileRef.class ); - // A profile never lands where a legacy caller already puts something else. + // A profile never lands where a legacy caller already fills every argument. assertThrows( NoSuchMethodException.class, () -> IncrementalPropositionExtraction.class.getMethod( @@ -153,14 +132,6 @@ void retainsEveryLegacyRememberDescriptorAndAddsProfileAwareOnes() throws Except ExtractionContentProfileRef.class ) ); - assertThrows( - NoSuchMethodException.class, - () -> IncrementalPropositionExtraction.class.getMethod( - "rememberFile", - InputStream.class, String.class, NamedEntity.class, - ExtractionRunRef.class - ) - ); } @Test @@ -169,13 +140,10 @@ void legacyAndProfileAwareJavaEventSubclassesUseTheBaseConstructor() { LegacyJavaEvent legacy = new LegacyJavaEvent(this, user); assertNull(legacy.profile()); - assertNull(legacy.currentRun()); ExtractionContentProfileRef profile = new ExtractionContentProfileRef("house-style", "v1"); - ExtractionRunRef run = new ExtractionRunRef("run-1"); - ProfileAwareJavaEvent profileAware = new ProfileAwareJavaEvent(this, user, profile, run); + ProfileAwareJavaEvent profileAware = new ProfileAwareJavaEvent(this, user, profile); assertSame(profile, profileAware.profile()); - assertSame(run, profileAware.currentRun()); // A subclass that only knows about profiles still carries no source provenance. assertNull(profileAware.sourceLocator()); assertNull(profileAware.sourceRevision()); @@ -186,17 +154,19 @@ void conversationEventKeepsItsFiveArgumentConstructorAndGainsProfileAwareOnes() ConversationAnalysisRequestEvent.class.getConstructor( Object.class, NamedEntity.class, Conversation.class ); + // The 4-argument form (locator only, no revision) is one `@JvmOverloads` emits from the + // longer constructor's first defaulted parameter — a Java caller that compiled against it + // must keep compiling. ConversationAnalysisRequestEvent.class.getConstructor( - Object.class, NamedEntity.class, Conversation.class, SourceLocator.class, - SourceRevisionRef.class + Object.class, NamedEntity.class, Conversation.class, SourceLocator.class ); ConversationAnalysisRequestEvent.class.getConstructor( Object.class, NamedEntity.class, Conversation.class, SourceLocator.class, - SourceRevisionRef.class, ExtractionContentProfileRef.class + SourceRevisionRef.class ); ConversationAnalysisRequestEvent.class.getConstructor( Object.class, NamedEntity.class, Conversation.class, SourceLocator.class, - SourceRevisionRef.class, ExtractionContentProfileRef.class, ExtractionRunRef.class + SourceRevisionRef.class, ExtractionContentProfileRef.class ); NamedEntity user = org.mockito.Mockito.mock(NamedEntity.class); @@ -204,25 +174,28 @@ void conversationEventKeepsItsFiveArgumentConstructorAndGainsProfileAwareOnes() SourceLocator locator = new ContentAddressedLocator("java-conversation"); SourceRevisionRef revision = new SourceRevisionRef(locator.key(), "r1"); ExtractionContentProfileRef profile = new ExtractionContentProfileRef("house-style", "v1"); - ExtractionRunRef run = new ExtractionRunRef("run-1"); + + ConversationAnalysisRequestEvent locatorOnly = + new ConversationAnalysisRequestEvent(this, user, conversation, locator); + assertSame(locator, locatorOnly.sourceLocator()); + assertNull(locatorOnly.sourceRevision()); + assertNull(locatorOnly.profile()); ConversationAnalysisRequestEvent legacy = new ConversationAnalysisRequestEvent(this, user, conversation, locator, revision); assertSame(locator, legacy.sourceLocator()); assertSame(revision, legacy.sourceRevision()); assertNull(legacy.profile()); - assertNull(legacy.currentRun()); ConversationAnalysisRequestEvent profiled = new ConversationAnalysisRequestEvent( - this, user, conversation, locator, revision, profile, run + this, user, conversation, locator, revision, profile ); assertSame(profile, profiled.profile()); - assertSame(run, profiled.currentRun()); // A profile without a typed source: the locator argument is nullable because the two // dimensions are independent. ConversationAnalysisRequestEvent profileOnly = new ConversationAnalysisRequestEvent( - this, user, conversation, null, null, profile, run + this, user, conversation, null, null, profile ); assertNull(profileOnly.sourceLocator()); assertSame(profile, profileOnly.profile()); @@ -341,21 +314,20 @@ private static void compileJavaSourceCalls( NamedEntity user, SourceLocator locator, SourceRevisionRef revision, - ExtractionContentProfileRef profile, - ExtractionRunRef run + ExtractionContentProfileRef profile ) { extraction.rememberText("legacy", "legacy-id", user); extraction.rememberText("legacy", "legacy-id", user, List.of(), null, null); - extraction.rememberText("profiled", "profiled-id", user, List.of(), null, null, profile, run); + extraction.rememberText("profiled", "profiled-id", user, List.of(), null, null, profile); extraction.rememberFile(input, "legacy.txt", user); - extraction.rememberFile(input, "profiled.txt", user, profile, run); + extraction.rememberFile(input, "profiled.txt", user, profile); extraction.rememberTextFromSource("source", "source-id", user, locator); extraction.rememberTextFromSource( - "source", "source-id", user, locator, revision, List.of(), null, null, profile, run + "source", "source-id", user, locator, revision, List.of(), null, null, profile ); extraction.rememberFileFromSource(input, "source.txt", user, locator); extraction.rememberFileFromSource( - input, "source.txt", user, locator, revision, profile, run + input, "source.txt", user, locator, revision, profile ); } @@ -374,17 +346,14 @@ public IncrementalSource incrementalSource() { private static final class ProfileAwareJavaEvent extends SourceAnalysisRequestEvent { private final ExtractionContentProfileRef profile; - private final ExtractionRunRef run; private ProfileAwareJavaEvent( Object source, NamedEntity user, - ExtractionContentProfileRef profile, - ExtractionRunRef run + ExtractionContentProfileRef profile ) { super(source, user); this.profile = profile; - this.run = run; } @Override @@ -396,10 +365,5 @@ public IncrementalSource incrementalSource() { public ExtractionContentProfileRef profile() { return profile; } - - @Override - public ExtractionRunRef currentRun() { - return run; - } } } diff --git a/dice/src/test/kotlin/com/embabel/dice/ExtractionProfileCompatibilityTest.kt b/dice/src/test/kotlin/com/embabel/dice/ExtractionProfileCompatibilityTest.kt index 815ae2e3..9a2d1638 100644 --- a/dice/src/test/kotlin/com/embabel/dice/ExtractionProfileCompatibilityTest.kt +++ b/dice/src/test/kotlin/com/embabel/dice/ExtractionProfileCompatibilityTest.kt @@ -24,7 +24,6 @@ import com.embabel.dice.common.SourceAnalysisContext import com.embabel.dice.common.resolver.AlwaysCreateEntityResolver import com.embabel.dice.proposition.extraction.ExtractionContentProfileRef import com.embabel.dice.proposition.extraction.ExtractionPerspective -import com.embabel.dice.proposition.extraction.ExtractionRunRef import com.embabel.dice.provenance.ContentAddressedLocator import com.embabel.dice.provenance.SourceLocator import com.embabel.dice.provenance.SourceRevisionRef @@ -44,17 +43,15 @@ class ExtractionProfileCompatibilityTest { private fun context( profile: ExtractionContentProfileRef? = null, - currentRun: ExtractionRunRef? = null, ) = SourceAnalysisContext( schema = DataDictionary.fromClasses("profile-compatibility"), entityResolver = AlwaysCreateEntityResolver, contextId = ContextId("profile-compatibility"), profile = profile, - currentRun = currentRun, ) @Test - fun `legacy Kotlin source constructors and copy calls see no profile and no run`() { + fun `legacy Kotlin source constructors and copy calls see no profile`() { val legacy = SourceAnalysisContext( schema = DataDictionary.fromClasses("profile-compatibility"), entityResolver = AlwaysCreateEntityResolver, @@ -63,31 +60,26 @@ class ExtractionProfileCompatibilityTest { assertEquals(true, legacy.promptVariables["legacy"]) assertNull(legacy.profile) - assertNull(legacy.currentRun) - // The Java-facing builder is unchanged too, and its result carries neither. + // The Java-facing builder is unchanged too, and its result carries none. val built = SourceAnalysisContext .withContextId("profile-compatibility") .withEntityResolver(AlwaysCreateEntityResolver) .withSchema(DataDictionary.fromClasses("profile-compatibility")) assertNull(built.profile) - assertNull(built.currentRun) } @Test - fun `new Kotlin source constructors and copy calls carry a profile and a run`() { + fun `new Kotlin source constructors and copy calls carry a profile`() { val profile = ExtractionContentProfileRef("house-style", "v3") - val run = ExtractionRunRef("run-42") - val fromConstructor = context(profile = profile, currentRun = run) + val fromConstructor = context(profile = profile) .copy(promptVariables = mapOf("profiled" to true)) assertSame(profile, fromConstructor.profile) - assertSame(run, fromConstructor.currentRun) assertEquals(true, fromConstructor.promptVariables["profiled"]) - val fromHelpers = context().withProfile(profile).withCurrentRun(run) + val fromHelpers = context().withProfile(profile) assertSame(profile, fromHelpers.profile) - assertSame(run, fromHelpers.currentRun) } @Test @@ -103,7 +95,6 @@ class ExtractionProfileCompatibilityTest { perspective = ExtractionPerspective.USER, sourceRevision = revision, profile = ExtractionContentProfileRef("house-style", "v3"), - currentRun = ExtractionRunRef("run-42"), ) assertSame(locator, context.sourceLocator) @@ -130,7 +121,6 @@ class ExtractionProfileCompatibilityTest { Map::class.java, SourceRevisionRef::class.java, ExtractionContentProfileRef::class.java, - ExtractionRunRef::class.java, ) val published = SourceAnalysisContext::class.java.constructors .map { it.parameterTypes.toList() } @@ -143,13 +133,11 @@ class ExtractionProfileCompatibilityTest { "constructor of $arity arguments no longer published", ) } - // 12 and 13 are what this slice adds, on the end. - for (arity in 12..13) { - assertTrue( - declared.take(arity) + marker in published, - "constructor of $arity arguments was not published", - ) - } + // 12 is what this slice adds, on the end. + assertTrue( + declared.take(12) + marker in published, + "constructor of 12 arguments was not published", + ) } @Test @@ -159,11 +147,11 @@ class ExtractionProfileCompatibilityTest { val copyArities = SourceAnalysisContext::class.java.declaredMethods .filter { it.name.startsWith("copy") && !it.name.endsWith("\$default") } .map { it.parameterCount } - assertEquals(listOf(13), copyArities) + assertEquals(listOf(12), copyArities) val componentCount = SourceAnalysisContext::class.java.declaredMethods .count { it.name.startsWith("component") } - assertEquals(13, componentCount) + assertEquals(12, componentCount) } @Test diff --git a/dice/src/test/kotlin/com/embabel/dice/common/ExtractionContextIndependenceTest.kt b/dice/src/test/kotlin/com/embabel/dice/common/ExtractionContextIndependenceTest.kt index 5568bedd..d056b430 100644 --- a/dice/src/test/kotlin/com/embabel/dice/common/ExtractionContextIndependenceTest.kt +++ b/dice/src/test/kotlin/com/embabel/dice/common/ExtractionContextIndependenceTest.kt @@ -20,7 +20,6 @@ import com.embabel.agent.core.DataDictionary import com.embabel.dice.common.resolver.AlwaysCreateEntityResolver import com.embabel.dice.proposition.extraction.ExtractionContentProfileRef import com.embabel.dice.proposition.extraction.ExtractionPerspective -import com.embabel.dice.proposition.extraction.ExtractionRunRef import com.embabel.dice.provenance.ContentAddressedLocator import com.embabel.dice.provenance.SourceRevisionRef import org.junit.jupiter.api.Assertions.assertEquals @@ -174,7 +173,6 @@ class ExtractionContextIndependenceTest { mintedEntityProperties = mapOf("owner" to "tenant-one"), sourceRevision = SourceRevisionRef(locator.key(), "r1"), profile = profiles[1], - currentRun = ExtractionRunRef("run-before"), ) // Comparing against copy(...) is a statement about every component at once: the helper @@ -182,9 +180,6 @@ class ExtractionContextIndependenceTest { val profile = ExtractionContentProfileRef("legal-review", "v9") assertEquals(base.copy(profile = profile), base.withProfile(profile)) - val run = ExtractionRunRef("run-after") - assertEquals(base.copy(currentRun = run), base.withCurrentRun(run)) - assertEquals( base.copy(perspective = ExtractionPerspective.ALL), base.withPerspective(ExtractionPerspective.ALL), @@ -229,28 +224,4 @@ class ExtractionContextIndependenceTest { ) } } - - @Test - fun `profile and current run are independent of each other`() { - val profile = profiles[1]!! - val run = ExtractionRunRef("run-7") - - val combinations = listOf( - null to null, - profile to null, - null to run, - profile to run, - ).map { (p, r) -> - SourceAnalysisContext( - schema = schemas[0], - entityResolver = AlwaysCreateEntityResolver, - contextId = tenants[0], - profile = p, - currentRun = r, - ) - } - - assertEquals(listOf(null, profile, null, profile), combinations.map { it.profile }) - assertEquals(listOf(null, null, run, run), combinations.map { it.currentRun }) - } } diff --git a/dice/src/test/kotlin/com/embabel/dice/common/SourceAnalysisRequestEventProfileTest.kt b/dice/src/test/kotlin/com/embabel/dice/common/SourceAnalysisRequestEventProfileTest.kt index 989a7fe9..72ee2a70 100644 --- a/dice/src/test/kotlin/com/embabel/dice/common/SourceAnalysisRequestEventProfileTest.kt +++ b/dice/src/test/kotlin/com/embabel/dice/common/SourceAnalysisRequestEventProfileTest.kt @@ -20,7 +20,6 @@ import com.embabel.chat.Conversation import com.embabel.chat.Message import com.embabel.dice.incremental.IncrementalSource import com.embabel.dice.proposition.extraction.ExtractionContentProfileRef -import com.embabel.dice.proposition.extraction.ExtractionRunRef import com.embabel.dice.provenance.ContentAddressedLocator import com.embabel.dice.provenance.SourceRevisionRef import org.junit.jupiter.api.Assertions.assertEquals @@ -30,27 +29,25 @@ import org.junit.jupiter.api.Test import org.mockito.Mockito.mock /** - * The async publisher's half of the profile contract: both accessors default to null, an - * existing subclass is unaffected, and the shipped conversation event carries a profile and a - * run when its longer constructor is used. + * The async publisher's half of the profile contract: the accessor defaults to null, an + * existing subclass is unaffected, and the shipped conversation event carries a profile when + * its longer constructor is used. */ class SourceAnalysisRequestEventProfileTest { private val profile = ExtractionContentProfileRef("house-style", "v1") - private val run = ExtractionRunRef("run-1") @Test - fun `a subclass written before profiles carries neither`() { + fun `a subclass written before profiles carries none`() { val legacy = LegacyEvent(this, mock(NamedEntity::class.java)) assertNull(legacy.profile()) - assertNull(legacy.currentRun()) assertNull(legacy.sourceLocator()) assertNull(legacy.sourceRevision()) } @Test - fun `the shipped conversation event defaults to no profile and no run`() { + fun `the shipped conversation event defaults to no profile`() { val event = ConversationAnalysisRequestEvent( source = this, user = mock(NamedEntity::class.java), @@ -58,11 +55,10 @@ class SourceAnalysisRequestEventProfileTest { ) assertNull(event.profile()) - assertNull(event.currentRun()) } @Test - fun `the conversation event carries the exact profile and run it was given`() { + fun `the conversation event carries the exact profile it was given`() { val locator = ContentAddressedLocator("event-source") val revision = SourceRevisionRef(locator.key(), "r1") @@ -73,13 +69,11 @@ class SourceAnalysisRequestEventProfileTest { sourceLocator = locator, sourceRevision = revision, profile = profile, - currentRun = run, ) assertSame(locator, event.sourceLocator()) assertSame(revision, event.sourceRevision()) assertSame(profile, event.profile()) - assertSame(run, event.currentRun()) } @Test @@ -95,7 +89,6 @@ class SourceAnalysisRequestEventProfileTest { assertNull(event.sourceLocator()) assertNull(event.sourceRevision()) assertSame(profile, event.profile()) - assertNull(event.currentRun()) } @Test @@ -108,7 +101,6 @@ class SourceAnalysisRequestEventProfileTest { } assertSame(profile, profileOnly.profile()) - assertNull(profileOnly.currentRun()) assertNull(profileOnly.sourceLocator()) } diff --git a/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionProfileContractTest.kt b/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionProfileContractTest.kt index f2d19b1b..20e55e98 100644 --- a/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionProfileContractTest.kt +++ b/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionProfileContractTest.kt @@ -22,8 +22,8 @@ import org.assertj.core.api.Assertions.assertThatIllegalArgumentException import org.junit.jupiter.api.Test /** - * What the two new reference types promise: stable name-and-version identity for a profile, - * an opaque id for a run, bounded strings, and no interpretation of either. + * What the new reference type promises: stable name-and-version identity for a profile, a + * bounded string, and no interpretation of either component. */ class ExtractionProfileContractTest { @@ -87,58 +87,4 @@ class ExtractionProfileContractTest { .isThrownBy { ExtractionContentProfileRef(name, version + "v") } .withMessageContaining("version") } - - @Test - fun `run ref preserves an opaque id`() { - val ref = ExtractionRunRef("01J9Z0V1XQ:host/7#a") - - assertThat(ref.runId).isEqualTo("01J9Z0V1XQ:host/7#a") - assertThat(objectMapper.readValue(objectMapper.writeValueAsString(ref))) - .isEqualTo(ref) - } - - @Test - fun `run identity is the id`() { - val run = ExtractionRunRef("run-1") - val same = ExtractionRunRef("run-1") - val other = ExtractionRunRef("run-2") - - assertThat(run).isEqualTo(same) - assertThat(run.hashCode()).isEqualTo(same.hashCode()) - assertThat(run).isNotEqualTo(other) - } - - @Test - fun `run ref rejects a blank id`() { - assertThatIllegalArgumentException() - .isThrownBy { ExtractionRunRef("") } - .withMessageContaining("runId") - assertThatIllegalArgumentException() - .isThrownBy { ExtractionRunRef(" ") } - .withMessageContaining("runId") - } - - @Test - fun `run ref accepts its length cap and rejects one character more`() { - val runId = "r".repeat(ExtractionRunRef.MAX_RUN_ID_LENGTH) - - assertThat(ExtractionRunRef(runId).runId).hasSize(ExtractionRunRef.MAX_RUN_ID_LENGTH) - assertThatIllegalArgumentException() - .isThrownBy { ExtractionRunRef(runId + "r") } - .withMessageContaining("runId") - } - - @Test - fun `the two references are unrelated types`() { - // Nothing converts one into the other and neither derives from the other. A profile says - // what extraction should do; a run says which execution this was. - assertThat( - ExtractionContentProfileRef::class.java.isAssignableFrom(ExtractionRunRef::class.java), - ).isFalse() - assertThat( - ExtractionRunRef::class.java.isAssignableFrom(ExtractionContentProfileRef::class.java), - ).isFalse() - assertThat(ExtractionContentProfileRef("run-1", "run-1")) - .isNotEqualTo(ExtractionRunRef("run-1")) - } } 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 08436a7b..63509edf 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 @@ -24,6 +24,7 @@ import com.embabel.agent.core.DataDictionary import com.embabel.agent.rag.model.NamedEntity import com.embabel.agent.rag.service.NamedEntityDataRepository import com.embabel.chat.Message +import com.embabel.dice.common.ConversationAnalysisRequestEvent import com.embabel.dice.common.EntityResolver import com.embabel.dice.common.Relations import com.embabel.dice.common.SourceAnalysisContext @@ -83,8 +84,8 @@ class IncrementalPropositionExtractionTest { .map { it.parameterTypes.toList() } .toSet() - // Every descriptor that existed before profiles is still here, and the profile and run - // arguments only ever add one descriptor on the end of each name. + // Every descriptor that existed before profiles is still here, and the profile + // argument only ever adds one descriptor on the end of each name. val legacyTextPrefix = listOf( String::class.java, String::class.java, @@ -95,9 +96,8 @@ class IncrementalPropositionExtractionTest { ExtractionPerspective::class.java, Boolean::class.javaObjectType, ) - val profileAndRun = listOf( + val profileOnly = listOf( ExtractionContentProfileRef::class.java, - ExtractionRunRef::class.java, ) assertEquals( setOf( @@ -105,7 +105,7 @@ class IncrementalPropositionExtractionTest { legacyTextPrefix + List::class.java, legacyTextPrefix + listOf(List::class.java, ExtractionPerspective::class.java), legacyTextFull, - legacyTextFull + profileAndRun, + legacyTextFull + profileOnly, ), rememberTextParameters, ) @@ -128,7 +128,7 @@ class IncrementalPropositionExtractionTest { ExtractionPerspective::class.java, ), sourceTextFull, - sourceTextFull + profileAndRun, + sourceTextFull + profileOnly, ), rememberTextFromSourceParameters, ) @@ -139,7 +139,7 @@ class IncrementalPropositionExtractionTest { NamedEntity::class.java, ) assertEquals( - setOf(legacyFile, legacyFile + profileAndRun), + setOf(legacyFile, legacyFile + profileOnly), rememberFileParameters, ) val sourceFileFull = @@ -148,24 +148,98 @@ class IncrementalPropositionExtractionTest { setOf( legacyFile + SourceLocator::class.java, sourceFileFull, - sourceFileFull + profileAndRun, + sourceFileFull + profileOnly, ), rememberFileFromSourceParameters, ) - // A profile-aware call can never collapse onto a legacy one: a profile and a run always - // arrive together, on the end, and never at an arity a legacy caller already fills. + // A profile-aware call can never collapse onto a legacy one: a profile always arrives + // on the end, at an arity a legacy caller never fills. val everyRememberParameterList = rememberTextParameters + rememberFileParameters + rememberTextFromSourceParameters + rememberFileFromSourceParameters everyRememberParameterList.forEach { parameters -> val hasProfile = ExtractionContentProfileRef::class.java in parameters - val hasRun = ExtractionRunRef::class.java in parameters - assertEquals(hasProfile, hasRun, "profile and run must arrive together: $parameters") if (hasProfile) { assertEquals( - profileAndRun, - parameters.takeLast(2), - "profile and run must be the last two parameters: $parameters", + profileOnly, + parameters.takeLast(1), + "profile must be the last parameter: $parameters", + ) + } + } + } + + @Test + fun `no entry point, context, constructor, or field anywhere carries a run reference`() { + // PR #94 review comment: buildContext accepted a currentRun and put it on the context. + // persistAndProject — the method that actually saves the extracted propositions — takes + // only a ChunkPropositionResult and never the context that would have carried it. No + // consuming write exists on this branch (the durable run store is DICE #67/#98/#99), so + // the fix removes the no-op parameter. + // + // persistAndProject has exactly one overload, and it is the one-argument shape: a second + // overload taking the context (a run reference's only possible route back in) would slip + // past a check that only confirms one particular arity exists. + 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(), + ) + + // The type itself is gone from the classpath, so there is nothing left for a caller to + // depend on — not even a reference to it, let alone a call to a member of it. + assertThrows(ClassNotFoundException::class.java) { + Class.forName("com.embabel.dice.proposition.extraction.ExtractionRunRef") + } + + // A run reference under another name (runRef, runId, AnalysisRunRef, ...) would satisfy a + // sweep that only recognizes "currentRun"/"extractionRun" literally, so this checks every + // declared member's name for "run" as a substring, not a fixed set of spellings. Verified + // before writing this: none of the four carriers has a legitimate declared method, field, + // or constructor parameter whose name contains "run" today, confirmed by a throwaway + // reflection dump run against the built classes, so this sweep starts from a clean + // baseline and any future match is either a reintroduced run reference or something that + // needs an explicit, named exclusion (there are none right now). + // + // Types are checked via the *generic* signature (genericType / genericReturnType / + // genericParameterTypes), not the erased Class. An erased check sees `List` for a field + // declared `List` and would miss it; `Type.toString()` on a generic + // signature includes the type argument, so the same substring match catches a run + // reference hidden inside a collection or other generic wrapper. Confirmed empirically, + // same as the name sweep: zero matches on the current classes. + // + // Constructor parameters are checked by type only, not name. The JVM does not preserve + // real parameter names in these classes' compiled constructors (reflection reports them + // as arg0, arg1, ...), so a name-based check on a constructor parameter would silently + // never fire; claiming otherwise here would be the same overclaim this test exists to + // avoid making about other code. + val carriers = listOf( + IncrementalPropositionExtraction::class.java, + SourceAnalysisContext::class.java, + SourceAnalysisRequestEvent::class.java, + ConversationAnalysisRequestEvent::class.java, + ) + val runInName = Regex("(?i)run") + fun suspectType(type: java.lang.reflect.Type) = runInName.containsMatchIn(type.toString()) + carriers.forEach { type -> + type.declaredMethods.forEach { method -> + assertFalse(runInName.containsMatchIn(method.name), "${type.simpleName}.${method.name}: name mentions run") + assertFalse( + method.genericParameterTypes.any(::suspectType) || suspectType(method.genericReturnType), + "${type.simpleName}.${method.name}: a parameter or return type mentions run", + ) + } + type.declaredFields.forEach { field -> + assertFalse(runInName.containsMatchIn(field.name), "${type.simpleName}.${field.name}: name mentions run") + assertFalse(suspectType(field.genericType), "${type.simpleName}.${field.name}: field type mentions run") + } + type.declaredConstructors.forEach { constructor -> + assertFalse( + constructor.genericParameterTypes.any(::suspectType), + "${type.simpleName} constructor $constructor: a parameter type mentions run", ) } } @@ -173,12 +247,12 @@ class IncrementalPropositionExtractionTest { @Test fun `the entry point signatures that were overridable before profiles still are`() { - // @JvmOverloads emits every reduced-arity overload as final. Folding the two new - // arguments into the existing declarations would therefore have turned each method's - // pre-change maximum arity — the signature a subclass overrides — into a final bridge. - // Callers would not have noticed; a subclass would have stopped compiling, and one - // already compiled could fail verification at load. Each shape is its own declaration - // instead, and this is the assertion that keeps it that way. + // @JvmOverloads emits every reduced-arity overload as final. Folding the new argument + // into the existing declarations would therefore have turned each method's pre-change + // maximum arity — the signature a subclass overrides — into a final bridge. Callers + // would not have noticed; a subclass would have stopped compiling, and one already + // compiled could fail verification at load. Each shape is its own declaration instead, + // and this is the assertion that keeps it that way. val stillOpen = mapOf( "rememberText" to listOf( String::class.java, @@ -222,13 +296,12 @@ class IncrementalPropositionExtractionTest { // The new maximum-arity forms are the single override point every call funnels through, // so they have to be open too. - val profileAndRun = arrayOf( + val profileOnly = arrayOf( ExtractionContentProfileRef::class.java, - ExtractionRunRef::class.java, ) stillOpen.forEach { (name, parameters) -> val method = IncrementalPropositionExtraction::class.java - .getMethod(name, *(parameters.toTypedArray() + profileAndRun)) + .getMethod(name, *(parameters.toTypedArray() + profileOnly)) assertFalse( Modifier.isFinal(method.modifiers), "the profile-aware $name must be overridable", @@ -364,13 +437,12 @@ class IncrementalPropositionExtractionTest { "legacy.txt", user(), ) - // A wide call carrying no references dispatches like the pre-profile call it resembles. + // A wide call carrying no reference dispatches like the pre-profile call it resembles. extraction.rememberFile( ByteArrayInputStream("legacy file text".toByteArray()), "wide-null.txt", user(), null, - null, ) extraction.rememberFileFromSource( ByteArrayInputStream("source file text".toByteArray()), @@ -386,7 +458,6 @@ class IncrementalPropositionExtractionTest { locator, revision, null, - null, ) assertEquals( @@ -437,9 +508,8 @@ class IncrementalPropositionExtractionTest { perspective: ExtractionPerspective?, mintNewEntities: Boolean?, profile: ExtractionContentProfileRef?, - currentRun: ExtractionRunRef?, ) { - seen += "wide:$sourceId:${profile?.name}:${currentRun?.runId}" + seen += "wide:$sourceId:${profile?.name}" } } @@ -448,10 +518,9 @@ class IncrementalPropositionExtractionTest { "profiled.txt", user(), ExtractionContentProfileRef("house-style", "v1"), - ExtractionRunRef("run-1"), ) - assertEquals(listOf("wide:remember:profiled.txt:house-style:run-1"), seen) + assertEquals(listOf("wide:remember:profiled.txt:house-style"), seen) verifyNoInteractions(pipeline) } @@ -684,7 +753,6 @@ class IncrementalPropositionExtractionTest { anyOrNull(), anyOrNull(), anyOrNull(), - anyOrNull(), ) doNothing().whenever(extraction).rememberTextFromSource( any(), @@ -696,7 +764,6 @@ class IncrementalPropositionExtractionTest { anyOrNull(), anyOrNull(), anyOrNull(), - anyOrNull(), ) extraction.rememberFile( @@ -722,7 +789,6 @@ class IncrementalPropositionExtractionTest { null, null, null, - null, ) verify(extraction).rememberTextFromSource( "source dispatch", @@ -734,7 +800,6 @@ class IncrementalPropositionExtractionTest { null, null, null, - null, ) verifyNoInteractions(pipeline) } @@ -822,14 +887,13 @@ class IncrementalPropositionExtractionTest { } @Test - fun `profile and run reach the context through both text entry points`() { + fun `profile reaches the context through both text entry points`() { val pipeline = pipelineReturningNoResult() val extraction = extraction(pipeline) val user = user() val locator = UriLocator("https://example.com/profiled") val revision = SourceRevisionRef(locator.key(), "r1") val profile = ExtractionContentProfileRef("house-style", "v1") - val run = ExtractionRunRef("run-1") extraction.rememberText( text = "legacy text", @@ -839,7 +903,6 @@ class IncrementalPropositionExtractionTest { perspective = null, mintNewEntities = null, profile = profile, - currentRun = run, ) extraction.rememberTextFromSource( text = "source text", @@ -851,35 +914,30 @@ class IncrementalPropositionExtractionTest { perspective = null, mintNewEntities = null, profile = profile, - currentRun = run, ) val fromLegacy = capturedContext(pipeline, "legacy text", "legacy:profiled", emptyList()) assertSame(profile, fromLegacy.profile) - assertSame(run, fromLegacy.currentRun) assertNull(fromLegacy.sourceLocator) val fromSource = capturedContext(pipeline, "source text", "source:profiled", emptyList()) assertSame(profile, fromSource.profile) - assertSame(run, fromSource.currentRun) assertSame(revision, fromSource.sourceRevision) } @Test - fun `profile and run reach the context through both file entry points`() { + fun `profile reaches the context through both file entry points`() { val pipeline = pipelineReturningNoResult() val extraction = extraction(pipeline) val user = user() val locator = UriLocator("file:///notes/profiled.txt") val profile = ExtractionContentProfileRef("house-style", "v1") - val run = ExtractionRunRef("run-1") extraction.rememberFile( inputStream = ByteArrayInputStream("legacy file text".toByteArray()), filename = "legacy-profiled.txt", user = user, profile = profile, - currentRun = run, ) extraction.rememberFileFromSource( inputStream = ByteArrayInputStream("source file text".toByteArray()), @@ -888,7 +946,6 @@ class IncrementalPropositionExtractionTest { sourceLocator = locator, sourceRevision = SourceRevisionRef(locator.key(), "r1"), profile = profile, - currentRun = run, ) // Both file calls land on the profile-aware text entry point, so the two contexts the // pipeline sees are the proof that carriage survives the file hop. @@ -904,12 +961,11 @@ class IncrementalPropositionExtractionTest { ) contextCaptor.allValues.forEach { context -> assertSame(profile, context.profile) - assertSame(run, context.currentRun) } } @Test - fun `legacy calls carry no profile and no run`() { + fun `legacy calls carry no profile`() { val pipeline = pipelineReturningNoResult() val extraction = extraction(pipeline) val user = user() @@ -932,7 +988,6 @@ class IncrementalPropositionExtractionTest { ) contextCaptor.allValues.forEach { context -> assertNull(context.profile) - assertNull(context.currentRun) } } @@ -943,7 +998,6 @@ class IncrementalPropositionExtractionTest { val user = user() val grounding = listOf("record:one") val profile = ExtractionContentProfileRef("house-style", "v1") - val run = ExtractionRunRef("run-1") extraction.rememberText( text = "same text", @@ -961,7 +1015,6 @@ class IncrementalPropositionExtractionTest { perspective = ExtractionPerspective.USER, mintNewEntities = true, profile = profile, - currentRun = run, ) val contextCaptor = argumentCaptor() @@ -975,48 +1028,40 @@ class IncrementalPropositionExtractionTest { ) val (plain, profiled) = contextCaptor.allValues - // Comparing whole contexts is the point: they agree on every component but the two the + // Comparing whole contexts is the point: they agree on every component but the one the // second call set. The resolver is substituted because buildContext constructs a fresh // one per call by design, so it is never the same instance twice. assertEquals( plain, - profiled.copy(profile = null, currentRun = null, entityResolver = plain.entityResolver), + profiled.copy(profile = null, entityResolver = plain.entityResolver), ) assertSame(profile, profiled.profile) - assertSame(run, profiled.currentRun) } @Test - fun `event profile and run reach the context observed by the pipeline`() { + fun `event profile reaches the context observed by the pipeline`() { val pipeline = pipelineReturningNoResult() val extraction = extraction(pipeline) val source = mock>() whenever(source.id).thenReturn("event-source") whenever(source.size).thenReturn(1) val profile = ExtractionContentProfileRef("house-style", "v1") - val run = ExtractionRunRef("run-1") val profileCalls = AtomicInteger() - val runCalls = AtomicInteger() val event = object : SourceAnalysisRequestEvent(this, user()) { override fun incrementalSource(): IncrementalSource = source override fun profile(): ExtractionContentProfileRef = profile.also { profileCalls.incrementAndGet() } - - override fun currentRun(): ExtractionRunRef = - run.also { runCalls.incrementAndGet() } } extraction.extractPropositions(event) - // One read each: the async path builds one context through the same buildContext the - // direct calls use, so there is nowhere else for a second read to happen. + // One read: the async path builds one context through the same buildContext the direct + // calls use, so there is nowhere else for a second read to happen. assertEquals(1, profileCalls.get()) - assertEquals(1, runCalls.get()) val contextCaptor = argumentCaptor() verify(pipeline).processChunk(any(), contextCaptor.capture()) assertSame(profile, contextCaptor.firstValue.profile) - assertSame(run, contextCaptor.firstValue.currentRun) assertNull(contextCaptor.firstValue.sourceLocator) } diff --git a/docs/design/INDEX.md b/docs/design/INDEX.md index 9478abe8..32f99796 100644 --- a/docs/design/INDEX.md +++ b/docs/design/INDEX.md @@ -22,9 +22,9 @@ you need. matched to existing entities (or minted as new ones) without blowing the LLM budget or fragmenting the graph with near-duplicates. - [extraction-profiles.md](extraction-profiles.md) — carrying a host's content-policy identity - and a run reference through extraction without DICE resolving either: why profile identity is - opaque, why the run reference ships ahead of the run, and why profile, perspective, schema and - tenant stay independent. EXPERIMENTAL. + through extraction without DICE resolving it: why profile identity is opaque, why profile, + perspective, schema and tenant stay independent, and why an extraction run reference shipped in + review and was pulled back out until its consuming write exists. EXPERIMENTAL. ## Propositions & lifecycle diff --git a/docs/design/extraction-profiles.md b/docs/design/extraction-profiles.md index b165af76..f656fd46 100644 --- a/docs/design/extraction-profiles.md +++ b/docs/design/extraction-profiles.md @@ -5,10 +5,11 @@ kind of material do?" DICE carries its name and version and nothing else. It nev profile up, never reads policy out of it, and never routes on it. The host owns the catalog, authorizes who may use which profile, and binds it to whatever it actually means. -This note covers DICE #66: the two reference types (`ExtractionContentProfileRef` and -`ExtractionRunRef`), where they sit on `SourceAnalysisContext` and the extraction entry points, -and why profile, perspective, schema and tenant are four independent dimensions rather than one -knob with four names. +This note covers DICE #66: `ExtractionContentProfileRef`, where it sits on +`SourceAnalysisContext` and the extraction entry points, and why profile, perspective, schema and +tenant are four independent dimensions rather than one knob with four names. An extraction run +reference travelled with this slice for one PR round and was pulled back out on review — see +[Why there is no run reference here](#why-there-is-no-run-reference-here) below. ## Why the identity is opaque @@ -21,9 +22,8 @@ data class ExtractionContentProfileRef( ) ``` -Neither is parsed. DICE compares them, carries them, and stores them (from #67 onward). The -alternative — DICE resolving a profile into an actual configuration — costs three things it has -no business owning: +Neither is parsed. DICE compares them and carries them. The alternative — DICE resolving a +profile into an actual configuration — costs three things it has no business owning: - **A catalog.** Someone would have to hold the mapping from profile to policy, and it would have to be reachable from inside extraction. Every host already has that mapping; DICE would be @@ -36,54 +36,20 @@ no business owning: So the split is: **DICE carries profile identity; the host authorizes and binds it.** A host that wants a particular prompt, a particular temperature, or a particular model for a given profile -makes that decision on its own side, before it calls DICE, and passes the reference along so the -resulting claims can be attributed to the policy they were extracted under. +makes that decision on its own side, before it calls DICE, and passes the reference along so its +own extractor can see which policy the call was made under. **A profile selects no provider, no model, and no credential.** That is a contract, not an oversight, and it is stated in the KDoc of every surface that accepts one. Nothing downstream of -`buildContext` reads either reference; a test compares the whole context built with a profile -against the one built without and asserts they differ in exactly that field. +`buildContext` reads it; a test compares the whole context built with a profile against the one +built without and asserts they differ in exactly that field. Identity is name *and* version together. A host that republishes `house-style` as `v2` gets a -distinct reference, so runs attributed to `v1` stay attributed to `v1` and a later comparison of -the two is a comparison of two identities rather than an archaeology problem. +reference distinct from `v1`, so a later comparison of the two compares two identities and stays +clear of archaeology. -Both strings are bounded — 256 characters for a name, 64 for a version — because #67 stores them -in run headers and indexes them. A reference is an identifier the host mints, not a place to put -a payload. - -## Why the run reference ships before the run - -`ExtractionRunRef` is a single opaque id and nothing else: no timing, no status, no counts, no -lineage. - -```kotlin -data class ExtractionRunRef( - val runId: String, -) -``` - -Durable extraction runs are DICE #67, a whole wave behind this one. The run model will be keyed -by (`ContextId`, `ExtractionRunRef`). Shipping the reference now means the entry points and the -run model meet at an opaque string rather than at a type one of them has to import from the -other's release: this slice can carry a run through extraction while #67 is still being designed, -and #67 can key its store on a type that already exists and is already tested. - -Carrying a reference is always allowed, and DICE does not check that the run exists — there is -nowhere yet to check against. What a store does with a reference to a run it has never seen is -#67's decision; this slice makes no promise about it in either direction, so #67 stays free to -require referential integrity when it wires proposition-to-run attribution. DICE mints no run ids -here either: a run is something the host starts, or, from #67's wiring slice, something DICE's own -run coordinator starts. - -Two properties the reference commits to now, because they are cheaper to hold than to retrofit: - -- **It is not part of source-provenance equality.** Two runs over the same material still produce - one piece of source evidence. What differs is which runs are attributed to it, which is a - separate relation in #67 rather than a field on the evidence. -- **It is a pseudonymous token.** Not an authorization token, not a direct identifier, and not - something a reader can dereference into personal data. Length-capped at 256 characters for the - same reason the profile strings are. +Both strings are bounded — 256 characters for a name, 64 for a version. A reference is an +identifier the host mints, not a place to put a payload. ## Four independent dimensions @@ -99,7 +65,6 @@ flowchart TD CTX --> E["perspective
ExtractionPerspective?
whose statements to mine"] CTX --> S["schema
DataDictionary
what types exist"] CTX --> T["contextId
ContextId
which tenant owns the result"] - CTX --> R["currentRun
ExtractionRunRef?
which execution this was"] CTX -.-> L["sourceLocator + sourceRevision
what is being read"] N["the one coupling on the context:
a revision requires a locator
whose key it matches"] L --- N @@ -107,8 +72,8 @@ flowchart TD The dotted branch is the only coupling on the context: a `sourceRevision` requires a `sourceLocator` whose key it matches, because a revision names a version of a specific source. -Profile and run are checked against nothing, and the `init` block says so — validating them -against another dimension would invent a relationship the contract does not have. +Profile is checked against nothing — the `init` block says so — validating it against another +dimension would invent a relationship the contract does not have. Independence is pinned as a matrix rather than argued. `ExtractionContextIndependenceTest` builds every combination of 4 profiles (including absent) × 4 perspectives (including absent) × 2 schemas @@ -119,8 +84,8 @@ every combination of 4 profiles (including absent) × 4 perspectives (including dimension that quietly disabled, defaulted or rejected another would leave a hole; 3. varying one dimension leaves the other three identical. -A fourth case pins the copy helpers by comparing `withProfile(p)` against `copy(profile = p)`, -which is a statement about all thirteen components at once rather than about the one field the +A fourth case pins the copy helper by comparing `withProfile(p)` against `copy(profile = p)`, +which is a statement about all twelve components at once rather than about the one field the helper names. ## Carrying it in @@ -132,19 +97,19 @@ for the same reason: parallel code in two places is where two paths start behavi ```mermaid flowchart TD A["rememberText / rememberTextFromSource
rememberFile / rememberFileFromSource"] --> C - B["SourceAnalysisRequestEvent.profile() / currentRun()"] --> C + B["SourceAnalysisRequestEvent.profile()"] --> C C["IncrementalPropositionExtraction.buildContext"] --> CTX - CTX["SourceAnalysisContext.profile
SourceAnalysisContext.currentRun"] --> X["carried, never consulted"] + CTX["SourceAnalysisContext.profile"] --> X["carried, never consulted"] ``` | Path | How a profile arrives | | --- | --- | -| `rememberText`, `rememberTextFromSource` | trailing optional `profile` and `currentRun` arguments | -| `rememberFile`, `rememberFileFromSource` | the same two arguments, forwarded to the text call | -| async `SourceAnalysisRequestEvent` | `profile()` / `currentRun()`, both open and null-defaulted | +| `rememberText`, `rememberTextFromSource` | a trailing optional `profile` argument | +| `rememberFile`, `rememberFileFromSource` | the same argument, forwarded to the text call | +| async `SourceAnalysisRequestEvent` | `profile()`, open and null-defaulted | | `ConversationAnalysisRequestEvent` | its longer constructor, whose `sourceLocator` is nullable so a publisher can name a profile for material it has no typed source for | -The direct calls take extra arguments rather than getting their own method names, which is the +The direct calls take an extra argument rather than getting their own method name, which is the opposite of what Wave A did for `rememberTextFromSource`. The reason is the difference in the contracts: a locator is *required* by the source-aware calls, so those are genuinely different methods and separate names keep every call site unambiguous. A profile is optional everywhere, so @@ -152,26 +117,26 @@ an extra name would buy nothing and double the surface. ### Why each entry point is two declarations -Growing the existing declarations with two defaulted parameters would have been the obvious -shape, and it is wrong. `@JvmOverloads` emits every reduced-arity overload as `final`, even on an -`open` function — only the declared maximum arity stays open. So adding `profile` and `currentRun` -to `rememberText` would have moved the open declaration from six arguments to eight and re-emitted -the six-argument form as a final bridge. Callers would not have noticed. A subclass overriding the -six-argument form would have stopped compiling, and one already compiled could fail verification -at class load. +Growing the existing declarations with a defaulted parameter would have been the obvious shape, +and it is wrong. `@JvmOverloads` emits every reduced-arity overload as `final`, even on an `open` +function — only the declared maximum arity stays open. So adding `profile` to `rememberText` +would have moved the open declaration from six arguments to seven and re-emitted the six-argument +form as a final bridge. Callers would not have noticed. A subclass overriding the six-argument +form would have stopped compiling, and one already compiled could fail verification at class +load. Each entry point is therefore two declarations: the pre-profile signature exactly as it was (`@JvmOverloads` where it already had it), delegating to a new maximum-arity form that takes the -two references and carries no defaults. The defaults are what force the split — two overloads that -both supply defaults for the same arity are ambiguous at a Kotlin call site, so the wide form -spells every argument out. +reference and carries no default. The default is what forces the split — two overloads that both +supply defaults for the same arity are ambiguous at a Kotlin call site, so the wide form spells +every argument out. | Method | Overridable before | Overridable now | | --- | --- | --- | -| `rememberText` | 6 args | 6 args, and 8 | -| `rememberTextFromSource` | 8 args | 8 args, and 10 | -| `rememberFile` | 3 args | 3 args, and 5 | -| `rememberFileFromSource` | 5 args | 5 args, and 7 | +| `rememberText` | 6 args | 6 args, and 7 | +| `rememberTextFromSource` | 8 args | 8 args, and 9 | +| `rememberFile` | 3 args | 3 args, and 4 | +| `rememberFileFromSource` | 5 args | 5 args, and 6 | The reduced arities `@JvmOverloads` generates were final before and still are — the fix restores the previous surface rather than widening it. @@ -184,16 +149,16 @@ Before profiles, `rememberFile` read the file and handed the text to the six-arg Routing the file paths straight to the wide text methods would have taken that away silently: the override would still compile, still fire for direct text calls, and stop seeing files. -So the rule is that a call dispatches like the call it resembles. A file call carrying no -references takes the pre-profile chain; one carrying a reference has to go wide, because the -legacy text signature cannot express a profile. +So the rule is that a call dispatches like the call it resembles. A file call carrying no profile +takes the pre-profile chain; one carrying a profile has to go wide, because the legacy text +signature cannot express it. ```mermaid flowchart LR F3["rememberFile(3)"] --> T6 - F5["rememberFile(5)"] -->|"both refs null"| F3 - F5 -->|"a ref is present"| T8 - T6["rememberText(6)"] --> T8["rememberText(8)"] --> I["rememberTextInternal"] + F4["rememberFile(4)"] -->|"profile null"| F3 + F4 -->|"profile present"| T7 + T6["rememberText(6)"] --> T7["rememberText(7)"] --> I["rememberTextInternal"] ``` `rememberFileFromSource` and `rememberTextFromSource` mirror it at their own arities. The wide @@ -204,7 +169,7 @@ Two consequences worth stating. A subclass overriding only a pre-profile text me everything it used to, files included. And unintercepted, every call still ends at the wide text form, so a host that wants one place to see all traffic overrides that. -The async path reads each accessor exactly once, which a test asserts by counting: there is one +The async path reads the accessor exactly once, which a test asserts by counting: there is one `buildContext` call and nowhere else for a second read to happen. ## Compatibility @@ -212,13 +177,13 @@ The async path reads each accessor exactly once, which a test asserts by countin **Additive, with the same scoped ABI boundary Wave A declared.** - Every Java-visible `SourceAnalysisContext` constructor descriptor that existed before this slice - is still published. `@JvmOverloads` adds the two new ones on the end. A test enumerates arities - 3 through 13 (each with the trailing `DefaultConstructorMarker` Kotlin emits because `contextId` + is still published. `@JvmOverloads` adds one new one on the end. A test enumerates arities + 3 through 12 (each with the trailing `DefaultConstructorMarker` Kotlin emits because `contextId` is a value class) and asserts every one resolves. - Every `rememberText`, `rememberTextFromSource`, `rememberFile` and `rememberFileFromSource` - descriptor survives, and the new arguments add exactly one descriptor per method name, on the - end. A test asserts the exact descriptor set of all four names, that a profile and a run always - arrive together, and that they are always the last two parameters. + descriptor survives, and the new argument adds exactly one descriptor per method name, on the + end. A test asserts the exact descriptor set of all four names, and that a profile is always the + last parameter. - **Subclass-override compatibility is claimed, and covers being reached as well as being overridable.** Every signature that was overridable before this slice still is, and every pre-profile call still dispatches through it. A reflection test asserts `Modifier.isFinal` is @@ -230,39 +195,82 @@ The async path reads each accessor exactly once, which a test asserts by countin call still reaches the override. Overridability alone is not enough, so two further tests pin the dispatch: a subclass overriding only the pre-profile *text* methods still sees both file entry points, and a file call that carries a profile goes to the wide text form instead. -- `ConversationAnalysisRequestEvent` keeps its five-argument constructor and gains six- and - seven-argument forms. Its `sourceLocator` parameter relaxes from non-null to nullable, which +- `ConversationAnalysisRequestEvent` keeps its five-argument constructor and gains a + six-argument form. Its `sourceLocator` parameter relaxes from non-null to nullable, which accepts strictly more calls than before. -- **Full Kotlin synthetic `copy` and `componentN` ABI is not claimed.** Adding two fields to a - data class rewrites `copy` and adds two `componentN` methods, so Kotlin code compiled against an - earlier jar must be recompiled rather than swapped in. This is the same half of the boundary - #64 declined, for the same mechanical reason, and a test pins it: exactly one `copy` remains and - it takes thirteen arguments. -- No stored data changes. Nothing serializes a profile or a run reference yet; #67 is where they - reach a store. +- **Full Kotlin synthetic `copy` and `componentN` ABI is not claimed.** Adding a field to a data + class rewrites `copy` and adds a `componentN` method, so Kotlin code compiled against an earlier + jar must be recompiled rather than swapped in. This is the same half of the boundary #64 + declined, for the same mechanical reason, and a test pins it: exactly one `copy` remains and it + takes twelve arguments. +- No stored data changes. Nothing serializes a profile yet. ## Status: EXPERIMENTAL -`ExtractionContentProfileRef` and `ExtractionRunRef` both carry `@ApiStatus.Experimental`, which -is the marker DICE already uses for API that may still move (`PrologProjector`, -`PropositionStatus.STALE`). The KDoc on both types and on every new parameter says the same in -words, and the CHANGELOG entry is labelled. - -The shape most likely to move is `ExtractionRunRef`, because #67 is where a run acquires -everything else about itself and may want the reference to carry more than one string. +`ExtractionContentProfileRef` carries `@ApiStatus.Experimental`, which is the marker DICE already +uses for API that may still move (`PrologProjector`, `PropositionStatus.STALE`). The KDoc on the +type and on every new parameter says the same in words, and the CHANGELOG entry is labelled. A Kotlin `@RequiresOptIn` annotation would make the experimental status enforceable at the call site rather than advisory. Nothing in DICE defines one today, and inventing an opt-in marker is a policy decision about the whole public surface, not about this slice. It is recorded here as an open question rather than answered. +## Why there is no run reference here + +The PR that shipped this slice also carried `ExtractionRunRef` — a single opaque id, identity +only, meant to ship ahead of the durable run store that would key on it (DICE #67). Review on the +PR (#94) named the reason that does not work as a standalone slice: `persistAndProject`, the +method that actually saves what extraction produces, takes only the pipeline's +`ChunkPropositionResult` — it never receives the `SourceAnalysisContext` that a run reference +would have been carried on. So a caller passing `currentRun` got it accepted onto the context and +then structurally unable to reach the write that saves propositions. + +Structurally, `profile` sat on that same context and was equally unreachable from +`persistAndProject`, and `currentRun` reached exactly as far as `profile` still does: both passed +through the identical extension point, `PropositionExtractor.extract(chunk, context)`. +`PropositionPipeline.withExtractor` seeds a pipeline with a host-implemented `PropositionExtractor`, +and `extract` receives the whole context — `currentRun` included, back when it existed. No +extractor DICE ships reads `context.profile` today, and nothing else downstream of `buildContext` +does either (`IncrementalPropositionExtraction.kt:612`); the test `profile reaches the context +through both text entry points` (and its file-entry counterpart) pins that the context reaching +the pipeline carries `profile`, which is the reachability half of the claim, and stops there — it +does not exercise any reading of the value. The asymmetry is about what a host-authored +`PropositionExtractor` has to act on once it does read the field. `profile` names the host's own +content-policy identity, something a host already knows the meaning of and can build a reader for +today. `currentRun` named a run, and nothing in the codebase — not DICE, not a host extractor — +had a run store to look that id up against; a reader for it would have had a value with nowhere to +resolve until DICE #67 supplies one. The gap that keeps `currentRun` out of this slice is that +missing store, exactly the gap `persistAndProject` also runs into. + +The comment offered two honest resolutions: wire a consuming write into this slice, or pull the +parameter until one exists. The first was not available here — no proposition-lineage write that +stamps run attribution exists on this branch; the durable run store and the write that would +consume the reference are DICE #67 and the run-model slices above it +(`extraction-run-model`, `-store-contract`, `-store`, `-lineage`), none of which this branch has. +So the parameter was pulled: `ExtractionRunRef`, `SourceAnalysisContext.currentRun`, +`withCurrentRun`, the `currentRun` argument on every `remember*` entry point, +`SourceAnalysisRequestEvent.currentRun()`, and the matching `ConversationAnalysisRequestEvent` +constructor argument are all removed from this slice. Every table, diagram and arity count above +in this note reflects a profile-only surface for that reason. + +Nothing on this branch called the parameter for anything (`grep` across this worktree found no +caller of `currentRun`/`ExtractionRunRef` outside the profile slice's own code and tests), so +there is nothing to migrate. The reference returns, unchanged in shape, once #67's store and a +write that consumes it land together — at that point a run reference reaching +`SourceAnalysisContext` will have somewhere to go. + ## What this slice does not do - **No host profile catalog.** DICE holds no mapping from a reference to a policy and offers no place to put one. -- **No durable run.** `ExtractionRunRef` names a run; nothing stores one until #67. +- **No extraction run reference.** See [Why there is no run reference here](#why-there-is-no-run-reference-here) — + it shipped in review and was pulled back out; it returns with DICE #67's store and the write + that consumes it. - **No REST surface.** `POST /extract` takes no profile. The REST request carries a source locator and a revision from Wave A and is otherwise unchanged; adding profile there is a separate decision about the authorized boundary, not a mechanical extension of this slice. -- **No behaviour.** Extraction, resolution and revision ordering are behaviour-identical. A - profile changes what a run is *attributed to*, not what it does. +- **No behaviour from DICE's own code.** Extraction, resolution and revision ordering are + behaviour-identical for every extractor DICE ships, with or without a profile. DICE's part ends + at carrying `context.profile` to whichever `PropositionExtractor` a host supplies; a host + extractor that reads the field is free to change what it does based on it. From 08a69e8f506117ed26538d2d94d264c8a0438e6d Mon Sep 17 00:00:00 2001 From: James Dunnam <7660553+jimador@users.noreply.github.com> Date: Wed, 2 Sep 2026 00:09:09 -0400 Subject: [PATCH 3/3] Introduce ExtractionRequest as the entry-point surface The remember entry points were growing one parameter per feature: the train had added wide overloads carrying revision and profile, and the next slices wanted run, products and pass index. ExtractionRequest carries locator, source revision and extraction profile behind one validated value, each entry point gains a single request overload, and the wide overloads are deleted before ever shipping. The signatures main publishes stay byte-identical, pinned by a compile-level test, and the run reference lands as a request field on the next slice with no signature change. Signed-off-by: James Dunnam <7660553+jimador@users.noreply.github.com> --- CHANGELOG.md | 104 ++-- .../dice/common/SourceAnalysisRequestEvent.kt | 8 +- .../extraction/ExtractionRequest.kt | 107 ++++ .../IncrementalPropositionExtraction.kt | 255 ++-------- .../ExtractionProfileJavaInteropTest.java | 119 ++--- .../dice/SourceRevisionJavaInteropTest.java | 105 ++-- .../extraction/ExtractionRequestTest.kt | 153 ++++++ .../IncrementalPropositionExtractionTest.kt | 474 +++++++++--------- .../extraction/PreRequestEntryPointPinTest.kt | 173 +++++++ docs/design/extraction-profiles.md | 137 ++--- docs/design/source-revisions.md | 40 +- 11 files changed, 1005 insertions(+), 670 deletions(-) create mode 100644 dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRequest.kt create mode 100644 dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRequestTest.kt create mode 100644 dice/src/test/kotlin/com/embabel/dice/proposition/extraction/PreRequestEntryPointPinTest.kt diff --git a/CHANGELOG.md b/CHANGELOG.md index e3239cd8..345e7df3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -989,14 +989,13 @@ and the consumer PRs that deliver it). `sourceKey` to equal that locator's key. One check therefore covers every caller, and the content-hash locator the pipeline falls back to when no locator was supplied can never acquire a revision. `PropositionPipeline` stamps the revision onto each `ProvenanceEntry` it writes. - `IncrementalPropositionExtraction` gains `rememberTextFromSource` and `rememberFileFromSource` - beside the existing `rememberText` and `rememberFile`. They are separate methods rather than more - optional arguments, because a locator is required on the source-aware pair and absent from the - legacy pair, which keeps every Kotlin and Java call site resolving to exactly one of them; a test - enumerates the JVM descriptors of all four names and asserts the sets. Passing a revision on either - call asserts that the locator's revision covers the whole text or the whole file as extracted — - DICE cannot derive that from an untyped `sourceId` or from `additionalGrounding`, and the KDoc says - so. `SourceAnalysisRequestEvent` gains `sourceLocator()` and `sourceRevision()`, both open and both + `IncrementalPropositionExtraction` takes a locator and a revision on an `ExtractionRequest`, + described in the extraction-profiles entry below: `rememberText` and `rememberFile` each gain one + overload taking it, and the two signatures they already had are untouched. Supplying a revision + asserts that the locator's revision covers the whole text or the whole file as extracted — DICE + cannot derive that from an untyped `sourceId` or from `additionalGrounding`, and the KDoc says + so. The request refuses a revision with no locator, or one naming a different source key, while + it is being built, so a mismatched pair never reaches an entry point at all. `SourceAnalysisRequestEvent` gains `sourceLocator()` and `sourceRevision()`, both open and both returning null, and `ConversationAnalysisRequestEvent` gains a constructor that takes a locator and an optional revision; the listener feeds both into the same `buildContext` call the direct entry points use, so the async path grounds propositions identically and a test captures the context the @@ -1040,8 +1039,7 @@ and the consumer PRs that deliver it). and Java constructor-descriptor compatibility are claimed. `@JvmOverloads` on `SourceAnalysisContext` preserves every concrete Java constructor descriptor and adds one on the end, and the same holds for `ExtractRequest`, `ProvenanceEntryDto`, and `LineageDto`; `rememberText` and `rememberFile` keep - every descriptor they had, because the revision-aware calls are separate method names rather than - extra parameters. Full Kotlin synthetic constructor and `copy` ABI is **not** claimed for + every descriptor they had, and each gains exactly one more, taking an `ExtractionRequest`. Full Kotlin synthetic constructor and `copy` ABI is **not** claimed for `SourceAnalysisContext`: adding a field to a data class changes `copy`/`componentN` and the synthetic `$default` constructor, so Kotlin code compiled against an earlier jar must be recompiled rather than swapped in. `SourceRevisionBinaryCompatibilityTest` runs the pinned legacy client and @@ -1079,8 +1077,26 @@ and the consumer PRs that deliver it). No stored data migrates: `sourceRevision` stays absent from the JSON of a revisionless entry, which is byte-identical to what was written before. -- **EXPERIMENTAL.** Versioned extraction content profiles carried through the extraction entry - points (DICE #66). `ExtractionContentProfileRef(name, version)` names a version of a host's +- **EXPERIMENTAL.** One request object for the extraction entry points, carrying source provenance + and versioned extraction content profiles (DICE #66). `ExtractionRequest` holds what a caller + wants to say about one extraction on top of the text and the user it belongs to: the source it + was read from, the revision of that source, and the content profile it runs under. + `rememberText` and `rememberFile` each gain exactly one overload taking it — + `rememberText(text, sourceId, user, additionalGrounding, perspective, mintNewEntities, request)` + and `rememberFile(inputStream, filename, user, request)` — and the signatures those two methods + already had are untouched, so every call and every override written against them keeps working. + The next dimension extraction learns about arrives as a field on the request while the + entry-point signatures stay where they are, so a host that overrides one keeps compiling and + sees the new value without touching its override. A request checks its own coupling as it is + built: a `sourceRevision` requires a `sourceLocator` whose key it matches, because a revision + names a version of one specific source. A mismatched pair is refused there, before any entry + point is called, so a caller can never reach extraction holding one. A profile is checked + against nothing, because it is independent of where the material came from. + `ExtractionRequest.NONE` is the empty request; a file call carrying it dispatches exactly as the + three-argument call it resembles, so a subclass overriding only the six-argument `rememberText` + still intercepts file ingestion the way it always did. Unintercepted, every call ends at the + request-taking text method, so a host that wants one place to see all traffic overrides that + one. `ExtractionContentProfileRef(name, version)` names a version of a host's content profile — the host's durable answer to what extraction of this kind of material should do. DICE carries the two strings and nothing else: it never looks a profile up, never reads policy out of it, and **selects no provider, model, or credential from it**. The host owns the @@ -1088,27 +1104,19 @@ and the consumer PRs that deliver it). version together, so republishing a profile under a new version yields a reference the host can tell apart from the old one. The type validates non-blank components and caps their lengths (256 for a name, 64 for a version), because a reference is an identifier - rather than a place to put a payload. It is not an authorization token, and may not carry a - direct identifier or a dereferenceable secret. `SourceAnalysisContext` gains an optional + the host mints and no place to put a payload. It is not an authorization token, and may not + carry a direct identifier or a dereferenceable secret. `SourceAnalysisContext` gains an optional `profile`, defaulting to null, with a `withProfile` copy helper. It is checked against no other field: a `sourceRevision` is coupled to its `sourceLocator` because it names a version of a - specific source, while a profile is independent of everything else, and the `init` block says - so rather than inventing a relationship the contract does not have. `rememberText`, - `rememberTextFromSource`, `rememberFile` and `rememberFileFromSource` each take `profile` as a - trailing argument — an extra argument rather than a new method name, which is the opposite of - Wave A's `rememberTextFromSource` split, because a locator is *required* by the source-aware - calls and a profile is optional everywhere. Each of the four is now two declarations: the - pre-profile signature exactly as it was, delegating to a new maximum-arity form that takes the - reference. That split is deliberate — `@JvmOverloads` emits every reduced-arity overload as - `final` even on an `open` function, so folding the new argument into the existing declarations - would have turned each method's pre-change maximum arity into a final bridge and broken - subclasses that override it. Dispatch follows the same principle: a call that carries no - profile takes the chain it took before profiles existed — the file entry points hand their text - to the *pre-profile* text signature, so a subclass overriding only that one still intercepts - file ingestion — while a call that actually carries a profile goes wide, because the legacy - signature cannot express one. The wide forms are terminal and never route back, so there is no - cycle. Unintercepted, every call still ends at the maximum-arity text form, so overriding that - one sees all traffic. `SourceAnalysisRequestEvent` gains `profile()`, open and null-defaulted, + specific source, while a profile is independent of everything else, and the `init` block says so + without inventing a relationship the contract does not have. The request is the single door onto + that field from the entry points, so nothing has to keep two copies of a profile in step. Adding + the request argument by growing the existing declarations would have broken subclasses: + `@JvmOverloads` emits every reduced-arity overload as `final` even on an `open` function, so + folding it into `rememberText` would have turned the six-argument form into a final bridge and + stopped a subclass overriding it from compiling. Each entry point is therefore two declarations, + the one that was already there and the one taking a request. + `SourceAnalysisRequestEvent` gains `profile()`, open and null-defaulted, and `ConversationAnalysisRequestEvent` takes it on its longer constructor. Both paths feed one `buildContext`, which is what makes the async path carry a profile identically; a test counts the accessor being read exactly once. Nothing downstream consults the reference — a test @@ -1123,22 +1131,22 @@ and the consumer PRs that deliver it). Java constructor-descriptor compatibility are claimed. `@JvmOverloads` on `SourceAnalysisContext` preserves every published constructor descriptor and adds one on the end; a test enumerates arities 3 through 12 (each with the trailing `DefaultConstructorMarker` Kotlin emits because - `contextId` is a value class) and asserts all of them resolve. Every `rememberText`, - `rememberTextFromSource`, `rememberFile` and `rememberFileFromSource` descriptor survives, with - exactly one added per method name, on the end; a test pins the exact descriptor set of all four - names and that the profile is always the last parameter. + `contextId` is a value class) and asserts all of them resolve. Every `rememberText` and + `rememberFile` descriptor survives, with exactly one added per method name, on the end; a test + pins the exact descriptor set of both names, asserts the request is always the last parameter, + and asserts no entry point takes a locator, a revision or a profile as a loose argument. **Subclass-override compatibility is part of the claimed surface**: every signature that was - overridable before this slice still is — `rememberText` at six arguments, `rememberFile` at - three, `rememberTextFromSource` at eight, `rememberFileFromSource` at five — and each method's - new maximum-arity form is overridable too. It is proven twice: a reflection test asserts - `Modifier.isFinal` is false on all eight and true on the reduced arities that were already final - bridges, and a Java subclass in the compat suite overrides all four pre-profile signatures, so - the suite compiling is the second proof (`javac` rejects `@Override` on a final method). A - Kotlin test additionally constructs a subclass overriding the six-argument `rememberText` and - the three-argument `rememberFile` and asserts a three-argument call still reaches the override. - Being overridable is not the whole guarantee — the override also has to be reached — so two - further tests pin the dispatch rule: a subclass overriding only the pre-profile text methods - still sees both file entry points, and a file call carrying a profile goes wide instead. + overridable before this slice still is — `rememberText` at six arguments and `rememberFile` at + three — and each method's request-taking form is overridable too. It is proven twice: a + reflection test asserts `Modifier.isFinal` is false on all four and true on the reduced arities + that were already final bridges, and a Java subclass in the compat suite overrides both + signatures, so the suite compiling is the second proof (`javac` rejects `@Override` on a final + method). A Kotlin pin calls every shape a caller could have written against those two signatures + — every published arity, positional and named — from a subclass that overrides both, and asserts + each call reaches the override. Being overridable is not the whole guarantee — the override also + has to be reached — so two further tests pin the dispatch rule: a subclass overriding only the + text entry points still sees file ingestion, and a file call carrying a request goes to the + request-taking text method. `ConversationAnalysisRequestEvent` keeps its five-argument constructor and gains a six-argument form; its `sourceLocator` parameter relaxes from non-null to nullable, so a publisher can name a profile for material it has no typed source for, and every call that @@ -1171,4 +1179,6 @@ and the consumer PRs that deliver it). `SourceAnalysisContext`, every `remember*` entry point, `SourceAnalysisRequestEvent`, and `ConversationAnalysisRequestEvent` — and return together with the write that consumes them once the durable run store lands (DICE #67 and the run-model slices above it). No caller outside this - slice's own code and tests used the parameter for anything, so there is nothing to migrate. + slice's own code and tests used the parameter for anything, so there is nothing to migrate. When + the reference does return it arrives as a field on `ExtractionRequest`, which is what the request + object is for: the entry-point signatures will be the ones this entry describes. 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 a10cd4f3..46d37cd1 100644 --- a/dice/src/main/kotlin/com/embabel/dice/common/SourceAnalysisRequestEvent.kt +++ b/dice/src/main/kotlin/com/embabel/dice/common/SourceAnalysisRequestEvent.kt @@ -29,10 +29,10 @@ import org.springframework.context.ApplicationEvent * (e.g., UrbotUser, Customer) can be used directly. * * A publisher that knows where its material came from can say so by overriding - * [sourceLocator] and [sourceRevision]. The extraction listener puts both onto the - * `SourceAnalysisContext` it builds, so the async path grounds propositions exactly - * the way a direct `rememberTextFromSource` call does. Both default to null, so an - * existing subclass carries no provenance and behaves as it always did. + * [sourceLocator] and [sourceRevision]. The extraction listener collects both into an + * `ExtractionRequest` and puts them onto the `SourceAnalysisContext` it builds, so the async path + * grounds propositions exactly the way a direct `rememberText` call carrying a request does. Both + * default to null, so an existing subclass carries no provenance and behaves as it always did. * * [profile] works the same way and reaches the same context through the same call, so an async * publisher can attribute its extraction to a content profile without the listener growing a diff --git a/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRequest.kt b/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRequest.kt new file mode 100644 index 00000000..a5e5a74e --- /dev/null +++ b/dice/src/main/kotlin/com/embabel/dice/proposition/extraction/ExtractionRequest.kt @@ -0,0 +1,107 @@ +/* + * Copyright 2024-2026 Embabel Pty Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.embabel.dice.proposition.extraction + +import com.embabel.dice.provenance.SourceLocator +import com.embabel.dice.provenance.SourceRevisionRef +import org.jetbrains.annotations.ApiStatus + +/** + * What a caller wants to say about one extraction, on top of the text and the user it belongs to. + * + * Extraction keeps learning about new dimensions — where the material came from, which version of + * it, which content policy it runs under — and each one would otherwise mean another argument on + * [IncrementalPropositionExtraction.rememberText] and another overload to keep the old shape + * callable. They travel here together, so the next dimension is a field on this type and the + * entry-point signatures stay put. A host that overrides an entry point keeps compiling when one + * is added, and sees the new value without touching its override. + * + * Everything is optional. An empty request — [NONE], or `ExtractionRequest()` — asks for the + * extraction DICE has always done. + * + * A [sourceRevision] needs a [sourceLocator] whose key it matches, because a revision names one + * version of one specific source. That pairing is checked while the request is being built, so a + * caller finds out about a mismatch before extraction reads a byte. [profile] is checked against + * nothing: it is independent of where the material came from, and coupling it to the other two + * would invent a relationship the contract does not have. + * + * EXPERIMENTAL, for as long as [ExtractionContentProfileRef] is. + * + * @property sourceLocator where this run's material lives, when the caller has a typed source for + * it. The pipeline stamps it onto every proposition's provenance, so a caller who knows the real + * source gets richer grounding than the content-hash fallback. + * @property sourceRevision the provider's own identifier for the version of [sourceLocator] this + * run reads. Supplying one asserts that the revision covers the whole text or the whole file + * being extracted; DICE reads material as one aggregate and cannot work that out for itself. + * @property profile the host's content-profile identity for this extraction. DICE carries it and + * does nothing else with it: no provider, model, or credential is chosen from it, and extraction + * runs exactly as it would without one. + */ +@ApiStatus.Experimental +data class ExtractionRequest @JvmOverloads constructor( + val sourceLocator: SourceLocator? = null, + val sourceRevision: SourceRevisionRef? = null, + val profile: ExtractionContentProfileRef? = null, +) { + + init { + sourceRevision?.let { revision -> + val locator = requireNotNull(sourceLocator) { + "sourceLocator is required when sourceRevision is set" + } + require(revision.sourceKey == locator.key()) { + "sourceRevision source key must match sourceLocator source key" + } + } + } + + /** + * True when this request carries nothing at all, so the call means what the same call meant + * before requests existed. Comparing against [NONE] keeps this honest as fields are added. + */ + val isEmpty: Boolean + get() = this == NONE + + /** + * Returns a copy grounded in the given source. + */ + fun withSourceLocator(sourceLocator: SourceLocator): ExtractionRequest = + copy(sourceLocator = sourceLocator) + + /** + * Returns a copy carrying a revision of this request's source. Throws if there is no locator, + * or if the revision names a different source key. + */ + fun withSourceRevision(sourceRevision: SourceRevisionRef): ExtractionRequest = + copy(sourceRevision = sourceRevision) + + /** + * Returns a copy attributed to the given content [profile]. EXPERIMENTAL. Changes no other + * field and no extraction behaviour. + */ + fun withProfile(profile: ExtractionContentProfileRef): ExtractionRequest = + copy(profile = profile) + + companion object { + + /** + * The request that asks for nothing beyond plain extraction. Entry points use it as the + * default, and a caller can build outwards from it with the `with...` helpers. + */ + @JvmField + val NONE: ExtractionRequest = ExtractionRequest() + } +} 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 2cf60c3a..700032c0 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 @@ -34,8 +34,6 @@ import com.embabel.dice.pipeline.ChunkPropositionResult import com.embabel.dice.pipeline.PropositionPipeline import com.embabel.dice.projection.graph.GraphProjectionService import com.embabel.dice.proposition.PropositionRepository -import com.embabel.dice.provenance.SourceLocator -import com.embabel.dice.provenance.SourceRevisionRef import org.slf4j.LoggerFactory import org.springframework.context.event.EventListener import org.springframework.scheduling.annotation.Async @@ -164,7 +162,7 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( * Extract propositions from a file via Tika and persist them. * Requires `embabel-agent-rag-tika` on the classpath. * - * This is the signature that existed before profiles, kept as its own declaration so it + * This is the signature that existed before requests, kept as its own declaration so it * stays overridable — see the note on [rememberText]. */ open fun rememberFile( @@ -172,26 +170,26 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( filename: String, user: NamedEntity, ) = withRememberedFileText(inputStream, filename) { text -> - // Deliberately the pre-profile text signature, not the wide one. This is the dispatch - // this method had before profiles existed, so a subclass that overrides only that - // signature still intercepts file ingestion the way it always did. + // Deliberately the three-argument-era text signature. This is the dispatch this method + // had before requests existed, so a subclass that overrides only that signature still + // intercepts file ingestion the way it always did. rememberText(text, "remember:$filename", user, emptyList(), null, null) } /** - * Extract propositions from a file via Tika, attributed to an extraction content profile. - * EXPERIMENTAL; see [rememberText] for what the reference means. + * Extract propositions from a file via Tika, on the terms the [request] sets — the source it + * was read from, the revision of that source, the content profile it runs under. * - * With no profile to carry this hands straight back to the three-argument form, so a call - * that looks like a pre-profile call also dispatches like one. + * A [request] that carries nothing hands straight back to the three-argument form, so a call + * that looks like a pre-request call also dispatches like one. */ open fun rememberFile( inputStream: InputStream, filename: String, user: NamedEntity, - profile: ExtractionContentProfileRef?, + request: ExtractionRequest, ) { - if (profile == null) { + if (request.isEmpty) { rememberFile(inputStream, filename, user) return } @@ -203,81 +201,7 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( emptyList(), null, null, - profile, - ) - } - } - - /** - * Extract propositions from a file and ground them in the caller's typed source. - * - * Passing a [sourceRevision] asserts that the locator's revision covers the whole file, all - * of it, as extracted here. DICE reads the file as one aggregate and has no way to work out - * whether a provider's revision really spans it, so the host has to know that and say so. - * - * This is the signature that existed before profiles, kept as its own declaration so it - * stays overridable — see the note on [rememberText]. - */ - @JvmOverloads - open fun rememberFileFromSource( - inputStream: InputStream, - filename: String, - user: NamedEntity, - sourceLocator: SourceLocator, - sourceRevision: SourceRevisionRef? = null, - ) { - require(sourceRevision == null || sourceRevision.sourceKey == sourceLocator.key()) { - "sourceRevision source key must match sourceLocator source key" - } - withRememberedFileText(inputStream, filename) { text -> - // The pre-profile text signature, for the same reason as [rememberFile]. - rememberTextFromSource( - text, - "remember:$filename", - user, - sourceLocator, - sourceRevision, - emptyList(), - null, - null, - ) - } - } - - /** - * Extract propositions from a file, ground them in the caller's typed source, and attribute - * them to an extraction content profile. EXPERIMENTAL; see [rememberText] for what the - * reference means. The profile is independent of the locator and the revision. - * - * With no profile to carry this hands straight back to the five-argument form, so a call - * that looks like a pre-profile call also dispatches like one. - */ - open fun rememberFileFromSource( - inputStream: InputStream, - filename: String, - user: NamedEntity, - sourceLocator: SourceLocator, - sourceRevision: SourceRevisionRef?, - profile: ExtractionContentProfileRef?, - ) { - if (profile == null) { - rememberFileFromSource(inputStream, filename, user, sourceLocator, sourceRevision) - return - } - require(sourceRevision == null || sourceRevision.sourceKey == sourceLocator.key()) { - "sourceRevision source key must match sourceLocator source key" - } - withRememberedFileText(inputStream, filename) { text -> - rememberTextFromSource( - text, - "remember:$filename", - user, - sourceLocator, - sourceRevision, - emptyList(), - null, - null, - profile, + request, ) } } @@ -320,14 +244,15 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( * be persisted as NEW entities. `null` (default) uses the extractor * instance's default; see [SourceAnalysisContext.mintNewEntities]. * - * This is the signature that existed before profiles, and it stays its own declaration - * rather than growing a defaulted parameter. `@JvmOverloads` emits every reduced-arity - * overload as `final`, so folding the new argument into this method would have turned the - * six-argument form — the one a subclass overrides — into a final bridge. Declaring the two - * shapes separately keeps both open. Unintercepted, every call lands on the seven-argument + * This is the signature that existed before requests, and it stays its own declaration. + * Growing it with a defaulted parameter would break subclasses: `@JvmOverloads` emits every + * reduced-arity overload as `final`, so folding the new argument into this method would have + * turned the six-argument form — the one a subclass overrides — into a final bridge. + * Declaring the two shapes separately keeps both open. Unintercepted, every call lands on the + * seven-argument * form below, so overriding that one sees everything; overriding this one sees everything a - * pre-profile subclass used to see, including file ingestion, which still routes through - * here rather than jumping to the wide form. + * subclass written before requests used to see, including file ingestion, which still + * routes through here. */ @JvmOverloads open fun rememberText( @@ -337,123 +262,44 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( additionalGrounding: List = emptyList(), perspective: ExtractionPerspective? = null, mintNewEntities: Boolean? = null, - ) = rememberText(text, sourceId, user, additionalGrounding, perspective, mintNewEntities, null) - - /** - * Extract propositions from raw text, attributed to an extraction content profile. - * - * Every other text entry point funnels here, so this is the one method to override to see - * every call. - * - * @param profile extraction content profile this call should be attributed to, or null. - * EXPERIMENTAL. DICE puts it on the context and does nothing else with it: no provider, - * model, or credential is selected from it, and extraction runs exactly as it would - * without one. - */ - open fun rememberText( - text: String, - sourceId: String, - user: NamedEntity, - additionalGrounding: List, - perspective: ExtractionPerspective?, - mintNewEntities: Boolean?, - profile: ExtractionContentProfileRef?, - ) = - rememberTextInternal( - text = text, - sourceId = sourceId, - user = user, - additionalGrounding = additionalGrounding, - perspective = perspective, - mintNewEntities = mintNewEntities, - profile = profile, - ) - - /** - * Extract propositions from raw text and ground them in the caller's typed source. - * - * A separate method rather than more optional arguments on [rememberText]: a locator is - * required here, so the two entry points have genuinely different contracts, and Kotlin and - * Java call sites both stay unambiguous. - * - * [sourceId] keeps its old meaning — the exact caller-supplied chunk and grounding - * identifier. Passing a [sourceRevision] asserts that [sourceLocator]'s revision covers the - * whole text. DICE cannot read revision coverage out of an untyped [sourceId] or out of - * [additionalGrounding], so the host has to know that and say so. - * - * This is the signature that existed before profiles, kept as its own declaration so it - * stays overridable — see the note on [rememberText]. - */ - @JvmOverloads - open fun rememberTextFromSource( - text: String, - sourceId: String, - user: NamedEntity, - sourceLocator: SourceLocator, - sourceRevision: SourceRevisionRef? = null, - additionalGrounding: List = emptyList(), - perspective: ExtractionPerspective? = null, - mintNewEntities: Boolean? = null, - ) = rememberTextFromSource( + ) = rememberText( text, sourceId, user, - sourceLocator, - sourceRevision, additionalGrounding, perspective, mintNewEntities, - null, + ExtractionRequest.NONE, ) /** - * Extract propositions from raw text, ground them in the caller's typed source, and - * attribute them to an extraction content profile. EXPERIMENTAL; see [rememberText] for what - * the reference means. A profile is independent of the locator and the revision — it says - * what extraction should do, they say what it is reading. + * Extract propositions from raw text on the terms the [request] sets — the source it was read + * from, the revision of that source, the content profile it runs under. + * + * Every other entry point funnels here, so this is the one method to override to see every + * call. It is also where a new extraction dimension shows up: it arrives as a field on + * [ExtractionRequest] and this signature stays as it is. + * + * [sourceId] keeps its old meaning — the exact caller-supplied chunk and grounding + * identifier. DICE cannot read source identity or revision coverage out of an untyped + * [sourceId] or out of [additionalGrounding], which is why the [request] carries them + * explicitly. */ - open fun rememberTextFromSource( + open fun rememberText( text: String, sourceId: String, user: NamedEntity, - sourceLocator: SourceLocator, - sourceRevision: SourceRevisionRef?, additionalGrounding: List, perspective: ExtractionPerspective?, mintNewEntities: Boolean?, - profile: ExtractionContentProfileRef?, - ) = - rememberTextInternal( - text = text, - sourceId = sourceId, - user = user, - sourceLocator = sourceLocator, - sourceRevision = sourceRevision, - additionalGrounding = additionalGrounding, - perspective = perspective, - mintNewEntities = mintNewEntities, - profile = profile, - ) - - private fun rememberTextInternal( - text: String, - sourceId: String, - user: NamedEntity, - sourceLocator: SourceLocator? = null, - sourceRevision: SourceRevisionRef? = null, - additionalGrounding: List = emptyList(), - perspective: ExtractionPerspective? = null, - mintNewEntities: Boolean? = null, - profile: ExtractionContentProfileRef? = null, + request: ExtractionRequest, ) { val context = buildContext( user = user, sourceId = sourceId, perspective = perspective, mintNewEntities = mintNewEntities, - sourceLocator = sourceLocator, - sourceRevision = sourceRevision, - profile = profile, + request = request, ) val result = propositionPipeline.processOnce( text, sourceId, context, additionalGrounding = additionalGrounding, @@ -501,15 +347,16 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( return } - // The async path grounds propositions exactly the way rememberTextFromSource does: - // whatever provenance and profile the event carries goes through the same - // buildContext call. + // The async path grounds propositions exactly the way a direct call does: whatever the + // event carries becomes a request and goes through the same buildContext call. val context = buildContext( user = event.user, sourceId = source.id, - sourceLocator = event.sourceLocator(), - sourceRevision = event.sourceRevision(), - profile = event.profile(), + request = ExtractionRequest( + sourceLocator = event.sourceLocator(), + sourceRevision = event.sourceRevision(), + profile = event.profile(), + ), ) logger.info( "Context relations count: {}, injected relations count: {}", @@ -541,9 +388,7 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( sourceId: String = "", perspective: ExtractionPerspective? = null, mintNewEntities: Boolean? = null, - sourceLocator: SourceLocator? = null, - sourceRevision: SourceRevisionRef? = null, - profile: ExtractionContentProfileRef? = null, + request: ExtractionRequest = ExtractionRequest.NONE, ): SourceAnalysisContext { val aliases = try { currentUserAliasesProvider(user) @@ -603,17 +448,13 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( ctx = ctx.withMintedEntityProperties(stamped) } } - if (sourceLocator != null) { - ctx = ctx.withSourceLocator(sourceLocator) - } - if (sourceRevision != null) { - ctx = ctx.withSourceRevision(sourceRevision) - } + // The request is the one door everything the caller asked for comes through, so the + // 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 it — that is what "DICE // holds profile identity and the host binds policy" means in code. - if (profile != null) { - ctx = ctx.withProfile(profile) - } + request.profile?.let { ctx = ctx.withProfile(it) } return ctx } diff --git a/dice/src/test/java/com/embabel/dice/ExtractionProfileJavaInteropTest.java b/dice/src/test/java/com/embabel/dice/ExtractionProfileJavaInteropTest.java index 4c948c03..57cb31d5 100644 --- a/dice/src/test/java/com/embabel/dice/ExtractionProfileJavaInteropTest.java +++ b/dice/src/test/java/com/embabel/dice/ExtractionProfileJavaInteropTest.java @@ -26,6 +26,7 @@ import com.embabel.dice.incremental.IncrementalSource; import com.embabel.dice.proposition.extraction.ExtractionContentProfileRef; import com.embabel.dice.proposition.extraction.ExtractionPerspective; +import com.embabel.dice.proposition.extraction.ExtractionRequest; import com.embabel.dice.proposition.extraction.IncrementalPropositionExtraction; import com.embabel.dice.provenance.ContentAddressedLocator; import com.embabel.dice.provenance.SourceLocator; @@ -44,10 +45,9 @@ import static org.junit.jupiter.api.Assertions.assertThrows; /** - * Java's view of the profile contract, with and without a profile present. - * {@code @JvmOverloads} means the remember entry points keep every descriptor a Java caller - * could already have compiled against, and the profile argument only ever adds a descriptor - * on the end. + * Java's view of the profile contract, with and without a profile present. A profile travels on an + * {@link ExtractionRequest}, so the remember entry points keep every descriptor a Java caller could + * already have compiled against and each name gains exactly one more, taking the request. */ class ExtractionProfileJavaInteropTest { @@ -85,7 +85,7 @@ void javaBuiltContextsCarryNoProfileUnlessAsked() { } @Test - void retainsEveryLegacyRememberDescriptorAndAddsProfileAwareOnes() throws Exception { + void retainsEveryLegacyRememberDescriptorAndAddsRequestAwareOnes() throws Exception { Class[][] legacyTextParameters = { {String.class, String.class, NamedEntity.class}, {String.class, String.class, NamedEntity.class, List.class}, @@ -99,14 +99,7 @@ void retainsEveryLegacyRememberDescriptorAndAddsProfileAwareOnes() throws Except IncrementalPropositionExtraction.class.getMethod( "rememberText", String.class, String.class, NamedEntity.class, List.class, - ExtractionPerspective.class, Boolean.class, ExtractionContentProfileRef.class - ); - - IncrementalPropositionExtraction.class.getMethod( - "rememberTextFromSource", - String.class, String.class, NamedEntity.class, SourceLocator.class, - SourceRevisionRef.class, List.class, ExtractionPerspective.class, Boolean.class, - ExtractionContentProfileRef.class + ExtractionPerspective.class, Boolean.class, ExtractionRequest.class ); IncrementalPropositionExtraction.class.getMethod( @@ -114,16 +107,11 @@ void retainsEveryLegacyRememberDescriptorAndAddsProfileAwareOnes() throws Except ); IncrementalPropositionExtraction.class.getMethod( "rememberFile", - InputStream.class, String.class, NamedEntity.class, - ExtractionContentProfileRef.class - ); - IncrementalPropositionExtraction.class.getMethod( - "rememberFileFromSource", - InputStream.class, String.class, NamedEntity.class, SourceLocator.class, - SourceRevisionRef.class, ExtractionContentProfileRef.class + InputStream.class, String.class, NamedEntity.class, ExtractionRequest.class ); - // A profile never lands where a legacy caller already fills every argument. + // A profile reaches extraction on the request, so no entry point takes one directly and + // none lands where a caller filling every legacy argument already is. assertThrows( NoSuchMethodException.class, () -> IncrementalPropositionExtraction.class.getMethod( @@ -132,6 +120,46 @@ void retainsEveryLegacyRememberDescriptorAndAddsProfileAwareOnes() throws Except ExtractionContentProfileRef.class ) ); + assertThrows( + NoSuchMethodException.class, + () -> IncrementalPropositionExtraction.class.getMethod( + "rememberText", + String.class, String.class, NamedEntity.class, List.class, + ExtractionPerspective.class, Boolean.class, ExtractionContentProfileRef.class + ) + ); + assertThrows( + NoSuchMethodException.class, + () -> IncrementalPropositionExtraction.class.getMethod( + "rememberFile", + InputStream.class, String.class, NamedEntity.class, + ExtractionContentProfileRef.class + ) + ); + } + + @Test + void javaCallersBuildAndReadProfileBearingRequests() throws Exception { + ExtractionContentProfileRef profile = new ExtractionContentProfileRef("house-style", "v1"); + SourceLocator locator = new ContentAddressedLocator("java-profile-source"); + SourceRevisionRef revision = new SourceRevisionRef(locator.key(), "r1"); + + ExtractionRequest.class.getConstructor( + SourceLocator.class, SourceRevisionRef.class, ExtractionContentProfileRef.class + ); + + // A profile needs no source of its own: the two dimensions stay independent on a request. + ExtractionRequest profileOnly = new ExtractionRequest(null, null, profile); + assertSame(profile, profileOnly.getProfile()); + assertNull(profileOnly.getSourceLocator()); + + ExtractionRequest everything = new ExtractionRequest(locator, revision, profile); + assertSame(profile, everything.getProfile()); + assertSame(revision, everything.getSourceRevision()); + + // The copy helper is a copy: the request it was called on is untouched. + assertSame(profile, ExtractionRequest.NONE.withProfile(profile).getProfile()); + assertNull(ExtractionRequest.NONE.getProfile()); } @Test @@ -202,9 +230,9 @@ void conversationEventKeepsItsFiveArgumentConstructorAndGainsProfileAwareOnes() } @Test - void javaSubclassesStillOverrideThePreProfileSignatures() throws Exception { + void javaSubclassesStillOverrideTheSignaturesThatPredateRequests() throws Exception { // The proof is that this file compiles: javac rejects @Override on a final method, so - // LegacyOverridingJavaExtraction would not build if adding the profile arguments had + // LegacyOverridingJavaExtraction would not build if adding the request argument had // turned the six-argument rememberText or the three-argument rememberFile into the // final bridges @JvmOverloads emits for reduced arities. assertFalse( @@ -235,7 +263,7 @@ void javaSubclassesStillOverrideThePreProfileSignatures() throws Exception { } /** - * A Java subclass written before profiles existed, overriding the entry-point signatures + * A Java subclass written before requests existed, overriding the entry-point signatures * that were open then. It is never instantiated; compiling it is the assertion. */ @SuppressWarnings("unused") @@ -275,31 +303,6 @@ public void rememberText( public void rememberFile(InputStream inputStream, String filename, NamedEntity user) { // A host's interception point; deliberately does nothing. } - - @Override - public void rememberFileFromSource( - InputStream inputStream, - String filename, - NamedEntity user, - SourceLocator sourceLocator, - SourceRevisionRef sourceRevision - ) { - // A host's interception point; deliberately does nothing. - } - - @Override - public void rememberTextFromSource( - String text, - String sourceId, - NamedEntity user, - SourceLocator sourceLocator, - SourceRevisionRef sourceRevision, - List additionalGrounding, - ExtractionPerspective perspective, - Boolean mintNewEntities - ) { - // A host's interception point; deliberately does nothing. - } } /** @@ -318,16 +321,18 @@ private static void compileJavaSourceCalls( ) { extraction.rememberText("legacy", "legacy-id", user); extraction.rememberText("legacy", "legacy-id", user, List.of(), null, null); - extraction.rememberText("profiled", "profiled-id", user, List.of(), null, null, profile); + extraction.rememberText( + "profiled", "profiled-id", user, List.of(), null, null, + new ExtractionRequest(null, null, profile) + ); extraction.rememberFile(input, "legacy.txt", user); - extraction.rememberFile(input, "profiled.txt", user, profile); - extraction.rememberTextFromSource("source", "source-id", user, locator); - extraction.rememberTextFromSource( - "source", "source-id", user, locator, revision, List.of(), null, null, profile + extraction.rememberFile(input, "profiled.txt", user, new ExtractionRequest(null, null, profile)); + extraction.rememberText( + "source", "source-id", user, List.of(), null, null, + new ExtractionRequest(locator, revision, profile) ); - extraction.rememberFileFromSource(input, "source.txt", user, locator); - extraction.rememberFileFromSource( - input, "source.txt", user, locator, revision, profile + extraction.rememberFile( + input, "source.txt", user, new ExtractionRequest(locator, revision, profile) ); } diff --git a/dice/src/test/java/com/embabel/dice/SourceRevisionJavaInteropTest.java b/dice/src/test/java/com/embabel/dice/SourceRevisionJavaInteropTest.java index 8a6f9bff..3b4dc216 100644 --- a/dice/src/test/java/com/embabel/dice/SourceRevisionJavaInteropTest.java +++ b/dice/src/test/java/com/embabel/dice/SourceRevisionJavaInteropTest.java @@ -21,6 +21,7 @@ import com.embabel.dice.incremental.IncrementalSource; import com.embabel.dice.proposition.PropositionStatus; import com.embabel.dice.proposition.extraction.ExtractionPerspective; +import com.embabel.dice.proposition.extraction.ExtractionRequest; import com.embabel.dice.proposition.extraction.IncrementalPropositionExtraction; import com.embabel.dice.provenance.ContentAddressedLocator; import com.embabel.dice.provenance.ProvenanceEntry; @@ -48,8 +49,9 @@ * {@link RetiredProposition}, {@link ExtractRequest}, {@link ProvenanceEntryDto} and * {@link LineageDto} means every constructor descriptor a Java caller could already have * compiled against survives, and each new argument arrives as one extra descriptor on the end. - * The remember entry points work the other way round: the revision-aware calls are separate - * methods, so the old {@code rememberText} and {@code rememberFile} descriptors never move. + * The remember entry points carry a revision on an {@link ExtractionRequest}, so the + * {@code rememberText} and {@code rememberFile} descriptors a Java caller already had never move + * and each name gains exactly one more. */ class SourceRevisionJavaInteropTest { @@ -160,7 +162,7 @@ void javaCallersReadAndBuildRevisionValues() { } @Test - void retainsEveryLegacyRememberDescriptorAndAddsDistinctSourceDescriptors() throws Exception { + void retainsEveryLegacyRememberDescriptorAndAddsOneRequestDescriptorPerName() throws Exception { Class[][] legacyTextParameters = { {String.class, String.class, NamedEntity.class}, {String.class, String.class, NamedEntity.class, List.class}, @@ -171,20 +173,24 @@ void retainsEveryLegacyRememberDescriptorAndAddsDistinctSourceDescriptors() thro for (Class[] parameters : legacyTextParameters) { IncrementalPropositionExtraction.class.getMethod("rememberText", parameters); } + IncrementalPropositionExtraction.class.getMethod( + "rememberText", + String.class, String.class, NamedEntity.class, List.class, + ExtractionPerspective.class, Boolean.class, ExtractionRequest.class + ); + IncrementalPropositionExtraction.class.getMethod( + "rememberFile", + InputStream.class, + String.class, + NamedEntity.class + ); + IncrementalPropositionExtraction.class.getMethod( + "rememberFile", + InputStream.class, String.class, NamedEntity.class, ExtractionRequest.class + ); - Class[][] sourceAwareTextParameters = { - {String.class, String.class, NamedEntity.class, SourceLocator.class}, - {String.class, String.class, NamedEntity.class, SourceLocator.class, SourceRevisionRef.class}, - {String.class, String.class, NamedEntity.class, SourceLocator.class, SourceRevisionRef.class, - List.class}, - {String.class, String.class, NamedEntity.class, SourceLocator.class, SourceRevisionRef.class, - List.class, ExtractionPerspective.class}, - {String.class, String.class, NamedEntity.class, SourceLocator.class, SourceRevisionRef.class, - List.class, ExtractionPerspective.class, Boolean.class}, - }; - for (Class[] parameters : sourceAwareTextParameters) { - IncrementalPropositionExtraction.class.getMethod("rememberTextFromSource", parameters); - } + // A locator reaches extraction on the request, so it never appears as a loose argument + // and there is no second method name carrying it. assertThrows( NoSuchMethodException.class, () -> IncrementalPropositionExtraction.class.getMethod( @@ -195,28 +201,6 @@ void retainsEveryLegacyRememberDescriptorAndAddsDistinctSourceDescriptors() thro SourceLocator.class ) ); - - IncrementalPropositionExtraction.class.getMethod( - "rememberFile", - InputStream.class, - String.class, - NamedEntity.class - ); - IncrementalPropositionExtraction.class.getMethod( - "rememberFileFromSource", - InputStream.class, - String.class, - NamedEntity.class, - SourceLocator.class - ); - IncrementalPropositionExtraction.class.getMethod( - "rememberFileFromSource", - InputStream.class, - String.class, - NamedEntity.class, - SourceLocator.class, - SourceRevisionRef.class - ); assertThrows( NoSuchMethodException.class, () -> IncrementalPropositionExtraction.class.getMethod( @@ -229,6 +213,29 @@ void retainsEveryLegacyRememberDescriptorAndAddsDistinctSourceDescriptors() thro ); } + @Test + void javaCallersBuildRequestsAtEveryArityAndAreHeldToThePairingRule() throws Exception { + SourceLocator locator = new ContentAddressedLocator("java-request-source"); + SourceRevisionRef revision = new SourceRevisionRef(locator.key(), "opaque-r1"); + + ExtractionRequest.class.getConstructor(); + ExtractionRequest.class.getConstructor(SourceLocator.class); + ExtractionRequest.class.getConstructor(SourceLocator.class, SourceRevisionRef.class); + + assertTrue(ExtractionRequest.NONE.isEmpty()); + assertNull(new ExtractionRequest().getSourceLocator()); + assertSame(locator, new ExtractionRequest(locator).getSourceLocator()); + assertSame(revision, new ExtractionRequest(locator, revision).getSourceRevision()); + + assertThrows( + IllegalArgumentException.class, + () -> new ExtractionRequest( + new ContentAddressedLocator("some-other-source"), + revision + ) + ); + } + @Test void legacyAndRevisionAwareJavaEventSubclassesUseTheBaseConstructor() throws Exception { assertArrayEquals( @@ -266,22 +273,24 @@ private static void compileJavaSourceCalls( SourceRevisionRef revision ) { extraction.rememberFile(input, "legacy.txt", user); - extraction.rememberFileFromSource(input, "source.txt", user, locator); - extraction.rememberFileFromSource(input, "revisioned.txt", user, locator, revision); extraction.rememberText("legacy", "legacy-id", user); extraction.rememberText("legacy", "legacy-id", user, List.of()); extraction.rememberText("legacy", "legacy-id", user, List.of(), null); extraction.rememberText("legacy", "legacy-id", user, List.of(), null, null); - extraction.rememberTextFromSource("revisioned", "revisioned-id", user, locator); - extraction.rememberTextFromSource("revisioned", "revisioned-id", user, locator, revision); - extraction.rememberTextFromSource( - "revisioned", "revisioned-id", user, locator, revision, List.of() + + // Java's view of the request: @JvmOverloads publishes it at every arity, and the + // locator-and-revision pairing is checked while it is being built. + extraction.rememberFile(input, "source.txt", user, new ExtractionRequest(locator)); + extraction.rememberFile( + input, "revisioned.txt", user, new ExtractionRequest(locator, revision) ); - extraction.rememberTextFromSource( - "revisioned", "revisioned-id", user, locator, revision, List.of(), null + extraction.rememberText( + "revisioned", "revisioned-id", user, List.of(), null, null, + new ExtractionRequest(locator, revision) ); - extraction.rememberTextFromSource( - "revisioned", "revisioned-id", user, locator, revision, List.of(), null, null + extraction.rememberText( + "revisioned", "revisioned-id", user, List.of(), null, null, + ExtractionRequest.NONE ); } diff --git a/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRequestTest.kt b/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRequestTest.kt new file mode 100644 index 00000000..73de8762 --- /dev/null +++ b/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionRequestTest.kt @@ -0,0 +1,153 @@ +/* + * Copyright 2024-2026 Embabel Pty Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.embabel.dice.proposition.extraction + +import com.embabel.dice.provenance.ContentAddressedLocator +import com.embabel.dice.provenance.SourceRevisionRef +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 the request promises the entry points: the pairing rule holds wherever a request is built, + * a profile is coupled to nothing, and an empty request means plain extraction. + */ +class ExtractionRequestTest { + + private val locator = UriLocator("https://example.com/source") + private val revision = SourceRevisionRef(locator.key(), "r1") + private val profile = ExtractionContentProfileRef("house-style", "v1") + + @Test + fun `an empty request carries nothing and equals NONE`() { + val empty = ExtractionRequest() + + assertThat(empty.sourceLocator).isNull() + assertThat(empty.sourceRevision).isNull() + assertThat(empty.profile).isNull() + assertThat(empty).isEqualTo(ExtractionRequest.NONE) + assertThat(empty.isEmpty).isTrue() + } + + @Test + fun `any field at all makes a request non-empty`() { + // isEmpty is the switch the file entry point routes on, so each field has to flip it. + assertThat(ExtractionRequest(sourceLocator = locator).isEmpty).isFalse() + assertThat( + ExtractionRequest(sourceLocator = locator, sourceRevision = revision).isEmpty, + ).isFalse() + assertThat(ExtractionRequest(profile = profile).isEmpty).isFalse() + } + + @Test + fun `a request keeps exactly what it was given`() { + val request = ExtractionRequest( + sourceLocator = locator, + sourceRevision = revision, + profile = profile, + ) + + assertThat(request.sourceLocator).isSameAs(locator) + assertThat(request.sourceRevision).isSameAs(revision) + assertThat(request.profile).isSameAs(profile) + } + + @Test + fun `a revision requires a locator`() { + assertThatIllegalArgumentException() + .isThrownBy { ExtractionRequest(sourceRevision = revision) } + .withMessageContaining("sourceLocator is required") + + // Same rule through the copy helper, which is where a caller assembling a request + // incrementally would otherwise slip past it. + assertThatIllegalArgumentException() + .isThrownBy { ExtractionRequest.NONE.withSourceRevision(revision) } + .withMessageContaining("sourceLocator is required") + } + + @Test + fun `a revision must name the locator it travels with`() { + val otherLocator = ContentAddressedLocator("some other source") + + assertThatIllegalArgumentException() + .isThrownBy { + ExtractionRequest(sourceLocator = otherLocator, sourceRevision = revision) + } + .withMessageContaining("must match") + + assertThatIllegalArgumentException() + .isThrownBy { + ExtractionRequest(sourceLocator = otherLocator).withSourceRevision(revision) + } + .withMessageContaining("must match") + + // Replacing the locator under a revision that already matched is caught the same way. + assertThatIllegalArgumentException() + .isThrownBy { + ExtractionRequest(sourceLocator = locator, sourceRevision = revision) + .withSourceLocator(otherLocator) + } + .withMessageContaining("must match") + } + + @Test + fun `a profile is checked against nothing`() { + // Profile and source provenance are independent, so every combination is constructible. + assertThat(ExtractionRequest(profile = profile).profile).isSameAs(profile) + assertThat( + ExtractionRequest(sourceLocator = locator, profile = profile).sourceLocator, + ).isSameAs(locator) + assertThat( + ExtractionRequest( + sourceLocator = locator, + sourceRevision = revision, + profile = profile, + ).profile, + ).isSameAs(profile) + } + + @Test + fun `the copy helpers change one field and leave the rest alone`() { + val base = ExtractionRequest(sourceLocator = locator, profile = profile) + + val revised = base.withSourceRevision(revision) + assertThat(revised).isEqualTo(base.copy(sourceRevision = revision)) + assertThat(base.sourceRevision).isNull() + + val reprofiled = base.withProfile(ExtractionContentProfileRef("legal-review", "v2")) + assertThat(reprofiled.sourceLocator).isSameAs(locator) + assertThat(reprofiled.profile).isEqualTo(ExtractionContentProfileRef("legal-review", "v2")) + } + + @Test + fun `two requests carrying the same values are the same request`() { + val one = ExtractionRequest( + sourceLocator = UriLocator("https://example.com/source"), + sourceRevision = SourceRevisionRef(locator.key(), "r1"), + profile = ExtractionContentProfileRef("house-style", "v1"), + ) + val two = ExtractionRequest( + sourceLocator = UriLocator("https://example.com/source"), + sourceRevision = SourceRevisionRef(locator.key(), "r1"), + profile = ExtractionContentProfileRef("house-style", "v1"), + ) + + assertThat(one).isEqualTo(two) + assertThat(one.hashCode()).isEqualTo(two.hashCode()) + assertThat(one).isNotEqualTo(one.withProfile(ExtractionContentProfileRef("house-style", "v2"))) + } +} 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 63509edf..f5522934 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 @@ -66,26 +66,12 @@ import java.util.concurrent.atomic.AtomicInteger class IncrementalPropositionExtractionTest { @Test - fun `legacy and source aware JVM descriptors are exact`() { - val rememberTextParameters = IncrementalPropositionExtraction::class.java.declaredMethods - .filter { it.name == "rememberText" && !it.isSynthetic } - .map { it.parameterTypes.toList() } - .toSet() - val rememberFileParameters = IncrementalPropositionExtraction::class.java.declaredMethods - .filter { it.name == "rememberFile" && !it.isSynthetic } - .map { it.parameterTypes.toList() } - .toSet() - val rememberTextFromSourceParameters = IncrementalPropositionExtraction::class.java.declaredMethods - .filter { it.name == "rememberTextFromSource" && !it.isSynthetic } - .map { it.parameterTypes.toList() } - .toSet() - val rememberFileFromSourceParameters = IncrementalPropositionExtraction::class.java.declaredMethods - .filter { it.name == "rememberFileFromSource" && !it.isSynthetic } - .map { it.parameterTypes.toList() } - .toSet() + fun `entry point JVM descriptors are exact`() { + val rememberTextParameters = declaredParameterLists("rememberText") + val rememberFileParameters = declaredParameterLists("rememberFile") - // Every descriptor that existed before profiles is still here, and the profile - // argument only ever adds one descriptor on the end of each name. + // Every descriptor that existed before requests is still here, and the request argument + // only ever adds one descriptor on the end of each name. val legacyTextPrefix = listOf( String::class.java, String::class.java, @@ -96,8 +82,8 @@ class IncrementalPropositionExtractionTest { ExtractionPerspective::class.java, Boolean::class.javaObjectType, ) - val profileOnly = listOf( - ExtractionContentProfileRef::class.java, + val requestOnly = listOf( + ExtractionRequest::class.java, ) assertEquals( setOf( @@ -105,70 +91,62 @@ class IncrementalPropositionExtractionTest { legacyTextPrefix + List::class.java, legacyTextPrefix + listOf(List::class.java, ExtractionPerspective::class.java), legacyTextFull, - legacyTextFull + profileOnly, + legacyTextFull + requestOnly, ), rememberTextParameters, ) - val sourceTextPrefix = legacyTextPrefix + SourceLocator::class.java - val sourceTextFull = sourceTextPrefix + listOf( - SourceRevisionRef::class.java, - List::class.java, - ExtractionPerspective::class.java, - Boolean::class.javaObjectType, - ) - assertEquals( - setOf( - sourceTextPrefix, - sourceTextPrefix + SourceRevisionRef::class.java, - sourceTextPrefix + listOf(SourceRevisionRef::class.java, List::class.java), - sourceTextPrefix + listOf( - SourceRevisionRef::class.java, - List::class.java, - ExtractionPerspective::class.java, - ), - sourceTextFull, - sourceTextFull + profileOnly, - ), - rememberTextFromSourceParameters, - ) - val legacyFile = listOf( InputStream::class.java, String::class.java, NamedEntity::class.java, ) assertEquals( - setOf(legacyFile, legacyFile + profileOnly), + setOf(legacyFile, legacyFile + requestOnly), rememberFileParameters, ) - val sourceFileFull = - legacyFile + listOf(SourceLocator::class.java, SourceRevisionRef::class.java) - assertEquals( - setOf( - legacyFile + SourceLocator::class.java, - sourceFileFull, - sourceFileFull + profileOnly, - ), - rememberFileFromSourceParameters, - ) - // A profile-aware call can never collapse onto a legacy one: a profile always arrives - // on the end, at an arity a legacy caller never fills. - val everyRememberParameterList = rememberTextParameters + rememberFileParameters + - rememberTextFromSourceParameters + rememberFileFromSourceParameters - everyRememberParameterList.forEach { parameters -> - val hasProfile = ExtractionContentProfileRef::class.java in parameters - if (hasProfile) { + // Two entry points, and no third and fourth name to keep in step with them. Everything a + // caller supplies beyond the arguments these methods always took rides on the request, so + // a locator, a revision and a profile add no method names and no arities. + assertEquals(emptySet>>(), declaredParameterLists("rememberTextFromSource")) + assertEquals(emptySet>>(), declaredParameterLists("rememberFileFromSource")) + + // A request-carrying call can never collapse onto one written without a request: the + // request always arrives on the end, at an arity the other caller never fills. + (rememberTextParameters + rememberFileParameters).forEach { parameters -> + if (ExtractionRequest::class.java in parameters) { assertEquals( - profileOnly, + requestOnly, parameters.takeLast(1), - "profile must be the last parameter: $parameters", + "the request must be the last parameter: $parameters", ) } } } + @Test + fun `no field, parameter or return type outside the request carries what the request carries`() { + // The point of the request object: a locator, a revision and a profile reach extraction + // through it and through nothing else. A future dimension added as a loose parameter on an + // entry point would put the surface back where it started, so this sweeps the whole class. + val carried = setOf( + SourceLocator::class.java, + SourceRevisionRef::class.java, + ExtractionContentProfileRef::class.java, + ) + IncrementalPropositionExtraction::class.java.declaredMethods + .filter { it.name.startsWith("remember") && '$' !in it.name } + .forEach { method -> + method.parameterTypes.forEach { parameter -> + assertFalse( + parameter in carried, + "${method.name} takes ${parameter.simpleName} directly; it belongs on the request", + ) + } + } + } + @Test fun `no entry point, context, constructor, or field anywhere carries a run reference`() { // PR #94 review comment: buildContext accepted a currentRun and put it on the context. @@ -198,7 +176,7 @@ class IncrementalPropositionExtractionTest { // A run reference under another name (runRef, runId, AnalysisRunRef, ...) would satisfy a // sweep that only recognizes "currentRun"/"extractionRun" literally, so this checks every // declared member's name for "run" as a substring, not a fixed set of spellings. Verified - // before writing this: none of the four carriers has a legitimate declared method, field, + // before writing this: none of the five carriers has a legitimate declared method, field, // or constructor parameter whose name contains "run" today, confirmed by a throwaway // reflection dump run against the built classes, so this sweep starts from a clean // baseline and any future match is either a reintroduced run reference or something that @@ -216,8 +194,12 @@ class IncrementalPropositionExtractionTest { // as arg0, arg1, ...), so a name-based check on a constructor parameter would silently // never fire; claiming otherwise here would be the same overclaim this test exists to // avoid making about other code. + // ExtractionRequest is swept alongside the rest: it is where a run reference would land + // once the store that consumes it arrives, so it is the likeliest place for one to + // reappear early. val carriers = listOf( IncrementalPropositionExtraction::class.java, + ExtractionRequest::class.java, SourceAnalysisContext::class.java, SourceAnalysisRequestEvent::class.java, ConversationAnalysisRequestEvent::class.java, @@ -246,13 +228,13 @@ class IncrementalPropositionExtractionTest { } @Test - fun `the entry point signatures that were overridable before profiles still are`() { + fun `the entry point signatures that were overridable before requests still are`() { // @JvmOverloads emits every reduced-arity overload as final. Folding the new argument // into the existing declarations would therefore have turned each method's pre-change // maximum arity — the signature a subclass overrides — into a final bridge. Callers // would not have noticed; a subclass would have stopped compiling, and one already - // compiled could fail verification at load. Each shape is its own declaration instead, - // and this is the assertion that keeps it that way. + // compiled could fail verification at load. Each shape is its own declaration, and this + // is the assertion that keeps it that way. val stillOpen = mapOf( "rememberText" to listOf( String::class.java, @@ -267,23 +249,6 @@ class IncrementalPropositionExtractionTest { String::class.java, NamedEntity::class.java, ), - "rememberTextFromSource" to listOf( - String::class.java, - String::class.java, - NamedEntity::class.java, - SourceLocator::class.java, - SourceRevisionRef::class.java, - List::class.java, - ExtractionPerspective::class.java, - Boolean::class.javaObjectType, - ), - "rememberFileFromSource" to listOf( - InputStream::class.java, - String::class.java, - NamedEntity::class.java, - SourceLocator::class.java, - SourceRevisionRef::class.java, - ), ) stillOpen.forEach { (name, parameters) -> val method = IncrementalPropositionExtraction::class.java @@ -294,40 +259,40 @@ class IncrementalPropositionExtractionTest { ) } - // The new maximum-arity forms are the single override point every call funnels through, - // so they have to be open too. - val profileOnly = arrayOf( - ExtractionContentProfileRef::class.java, + // The request-taking forms are the single override point every call funnels through, so + // they have to be open too. + val requestOnly = arrayOf( + ExtractionRequest::class.java, ) stillOpen.forEach { (name, parameters) -> val method = IncrementalPropositionExtraction::class.java - .getMethod(name, *(parameters.toTypedArray() + profileOnly)) + .getMethod(name, *(parameters.toTypedArray() + requestOnly)) assertFalse( Modifier.isFinal(method.modifiers), - "the profile-aware $name must be overridable", + "the request-taking $name must be overridable", ) } // The reduced-arity overloads @JvmOverloads generates were final before this slice and - // still are. Stating it pins that the fix restored the previous surface exactly rather - // than widening it. + // still are. Stating it pins that the surface is the one that was there, plus a request. val generatedBridges = listOf( "rememberText" to arrayOf>( String::class.java, String::class.java, NamedEntity::class.java, ), - "rememberTextFromSource" to arrayOf>( + "rememberText" to arrayOf>( String::class.java, String::class.java, NamedEntity::class.java, - SourceLocator::class.java, + List::class.java, ), - "rememberFileFromSource" to arrayOf>( - InputStream::class.java, + "rememberText" to arrayOf>( + String::class.java, String::class.java, NamedEntity::class.java, - SourceLocator::class.java, + List::class.java, + ExtractionPerspective::class.java, ), ) generatedBridges.forEach { (name, parameters) -> @@ -340,7 +305,7 @@ class IncrementalPropositionExtractionTest { } @Test - fun `a subclass overriding the pre-profile signatures still intercepts every call`() { + fun `a subclass overriding the signatures that predate requests still intercepts every call`() { val pipeline = pipelineReturningNoResult() val seen = mutableListOf() val extraction = object : IncrementalPropositionExtraction( @@ -386,12 +351,12 @@ class IncrementalPropositionExtractionTest { } @Test - fun `a subclass overriding only the pre-profile text methods still intercepts file ingestion`() { - // Before profiles, the file entry points read the file and handed the text to the - // pre-profile text methods, so a subclass overriding only those intercepted file - // ingestion too. Routing the file paths to the wide text methods would have quietly - // taken that away: the override would still compile, still be called for direct text - // calls, and silently stop seeing files. + fun `a subclass overriding only the text entry points still intercepts file ingestion`() { + // Before requests, the file entry point read the file and handed the text to the + // six-argument text method, so a subclass overriding only that one intercepted file + // ingestion too. Routing every file call to the request-taking text method would have + // quietly taken that away: the override would still compile, still be called for direct + // text calls, and silently stop seeing files. val pipeline = pipelineReturningNoResult() val seen = mutableListOf() val extraction = object : IncrementalPropositionExtraction( @@ -416,17 +381,16 @@ class IncrementalPropositionExtractionTest { seen += "text:$sourceId" } - override fun rememberTextFromSource( + override fun rememberText( text: String, sourceId: String, user: NamedEntity, - sourceLocator: SourceLocator, - sourceRevision: SourceRevisionRef?, additionalGrounding: List, perspective: ExtractionPerspective?, mintNewEntities: Boolean?, + request: ExtractionRequest, ) { - seen += "textFromSource:$sourceId" + seen += "request:$sourceId" } } val locator = UriLocator("file:///notes/dispatch.txt") @@ -437,35 +401,25 @@ class IncrementalPropositionExtractionTest { "legacy.txt", user(), ) - // A wide call carrying no reference dispatches like the pre-profile call it resembles. + // A request that carries nothing dispatches like the call it resembles. extraction.rememberFile( ByteArrayInputStream("legacy file text".toByteArray()), - "wide-null.txt", + "empty-request.txt", user(), - null, + ExtractionRequest.NONE, ) - extraction.rememberFileFromSource( + extraction.rememberFile( ByteArrayInputStream("source file text".toByteArray()), "source.txt", user(), - locator, - revision, - ) - extraction.rememberFileFromSource( - ByteArrayInputStream("source file text".toByteArray()), - "wide-null-source.txt", - user(), - locator, - revision, - null, + ExtractionRequest(sourceLocator = locator, sourceRevision = revision), ) assertEquals( listOf( "text:remember:legacy.txt", - "text:remember:wide-null.txt", - "textFromSource:remember:source.txt", - "textFromSource:remember:wide-null-source.txt", + "text:remember:empty-request.txt", + "request:remember:source.txt", ), seen, ) @@ -473,9 +427,9 @@ class IncrementalPropositionExtractionTest { } @Test - fun `a file call that carries a profile goes to the wide text entry point`() { - // The other half of the routing rule: once there is a reference to carry, the legacy - // text signature cannot express it, so the call has to go wide. + fun `a file call that carries a request goes to the request text entry point`() { + // The other half of the routing rule: once there is something to carry, the six-argument + // text signature cannot express it, so the call goes to the one that can. val pipeline = pipelineReturningNoResult() val seen = mutableListOf() val extraction = object : IncrementalPropositionExtraction( @@ -497,7 +451,7 @@ class IncrementalPropositionExtractionTest { perspective: ExtractionPerspective?, mintNewEntities: Boolean?, ) { - seen += "legacy:$sourceId" + seen += "text:$sourceId" } override fun rememberText( @@ -507,9 +461,9 @@ class IncrementalPropositionExtractionTest { additionalGrounding: List, perspective: ExtractionPerspective?, mintNewEntities: Boolean?, - profile: ExtractionContentProfileRef?, + request: ExtractionRequest, ) { - seen += "wide:$sourceId:${profile?.name}" + seen += "request:$sourceId:${request.profile?.name}" } } @@ -517,37 +471,42 @@ class IncrementalPropositionExtractionTest { ByteArrayInputStream("legacy file text".toByteArray()), "profiled.txt", user(), - ExtractionContentProfileRef("house-style", "v1"), + ExtractionRequest(profile = ExtractionContentProfileRef("house-style", "v1")), ) - assertEquals(listOf("wide:remember:profiled.txt:house-style"), seen) + assertEquals(listOf("request:remember:profiled.txt:house-style"), seen) verifyNoInteractions(pipeline) } @Test - fun `new text entry point retains exact typed and untyped inputs`() { + fun `the request text entry point retains exact typed and untyped inputs`() { val pipeline = pipelineReturningNoResult() val extraction = extraction(pipeline) val user = user() val locator = UriLocator("https://example.com/source") val revision = SourceRevisionRef(locator.key(), "r7") + val profile = ExtractionContentProfileRef("house-style", "v1") val perspective = mock() val grounding = listOf("record:one", "record:two") - extraction.rememberTextFromSource( + extraction.rememberText( text = "source text", sourceId = "caller:source:r7", user = user, - sourceLocator = locator, - sourceRevision = revision, additionalGrounding = grounding, perspective = perspective, mintNewEntities = true, + request = ExtractionRequest( + sourceLocator = locator, + sourceRevision = revision, + profile = profile, + ), ) val context = capturedContext(pipeline, "source text", "caller:source:r7", grounding) assertSame(locator, context.sourceLocator) assertSame(revision, context.sourceRevision) + assertSame(profile, context.profile) assertSame(perspective, context.perspective) assertEquals(true, context.mintNewEntities) } @@ -571,35 +530,24 @@ class IncrementalPropositionExtractionTest { } @Test - fun `mismatched typed provenance fails before pipeline invocation`() { + fun `mismatched typed provenance fails before the entry point is even called`() { + // The request validates the pairing while it is being built, so a text call and a file + // call are both unreachable with a mismatched pair — the caller never gets a request to + // pass. That is stricter than checking inside each entry point, and it is why neither + // entry point repeats the check. val pipeline = pipelineReturningNoResult() - val extraction = extraction(pipeline) - val locator = UriLocator("https://example.com/source") + val textLocator = UriLocator("https://example.com/source") + val fileLocator = UriLocator("file:///notes/example.txt") assertThrows(IllegalArgumentException::class.java) { - extraction.rememberTextFromSource( - text = "source text", - sourceId = "exact-source", - user = user(), - sourceLocator = locator, + ExtractionRequest( + sourceLocator = textLocator, sourceRevision = SourceRevisionRef("different-key", "r1"), ) } - verifyNoInteractions(pipeline) - } - - @Test - fun `mismatched file provenance fails before parsing or pipeline invocation`() { - val pipeline = pipelineReturningNoResult() - val extraction = extraction(pipeline) - val locator = UriLocator("file:///notes/example.txt") - assertThrows(IllegalArgumentException::class.java) { - extraction.rememberFileFromSource( - inputStream = ByteArrayInputStream("file source text".toByteArray()), - filename = "example.txt", - user = user(), - sourceLocator = locator, + ExtractionRequest( + sourceLocator = fileLocator, sourceRevision = SourceRevisionRef("different-key", "r1"), ) } @@ -631,18 +579,17 @@ class IncrementalPropositionExtractionTest { } @Test - fun `source aware file retains remember source id and typed provenance`() { + fun `a file request retains remember source id and typed provenance`() { val pipeline = pipelineReturningNoResult() val extraction = extraction(pipeline) val locator = UriLocator("file:///notes/example.txt") val revision = SourceRevisionRef(locator.key(), "file-r2") - extraction.rememberFileFromSource( + extraction.rememberFile( inputStream = ByteArrayInputStream("file source text".toByteArray()), filename = "example.txt", user = user(), - sourceLocator = locator, - sourceRevision = revision, + request = ExtractionRequest(sourceLocator = locator, sourceRevision = revision), ) val contextCaptor = argumentCaptor() @@ -676,75 +623,85 @@ class IncrementalPropositionExtractionTest { } @Test - fun `Kotlin callable references and named calls distinguish legacy and source entry points`() { + fun `Kotlin callable references and named calls resolve to one entry point each`() { val extraction = mock() val user = user() val locator = UriLocator("https://example.com/callable") val revision = SourceRevisionRef(locator.key(), "callable-r1") + val request = ExtractionRequest(sourceLocator = locator, sourceRevision = revision) val legacyText: (String, String, NamedEntity, List, ExtractionPerspective?, Boolean?) -> Unit = extraction::rememberText - val sourceText: + val requestText: ( String, String, NamedEntity, - SourceLocator, - SourceRevisionRef?, List, ExtractionPerspective?, Boolean?, - ) -> Unit = extraction::rememberTextFromSource + ExtractionRequest, + ) -> Unit = extraction::rememberText val legacyFile: (InputStream, String, NamedEntity) -> Unit = extraction::rememberFile - val sourceFile: - (InputStream, String, NamedEntity, SourceLocator, SourceRevisionRef?) -> Unit = - extraction::rememberFileFromSource + val requestFile: (InputStream, String, NamedEntity, ExtractionRequest) -> Unit = + extraction::rememberFile legacyText("callable legacy", "callable-legacy", user, emptyList(), null, null) - sourceText("callable source", "callable-source", user, locator, revision, emptyList(), null, null) + requestText("callable request", "callable-request", user, emptyList(), null, null, request) legacyFile(ByteArrayInputStream(byteArrayOf()), "callable-legacy.txt", user) - sourceFile(ByteArrayInputStream(byteArrayOf()), "callable-source.txt", user, locator, revision) + requestFile(ByteArrayInputStream(byteArrayOf()), "callable-request.txt", user, request) extraction.rememberText(text = "named legacy", sourceId = "named-legacy", user = user) - extraction.rememberTextFromSource( - text = "named source", - sourceId = "named-source", + extraction.rememberText( + text = "named request", + sourceId = "named-request", user = user, - sourceLocator = locator, + additionalGrounding = emptyList(), + perspective = null, + mintNewEntities = null, + request = request, ) extraction.rememberFile( inputStream = ByteArrayInputStream(byteArrayOf()), filename = "named-legacy.txt", user = user, ) - extraction.rememberFileFromSource( + extraction.rememberFile( inputStream = ByteArrayInputStream(byteArrayOf()), - filename = "named-source.txt", + filename = "named-request.txt", user = user, - sourceLocator = locator, + request = request, ) verify(extraction).rememberText("callable legacy", "callable-legacy", user, emptyList(), null, null) - verify(extraction).rememberTextFromSource( - "callable source", - "callable-source", + verify(extraction).rememberText( + "callable request", + "callable-request", user, - locator, - revision, emptyList(), null, null, + request, ) verify(extraction).rememberText("named legacy", "named-legacy", user) - verify(extraction).rememberTextFromSource("named source", "named-source", user, locator) + verify(extraction).rememberText( + "named request", + "named-request", + user, + emptyList(), + null, + null, + request, + ) } @Test - fun `legacy and source file entry points dispatch through their open text entry points`() { + fun `both file entry points dispatch through an open text entry point`() { val pipeline = pipelineReturningNoResult() val extraction = spy(extraction(pipeline)) val user = user() val locator = UriLocator("file:///notes/dispatch.txt") val revision = SourceRevisionRef(locator.key(), "dispatch-r1") + val request = ExtractionRequest(sourceLocator = locator, sourceRevision = revision) doNothing().whenever(extraction).rememberText( any(), any(), @@ -752,18 +709,15 @@ class IncrementalPropositionExtractionTest { any(), anyOrNull(), anyOrNull(), - anyOrNull(), ) - doNothing().whenever(extraction).rememberTextFromSource( - any(), + doNothing().whenever(extraction).rememberText( any(), any(), any(), - anyOrNull(), any(), anyOrNull(), anyOrNull(), - anyOrNull(), + any(), ) extraction.rememberFile( @@ -771,16 +725,16 @@ class IncrementalPropositionExtractionTest { "legacy-dispatch.txt", user, ) - extraction.rememberFileFromSource( - ByteArrayInputStream("source dispatch".toByteArray()), - "source-dispatch.txt", + extraction.rememberFile( + ByteArrayInputStream("request dispatch".toByteArray()), + "request-dispatch.txt", user, - locator, - revision, + request, ) - // Both file entry points land on the maximum-arity text method, which is the single - // override point every call funnels through. + // A file call carrying nothing lands on the six-argument text method, the one a subclass + // written before requests overrides. A file call carrying a request lands on the + // request-taking method, the single override point every call funnels through. verify(extraction).rememberText( "legacy dispatch", "remember:legacy-dispatch.txt", @@ -788,18 +742,15 @@ class IncrementalPropositionExtractionTest { emptyList(), null, null, - null, ) - verify(extraction).rememberTextFromSource( - "source dispatch", - "remember:source-dispatch.txt", + verify(extraction).rememberText( + "request dispatch", + "remember:request-dispatch.txt", user, - locator, - revision, emptyList(), null, null, - null, + request, ) verifyNoInteractions(pipeline) } @@ -887,7 +838,7 @@ class IncrementalPropositionExtractionTest { } @Test - fun `profile reaches the context through both text entry points`() { + fun `a request carries profile and revision to the context with and without a source`() { val pipeline = pipelineReturningNoResult() val extraction = extraction(pipeline) val user = user() @@ -896,59 +847,67 @@ class IncrementalPropositionExtractionTest { val profile = ExtractionContentProfileRef("house-style", "v1") extraction.rememberText( - text = "legacy text", - sourceId = "legacy:profiled", + text = "untyped text", + sourceId = "untyped:profiled", user = user, additionalGrounding = emptyList(), perspective = null, mintNewEntities = null, - profile = profile, + request = ExtractionRequest(profile = profile), ) - extraction.rememberTextFromSource( + extraction.rememberText( text = "source text", sourceId = "source:profiled", user = user, - sourceLocator = locator, - sourceRevision = revision, additionalGrounding = emptyList(), perspective = null, mintNewEntities = null, - profile = profile, + request = ExtractionRequest( + sourceLocator = locator, + sourceRevision = revision, + profile = profile, + ), ) - val fromLegacy = capturedContext(pipeline, "legacy text", "legacy:profiled", emptyList()) - assertSame(profile, fromLegacy.profile) - assertNull(fromLegacy.sourceLocator) + // A profile needs no source of its own: the two dimensions stay independent on the way in. + val fromUntyped = capturedContext(pipeline, "untyped text", "untyped:profiled", emptyList()) + assertSame(profile, fromUntyped.profile) + assertNull(fromUntyped.sourceLocator) + assertNull(fromUntyped.sourceRevision) val fromSource = capturedContext(pipeline, "source text", "source:profiled", emptyList()) assertSame(profile, fromSource.profile) + assertSame(locator, fromSource.sourceLocator) assertSame(revision, fromSource.sourceRevision) } @Test - fun `profile reaches the context through both file entry points`() { + fun `a request reaches the context through the file entry point`() { val pipeline = pipelineReturningNoResult() val extraction = extraction(pipeline) val user = user() val locator = UriLocator("file:///notes/profiled.txt") + val revision = SourceRevisionRef(locator.key(), "r1") val profile = ExtractionContentProfileRef("house-style", "v1") extraction.rememberFile( - inputStream = ByteArrayInputStream("legacy file text".toByteArray()), - filename = "legacy-profiled.txt", + inputStream = ByteArrayInputStream("plain file text".toByteArray()), + filename = "profile-only.txt", user = user, - profile = profile, + request = ExtractionRequest(profile = profile), ) - extraction.rememberFileFromSource( + extraction.rememberFile( inputStream = ByteArrayInputStream("source file text".toByteArray()), filename = "source-profiled.txt", user = user, - sourceLocator = locator, - sourceRevision = SourceRevisionRef(locator.key(), "r1"), - profile = profile, + request = ExtractionRequest( + sourceLocator = locator, + sourceRevision = revision, + profile = profile, + ), ) - // Both file calls land on the profile-aware text entry point, so the two contexts the - // pipeline sees are the proof that carriage survives the file hop. + // Both file calls hand their text to the request-taking entry point, so the two contexts + // the pipeline sees are the proof that carriage survives the file hop. val contextCaptor = argumentCaptor() verify(pipeline, times(2)).processOnce( @@ -962,10 +921,13 @@ class IncrementalPropositionExtractionTest { contextCaptor.allValues.forEach { context -> assertSame(profile, context.profile) } + val fromSource = contextCaptor.allValues.single { it.sourceLocator != null } + assertSame(locator, fromSource.sourceLocator) + assertSame(revision, fromSource.sourceRevision) } @Test - fun `legacy calls carry no profile`() { + fun `calls written without a request carry no profile`() { val pipeline = pipelineReturningNoResult() val extraction = extraction(pipeline) val user = user() @@ -1014,7 +976,7 @@ class IncrementalPropositionExtractionTest { additionalGrounding = grounding, perspective = ExtractionPerspective.USER, mintNewEntities = true, - profile = profile, + request = ExtractionRequest(profile = profile), ) val contextCaptor = argumentCaptor() @@ -1065,6 +1027,56 @@ class IncrementalPropositionExtractionTest { assertNull(contextCaptor.firstValue.sourceLocator) } + @Test + fun `an empty request asks for exactly what a call without one asks for`() { + val pipeline = pipelineReturningNoResult() + val extraction = extraction(pipeline) + val user = user() + val grounding = listOf("record:one") + + extraction.rememberText( + text = "same text", + sourceId = "same:id", + user = user, + additionalGrounding = grounding, + perspective = ExtractionPerspective.USER, + mintNewEntities = true, + ) + extraction.rememberText( + text = "same text", + sourceId = "same:id", + user = user, + additionalGrounding = grounding, + perspective = ExtractionPerspective.USER, + mintNewEntities = true, + request = ExtractionRequest.NONE, + ) + + val contextCaptor = argumentCaptor() + verify(pipeline, times(2)).processOnce( + eq("same text"), + eq("same:id"), + contextCaptor.capture(), + anyOrNull(), + any(), + eq(grounding), + ) + val (withoutRequest, withEmptyRequest) = contextCaptor.allValues + + // Whole contexts again. The resolver is substituted because buildContext constructs a + // fresh one per call by design, so it is never the same instance twice. + assertEquals( + withoutRequest, + withEmptyRequest.copy(entityResolver = withoutRequest.entityResolver), + ) + } + + private fun declaredParameterLists(name: String): Set>> = + IncrementalPropositionExtraction::class.java.declaredMethods + .filter { it.name == name && !it.isSynthetic } + .map { it.parameterTypes.toList() } + .toSet() + private fun event( sourceId: String, locator: SourceLocator, diff --git a/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/PreRequestEntryPointPinTest.kt b/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/PreRequestEntryPointPinTest.kt new file mode 100644 index 00000000..a855682e --- /dev/null +++ b/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/PreRequestEntryPointPinTest.kt @@ -0,0 +1,173 @@ +/* + * 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.proposition.PropositionRepository +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Test +import org.mockito.kotlin.mock +import org.mockito.kotlin.verifyNoInteractions +import org.mockito.kotlin.whenever +import java.io.ByteArrayInputStream +import java.io.InputStream + +/** + * The extraction surface as it stands on `main`, pinned twice over. + * + * `main` declares exactly two entry points: `rememberFile(InputStream, String, NamedEntity)` and + * `@JvmOverloads rememberText(text, sourceId, user, additionalGrounding = emptyList(), + * perspective = null, mintNewEntities = null)`. This file calls every shape a caller could have + * written against those two, and overrides both from a subclass — so it stops compiling if either + * signature moves, and fails if a call stops reaching the override. The Java half of the same + * claim lives in `SourceRevisionJavaInteropTest` and `ExtractionProfileJavaInteropTest`. + */ +class PreRequestEntryPointPinTest { + + @Test + fun `every call a caller could write before requests still compiles and reaches the override`() { + val pipeline = mock() + val extraction = PreRequestSubclass(pipeline) + val user = user() + val input: InputStream = ByteArrayInputStream("file bytes".toByteArray()) + + // Positional, at every arity @JvmOverloads publishes. + extraction.rememberText("text", "three", user) + extraction.rememberText("text", "four", user, listOf("record:one")) + extraction.rememberText("text", "five", user, listOf("record:one"), ExtractionPerspective.USER) + extraction.rememberText( + "text", + "six", + user, + listOf("record:one"), + ExtractionPerspective.USER, + true, + ) + // Named, including out of order, which only resolves against the declared parameter names. + extraction.rememberText(text = "text", sourceId = "named", user = user) + extraction.rememberText( + sourceId = "named-full", + user = user, + text = "text", + mintNewEntities = null, + perspective = null, + additionalGrounding = emptyList(), + ) + extraction.rememberFile(input, "legacy.txt", user) + extraction.rememberFile(inputStream = input, filename = "named.txt", user = user) + + assertEquals( + listOf( + "text:three", + "text:four", + "text:five", + "text:six", + "text:named", + "text:named-full", + "file:legacy.txt", + "file:named.txt", + ), + extraction.seen, + ) + // Every one of those was intercepted, so nothing reached extraction proper. + verifyNoInteractions(pipeline) + } + + @Test + fun `the entry point descriptors are the ones main publishes, plus one request form each`() { + // Kotlin compiles the file entry points' lambdas into methods named `rememberFile$lambda$N` + // that reflection does not report as synthetic, so they are excluded by name. + val published = IncrementalPropositionExtraction::class.java.declaredMethods + .filter { it.name.startsWith("remember") && '$' !in it.name && !it.isSynthetic } + .map { it.name to it.parameterTypes.toList() } + .toSet() + + val textPrefix = listOf(String::class.java, String::class.java, NamedEntity::class.java) + val textFull = textPrefix + listOf( + List::class.java, + ExtractionPerspective::class.java, + Boolean::class.javaObjectType, + ) + val filePrefix = listOf(InputStream::class.java, String::class.java, NamedEntity::class.java) + assertEquals( + setOf( + "rememberText" to textPrefix, + "rememberText" to (textPrefix + List::class.java), + "rememberText" to (textPrefix + listOf(List::class.java, ExtractionPerspective::class.java)), + "rememberText" to textFull, + "rememberText" to (textFull + ExtractionRequest::class.java), + "rememberFile" to filePrefix, + "rememberFile" to (filePrefix + ExtractionRequest::class.java), + ), + published, + ) + } + + private fun user(): NamedEntity = + mock().also { user -> + whenever(user.id).thenReturn("user-1") + whenever(user.name).thenReturn("Test User") + } + + /** + * A subclass written before requests existed, overriding the two signatures that were open + * then. Compiling it is half the assertion; the recorded calls are the other half. + */ + private class PreRequestSubclass(pipeline: PropositionPipeline) : IncrementalPropositionExtraction( + propositionPipeline = pipeline, + chunkHistoryStore = mock(), + dataDictionary = mock(), + relations = Relations.empty(), + propositionRepository = mock(), + entityRepository = mock(), + entityResolver = mock(), + graphProjectionService = mock(), + properties = mock().also { properties -> + whenever(properties.windowSize).thenReturn(1) + whenever(properties.overlapSize).thenReturn(1) + whenever(properties.triggerInterval).thenReturn(1) + }, + ) { + + val seen = mutableListOf() + + override fun rememberText( + text: String, + sourceId: String, + user: NamedEntity, + additionalGrounding: List, + perspective: ExtractionPerspective?, + mintNewEntities: Boolean?, + ) { + seen += "text:$sourceId" + } + + override fun rememberFile( + inputStream: InputStream, + filename: String, + user: NamedEntity, + ) { + seen += "file:$filename" + } + } +} diff --git a/docs/design/extraction-profiles.md b/docs/design/extraction-profiles.md index f656fd46..67c679c8 100644 --- a/docs/design/extraction-profiles.md +++ b/docs/design/extraction-profiles.md @@ -5,8 +5,8 @@ kind of material do?" DICE carries its name and version and nothing else. It nev profile up, never reads policy out of it, and never routes on it. The host owns the catalog, authorizes who may use which profile, and binds it to whatever it actually means. -This note covers DICE #66: `ExtractionContentProfileRef`, where it sits on -`SourceAnalysisContext` and the extraction entry points, and why profile, perspective, schema and +This note covers DICE #66: `ExtractionContentProfileRef`, the `ExtractionRequest` that carries it +to `SourceAnalysisContext` through the extraction entry points, and why profile, perspective, schema and tenant are four independent dimensions rather than one knob with four names. An extraction run reference travelled with this slice for one PR round and was pulled back out on review — see [Why there is no run reference here](#why-there-is-no-run-reference-here) below. @@ -96,7 +96,7 @@ for the same reason: parallel code in two places is where two paths start behavi ```mermaid flowchart TD - A["rememberText / rememberTextFromSource
rememberFile / rememberFileFromSource"] --> C + A["rememberText / rememberFile
with an ExtractionRequest"] --> C B["SourceAnalysisRequestEvent.profile()"] --> C C["IncrementalPropositionExtraction.buildContext"] --> CTX CTX["SourceAnalysisContext.profile"] --> X["carried, never consulted"] @@ -104,70 +104,91 @@ flowchart TD | Path | How a profile arrives | | --- | --- | -| `rememberText`, `rememberTextFromSource` | a trailing optional `profile` argument | -| `rememberFile`, `rememberFileFromSource` | the same argument, forwarded to the text call | -| async `SourceAnalysisRequestEvent` | `profile()`, open and null-defaulted | +| `rememberText` | `ExtractionRequest.profile`, on the one overload that takes a request | +| `rememberFile` | the same request, forwarded to the text call | +| async `SourceAnalysisRequestEvent` | `profile()`, open and null-defaulted, which the listener puts on a request | | `ConversationAnalysisRequestEvent` | its longer constructor, whose `sourceLocator` is nullable so a publisher can name a profile for material it has no typed source for | -The direct calls take an extra argument rather than getting their own method name, which is the -opposite of what Wave A did for `rememberTextFromSource`. The reason is the difference in the -contracts: a locator is *required* by the source-aware calls, so those are genuinely different -methods and separate names keep every call site unambiguous. A profile is optional everywhere, so -an extra name would buy nothing and double the surface. +### Why the request object exists + +A profile is the third dimension to reach these entry points, after a source locator and a source +revision, and it will not be the last: an extraction run reference, a pass index and a products +list are all named by later slices. Adding each one as an argument means a wider signature and a +new overload to keep the old shape callable, on every entry-point name — surface that grows once +per feature and that a host has to track. + +`ExtractionRequest` holds them together: + +```kotlin +data class ExtractionRequest @JvmOverloads constructor( + val sourceLocator: SourceLocator? = null, + val sourceRevision: SourceRevisionRef? = null, + val profile: ExtractionContentProfileRef? = null, +) +``` + +The next dimension is a field here. `rememberText` and `rememberFile` keep the signatures they +have, a host that overrides one keeps compiling, and the new value shows up in the override with +no edit. `ExtractionRequest.NONE` is the empty request, and `isEmpty` compares against it, so the +"carries nothing" test stays right as fields are added. + +The one coupling lives in the `init` block: a `sourceRevision` requires a `sourceLocator` whose key +it matches. Checking it here means a caller finds out while building the request, before any entry +point is called — a mismatched pair cannot be passed to extraction, because it cannot be built. +`SourceAnalysisContext` repeats the same check for its own field, which covers the async path and +the REST path; the two agree because they state the same rule. ### Why each entry point is two declarations Growing the existing declarations with a defaulted parameter would have been the obvious shape, and it is wrong. `@JvmOverloads` emits every reduced-arity overload as `final`, even on an `open` -function — only the declared maximum arity stays open. So adding `profile` to `rememberText` +function — only the declared maximum arity stays open. So adding the request to `rememberText` would have moved the open declaration from six arguments to seven and re-emitted the six-argument form as a final bridge. Callers would not have noticed. A subclass overriding the six-argument form would have stopped compiling, and one already compiled could fail verification at class load. -Each entry point is therefore two declarations: the pre-profile signature exactly as it was -(`@JvmOverloads` where it already had it), delegating to a new maximum-arity form that takes the -reference and carries no default. The default is what forces the split — two overloads that both -supply defaults for the same arity are ambiguous at a Kotlin call site, so the wide form spells -every argument out. +Each entry point is therefore two declarations: the signature that was already there, exactly as +it was (`@JvmOverloads` where it already had it), delegating to a form that takes the request and +carries no default. The default is what forces the split — two overloads that both supply defaults +for the same arity are ambiguous at a Kotlin call site, so the request form spells every argument +out. | Method | Overridable before | Overridable now | | --- | --- | --- | | `rememberText` | 6 args | 6 args, and 7 | -| `rememberTextFromSource` | 8 args | 8 args, and 9 | | `rememberFile` | 3 args | 3 args, and 4 | -| `rememberFileFromSource` | 5 args | 5 args, and 6 | -The reduced arities `@JvmOverloads` generates were final before and still are — the fix restores -the previous surface rather than widening it. +The reduced arities `@JvmOverloads` generates were final before and still are — this is the +previous surface with one overload added per name. ### Which chain a call takes Keeping the old signatures overridable is only half of it. They also have to still be *reached*. -Before profiles, `rememberFile` read the file and handed the text to the six-argument +Before requests, `rememberFile` read the file and handed the text to the six-argument `rememberText`, so a subclass overriding only that one intercepted file ingestion as well. -Routing the file paths straight to the wide text methods would have taken that away silently: the -override would still compile, still fire for direct text calls, and stop seeing files. +Routing every file call straight to the request-taking text method would have taken that away +silently: the override would still compile, still fire for direct text calls, and stop seeing +files. -So the rule is that a call dispatches like the call it resembles. A file call carrying no profile -takes the pre-profile chain; one carrying a profile has to go wide, because the legacy text -signature cannot express it. +So the rule is that a call dispatches like the call it resembles. A file call carrying an empty +request takes the older chain; one carrying anything at all goes to the request form, because the +six-argument text signature cannot express it. ```mermaid flowchart LR F3["rememberFile(3)"] --> T6 - F4["rememberFile(4)"] -->|"profile null"| F3 - F4 -->|"profile present"| T7 - T6["rememberText(6)"] --> T7["rememberText(7)"] --> I["rememberTextInternal"] + F4["rememberFile(4)"] -->|"request empty"| F3 + F4 -->|"request carries something"| T7 + T6["rememberText(6)"] --> T7["rememberText(7), terminal"] ``` -`rememberFileFromSource` and `rememberTextFromSource` mirror it at their own arities. The wide -text forms are terminal and never route back to the legacy ones — that would be a cycle, since -the legacy forms delegate forwards. +The request text form is terminal and never routes back to the six-argument one — that would be a +cycle, since the six-argument form delegates forwards. -Two consequences worth stating. A subclass overriding only a pre-profile text method sees -everything it used to, files included. And unintercepted, every call still ends at the wide text -form, so a host that wants one place to see all traffic overrides that. +Two consequences worth stating. A subclass overriding only the six-argument text method sees +everything it used to, files included. And unintercepted, every call still ends at the request +text form, so a host that wants one place to see all traffic overrides that. The async path reads the accessor exactly once, which a test asserts by counting: there is one `buildContext` call and nowhere else for a second read to happen. @@ -180,21 +201,22 @@ The async path reads the accessor exactly once, which a test asserts by counting is still published. `@JvmOverloads` adds one new one on the end. A test enumerates arities 3 through 12 (each with the trailing `DefaultConstructorMarker` Kotlin emits because `contextId` is a value class) and asserts every one resolves. -- Every `rememberText`, `rememberTextFromSource`, `rememberFile` and `rememberFileFromSource` - descriptor survives, and the new argument adds exactly one descriptor per method name, on the - end. A test asserts the exact descriptor set of all four names, and that a profile is always the - last parameter. +- Every `rememberText` and `rememberFile` descriptor survives, and the request adds exactly one + descriptor per method name, on the end. A test asserts the exact descriptor set of both names, + that the request is always the last parameter, and that no entry point takes a locator, a + revision or a profile as a loose argument — the last of those is what keeps the surface from + growing back one argument at a time. - **Subclass-override compatibility is claimed, and covers being reached as well as being - overridable.** Every signature that was overridable before this slice still is, and every - pre-profile call still dispatches through it. A reflection test asserts `Modifier.isFinal` is - false on all four pre-profile signatures and on the four new maximum-arity forms, and true on - the reduced arities that were final bridges already. A Java subclass in the compat suite - overrides all four pre-profile signatures; `javac` rejects `@Override` on a final method, so the - suite compiling is the second proof. A Kotlin test constructs a subclass overriding the - six-argument `rememberText` and the three-argument `rememberFile` and asserts a three-argument - call still reaches the override. Overridability alone is not enough, so two further tests pin - the dispatch: a subclass overriding only the pre-profile *text* methods still sees both file - entry points, and a file call that carries a profile goes to the wide text form instead. + overridable.** Every signature that was overridable before this slice still is, and every call + written before it still dispatches through one. A reflection test asserts `Modifier.isFinal` is + false on both earlier signatures and on the two request forms, and true on the reduced arities + that were final bridges already. A Java subclass in the compat suite overrides both earlier + signatures; `javac` rejects `@Override` on a final method, so the suite compiling is the second + proof. `PreRequestEntryPointPinTest` calls every shape a caller could have written against those + two signatures — every published arity, positional and named — from a subclass overriding both, + and asserts each call reaches the override. Overridability alone is not enough, so two further + tests pin the dispatch: a subclass overriding only the *text* entry points still sees file + ingestion, and a file call that carries a request goes to the request text form. - `ConversationAnalysisRequestEvent` keeps its five-argument constructor and gains a six-argument form. Its `sourceLocator` parameter relaxes from non-null to nullable, which accepts strictly more calls than before. @@ -232,9 +254,10 @@ through the identical extension point, `PropositionExtractor.extract(chunk, cont `PropositionPipeline.withExtractor` seeds a pipeline with a host-implemented `PropositionExtractor`, and `extract` receives the whole context — `currentRun` included, back when it existed. No extractor DICE ships reads `context.profile` today, and nothing else downstream of `buildContext` -does either (`IncrementalPropositionExtraction.kt:612`); the test `profile reaches the context -through both text entry points` (and its file-entry counterpart) pins that the context reaching -the pipeline carries `profile`, which is the reachability half of the claim, and stops there — it +does either (`IncrementalPropositionExtraction.kt:457`); the test `a request carries profile and +revision to the context with and without a source` (and its file-entry counterpart) pins that the +context reaching the pipeline carries `profile`, which is the reachability half of the claim, and +stops there — it does not exercise any reading of the value. The asymmetry is about what a host-authored `PropositionExtractor` has to act on once it does read the field. `profile` names the host's own content-policy identity, something a host already knows the meaning of and can build a reader for @@ -256,9 +279,11 @@ in this note reflects a profile-only surface for that reason. Nothing on this branch called the parameter for anything (`grep` across this worktree found no caller of `currentRun`/`ExtractionRunRef` outside the profile slice's own code and tests), so -there is nothing to migrate. The reference returns, unchanged in shape, once #67's store and a -write that consumes it land together — at that point a run reference reaching -`SourceAnalysisContext` will have somewhere to go. +there is nothing to migrate. The reference returns once #67's store and a write that consumes it +land together — at that point a run reference reaching `SourceAnalysisContext` will have somewhere +to go. It returns as a field on `ExtractionRequest`, which is what the request object is here for: +no entry-point signature moves when it arrives, and a host overriding one sees the new value with +no edit. ## What this slice does not do diff --git a/docs/design/source-revisions.md b/docs/design/source-revisions.md index 99fa97bb..43ed9a39 100644 --- a/docs/design/source-revisions.md +++ b/docs/design/source-revisions.md @@ -564,7 +564,7 @@ somebody else's revision — there is no revision to pick up. ```mermaid flowchart TD - A["rememberTextFromSource / rememberFileFromSource"] --> C + A["rememberText / rememberFile, with an ExtractionRequest"] --> C B["SourceAnalysisRequestEvent.sourceLocator() / sourceRevision()"] --> C R["POST /extract, POST /extract/file"] --> C2 C["IncrementalPropositionExtraction.buildContext"] --> CTX @@ -575,28 +575,28 @@ flowchart TD ### The three entry points -**Direct calls.** `rememberTextFromSource` and `rememberFileFromSource` sit beside `rememberText` -and `rememberFile` as separate methods. A locator is required on the source-aware pair and absent -from the legacy pair, so the two have genuinely different contracts and every Kotlin and Java call -site resolves to one of them without ambiguity. `IncrementalPropositionExtractionTest` enumerates -the exact JVM descriptors of all four names and asserts the sets, so a legacy descriptor cannot -quietly move and a source-aware call cannot collapse onto a legacy one; the same file exercises -Kotlin callable references, named arguments, and Mockito-shaped call sites for the same reason. -Both source-aware calls run through one private `rememberTextInternal`, and the file variants share -one `withRememberedFileText` reader, so parsing and grounding behave identically with or without a -revision. - -Passing a revision on either call is an assertion by the host that the locator's revision covers the -whole aggregate being extracted — the whole text, or the whole file as Tika read it. DICE has no way -to derive that: `sourceId` and `additionalGrounding` are untyped strings, and a file is read as one -document. The KDoc on both methods says so. +**Direct calls.** `rememberText` and `rememberFile` each take an `ExtractionRequest` on one extra +overload, and the locator and the revision travel on it. See +[extraction-profiles.md](extraction-profiles.md) for why the request exists and how the two +declarations per entry point keep every earlier call and override working. +`IncrementalPropositionExtractionTest` enumerates the exact JVM descriptors of both names and +asserts the sets, so an earlier descriptor cannot quietly move and a request-carrying call cannot +collapse onto one written without a request; the same file exercises Kotlin callable references, +named arguments, and Mockito-shaped call sites for the same reason. Both file entry points share +one `withRememberedFileText` reader and hand their text to a text entry point, so parsing and +grounding behave identically with or without a revision. + +Passing a revision is an assertion by the host that the locator's revision covers the whole +aggregate being extracted — the whole text, or the whole file as Tika read it. DICE has no way to +derive that: `sourceId` and `additionalGrounding` are untyped strings, and a file is read as one +document. The KDoc on `ExtractionRequest.sourceRevision` says so. **The async event path.** `SourceAnalysisRequestEvent` gains two open methods, `sourceLocator()` and `sourceRevision()`, both returning null by default, so an existing subclass carries no provenance and behaves as it did. `ConversationAnalysisRequestEvent` gains a constructor that takes a locator and an -optional revision. The listener passes whatever the event returns into the same `buildContext` call -`rememberTextFromSource` uses, which is what makes the two paths carry a revision identically rather -than similarly. Three tests pin it: `SourceAnalysisRequestEventRevisionTest` asserts the defaults, the +optional revision. The listener turns whatever the event returns into an `ExtractionRequest` and +passes it into the same `buildContext` call the direct entry points use, which is what makes the two +paths carry a revision identically. Three tests pin it: `SourceAnalysisRequestEventRevisionTest` asserts the defaults, the exact values a provenance-aware event carries, and that a mismatched key is rejected when the context is built; `IncrementalPropositionExtractionTest.event provenance reaches the context observed by the pipeline` runs a real event through `extractPropositions` and captures the context the pipeline was @@ -1029,7 +1029,7 @@ Four slices, each green on its own, together equal to the reviewed when a participant has gone, and `DrivineCollectorTraceStore` persists and reads the new field with rows written before it still readable. 4. **Entry points** — the whole write side above: `SourceAnalysisContext.sourceRevision` with its - locator-and-key invariant, `rememberTextFromSource` and `rememberFileFromSource`, the async + locator-and-key invariant, the `ExtractionRequest` the entry points take, the async `SourceAnalysisRequestEvent` path through the same `buildContext` call, the pipeline stamp, the REST request and response shapes with revision-stable chunk ids, and the binary-compatibility fixture running as a test rather than sitting in the tree.