diff --git a/CHANGELOG.md b/CHANGELOG.md index 0aba96c2..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 @@ -1078,3 +1076,109 @@ 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.** 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 + 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 + 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 + 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 + compares the whole context built with a profile against the one built without and asserts they + 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 + 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 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` 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 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 + compiled before still compiles. Full Kotlin synthetic `copy` and `componentN` ABI is **not** + 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. 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/ConversationAnalysisRequestEvent.kt b/dice/src/main/kotlin/com/embabel/dice/common/ConversationAnalysisRequestEvent.kt index 9b57f89f..fc3d9c21 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,7 @@ 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.provenance.SourceLocator import com.embabel.dice.provenance.SourceRevisionRef @@ -29,7 +30,14 @@ 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]. EXPERIMENTAL; see [ExtractionContentProfileRef] + * for what carrying it means and does not mean. + * + * [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, @@ -41,15 +49,20 @@ class ConversationAnalysisRequestEvent( private var eventSourceRevision: SourceRevisionRef? = null + private var eventProfile: ExtractionContentProfileRef? = null + + @JvmOverloads constructor( source: Any, user: NamedEntity, conversation: Conversation, - sourceLocator: SourceLocator, + sourceLocator: SourceLocator?, sourceRevision: SourceRevisionRef? = null, + profile: ExtractionContentProfileRef? = null, ) : this(source, user, conversation) { eventSourceLocator = sourceLocator eventSourceRevision = sourceRevision + eventProfile = profile } override fun incrementalSource(): IncrementalSource = @@ -58,4 +71,6 @@ class ConversationAnalysisRequestEvent( override fun sourceLocator(): SourceLocator? = eventSourceLocator override fun sourceRevision(): SourceRevisionRef? = eventSourceRevision + + override fun profile(): ExtractionContentProfileRef? = eventProfile } 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..322ad240 100644 --- a/dice/src/main/kotlin/com/embabel/dice/common/SourceAnalysisContext.kt +++ b/dice/src/main/kotlin/com/embabel/dice/common/SourceAnalysisContext.kt @@ -19,6 +19,7 @@ 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 /** @@ -39,6 +40,11 @@ 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 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 +71,7 @@ data class SourceAnalysisContext @JvmOverloads constructor( */ val mintedEntityProperties: Map = emptyMap(), val sourceRevision: SourceRevisionRef? = null, + val profile: ExtractionContentProfileRef? = null, ) { init { @@ -76,6 +83,10 @@ data class SourceAnalysisContext @JvmOverloads constructor( "sourceRevision source key must match sourceLocator source key" } } + // [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 { @@ -150,6 +161,13 @@ 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 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..46d37cd1 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,7 @@ 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.provenance.SourceLocator import com.embabel.dice.provenance.SourceRevisionRef import org.springframework.context.ApplicationEvent @@ -28,10 +29,14 @@ 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 + * second code path. It also defaults to null. */ abstract class SourceAnalysisRequestEvent( source: Any, @@ -51,4 +56,10 @@ 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 } 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/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 7a6300fa..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 @@ -163,41 +161,47 @@ 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 requests, kept as its own declaration so it + * stays overridable — see the note on [rememberText]. */ - open fun rememberFile(inputStream: InputStream, filename: String, user: NamedEntity) = - withRememberedFileText(inputStream, filename) { text -> - rememberText( - text = text, - sourceId = "remember:$filename", - user = user, - ) - } + open fun rememberFile( + inputStream: InputStream, + filename: String, + user: NamedEntity, + ) = withRememberedFileText(inputStream, filename) { text -> + // 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 and ground them in the caller's typed source. + * 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. * - * 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. + * 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. */ - @JvmOverloads - open fun rememberFileFromSource( + open fun rememberFile( inputStream: InputStream, filename: String, user: NamedEntity, - sourceLocator: SourceLocator, - sourceRevision: SourceRevisionRef? = null, + request: ExtractionRequest, ) { - require(sourceRevision == null || sourceRevision.sourceKey == sourceLocator.key()) { - "sourceRevision source key must match sourceLocator source key" + if (request.isEmpty) { + rememberFile(inputStream, filename, user) + return } withRememberedFileText(inputStream, filename) { text -> - rememberTextFromSource( - text = text, - sourceId = "remember:$filename", - user = user, - sourceLocator = sourceLocator, - sourceRevision = sourceRevision, + rememberText( + text, + "remember:$filename", + user, + emptyList(), + null, + null, + request, ) } } @@ -239,6 +243,16 @@ 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 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 + * subclass written before requests used to see, including file ingestion, which still + * routes through here. */ @JvmOverloads open fun rememberText( @@ -248,67 +262,44 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( additionalGrounding: List = emptyList(), perspective: ExtractionPerspective? = null, mintNewEntities: Boolean? = null, - ) = - rememberTextInternal( - text = text, - sourceId = sourceId, - user = user, - additionalGrounding = additionalGrounding, - perspective = perspective, - mintNewEntities = mintNewEntities, - ) + ) = rememberText( + text, + sourceId, + user, + additionalGrounding, + perspective, + mintNewEntities, + ExtractionRequest.NONE, + ) /** - * Extract propositions from raw text and ground them in the caller's typed source. + * 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. * - * 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. + * 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. 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. + * 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. */ - @JvmOverloads - open fun rememberTextFromSource( - text: String, - sourceId: String, - user: NamedEntity, - sourceLocator: SourceLocator, - sourceRevision: SourceRevisionRef? = null, - additionalGrounding: List = emptyList(), - perspective: ExtractionPerspective? = null, - mintNewEntities: Boolean? = null, - ) = - rememberTextInternal( - text = text, - sourceId = sourceId, - user = user, - sourceLocator = sourceLocator, - sourceRevision = sourceRevision, - additionalGrounding = additionalGrounding, - perspective = perspective, - mintNewEntities = mintNewEntities, - ) - - private fun rememberTextInternal( + open fun rememberText( text: String, sourceId: String, user: NamedEntity, - sourceLocator: SourceLocator? = null, - sourceRevision: SourceRevisionRef? = null, - additionalGrounding: List = emptyList(), - perspective: ExtractionPerspective? = null, - mintNewEntities: Boolean? = null, + additionalGrounding: List, + perspective: ExtractionPerspective?, + mintNewEntities: Boolean?, + request: ExtractionRequest, ) { val context = buildContext( user = user, sourceId = sourceId, perspective = perspective, mintNewEntities = mintNewEntities, - sourceLocator = sourceLocator, - sourceRevision = sourceRevision, + request = request, ) val result = propositionPipeline.processOnce( text, sourceId, context, additionalGrounding = additionalGrounding, @@ -356,13 +347,16 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( return } - // The async path grounds propositions exactly the way rememberTextFromSource does: - // whatever provenance 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(), + request = ExtractionRequest( + sourceLocator = event.sourceLocator(), + sourceRevision = event.sourceRevision(), + profile = event.profile(), + ), ) logger.info( "Context relations count: {}, injected relations count: {}", @@ -394,8 +388,7 @@ open class IncrementalPropositionExtraction @JvmOverloads constructor( sourceId: String = "", perspective: ExtractionPerspective? = null, mintNewEntities: Boolean? = null, - sourceLocator: SourceLocator? = null, - sourceRevision: SourceRevisionRef? = null, + request: ExtractionRequest = ExtractionRequest.NONE, ): SourceAnalysisContext { val aliases = try { currentUserAliasesProvider(user) @@ -455,12 +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. + 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 new file mode 100644 index 00000000..57cb31d5 --- /dev/null +++ b/dice/src/test/java/com/embabel/dice/ExtractionProfileJavaInteropTest.java @@ -0,0 +1,374 @@ +/* + * 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.ExtractionRequest; +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. 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 { + + private SourceAnalysisContext context() { + return SourceAnalysisContext + .withContextId("java-profile") + .withEntityResolver(AlwaysCreateEntityResolver.INSTANCE) + .withSchema(DataDictionary.fromClasses("java-profile")); + } + + @Test + void javaCallersBuildAndReadProfileValues() { + 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") + ); + } + + @Test + void javaBuiltContextsCarryNoProfileUnlessAsked() { + SourceAnalysisContext absent = context(); + assertNull(absent.getProfile()); + + ExtractionContentProfileRef profile = new ExtractionContentProfileRef("house-style", "v1"); + SourceAnalysisContext present = absent.withProfile(profile); + + assertSame(profile, present.getProfile()); + // The copy is a copy: the original is untouched. + assertNull(absent.getProfile()); + } + + @Test + void retainsEveryLegacyRememberDescriptorAndAddsRequestAwareOnes() 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, ExtractionRequest.class + ); + + IncrementalPropositionExtraction.class.getMethod( + "rememberFile", InputStream.class, String.class, NamedEntity.class + ); + IncrementalPropositionExtraction.class.getMethod( + "rememberFile", + InputStream.class, String.class, NamedEntity.class, ExtractionRequest.class + ); + + // 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( + "rememberText", + String.class, String.class, NamedEntity.class, + 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 + void legacyAndProfileAwareJavaEventSubclassesUseTheBaseConstructor() { + NamedEntity user = org.mockito.Mockito.mock(NamedEntity.class); + + LegacyJavaEvent legacy = new LegacyJavaEvent(this, user); + assertNull(legacy.profile()); + + ExtractionContentProfileRef profile = new ExtractionContentProfileRef("house-style", "v1"); + ProfileAwareJavaEvent profileAware = new ProfileAwareJavaEvent(this, user, profile); + assertSame(profile, profileAware.profile()); + // 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 + ); + // 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 + ); + 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 + ); + + 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"); + + 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()); + + ConversationAnalysisRequestEvent profiled = new ConversationAnalysisRequestEvent( + this, user, conversation, locator, revision, profile + ); + assertSame(profile, profiled.profile()); + + // 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 + ); + assertNull(profileOnly.sourceLocator()); + assertSame(profile, profileOnly.profile()); + } + + @Test + 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 request argument 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 requests 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. + } + } + + /** + * 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 + ) { + 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, + new ExtractionRequest(null, null, profile) + ); + extraction.rememberFile(input, "legacy.txt", user); + 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.rememberFile( + input, "source.txt", user, new ExtractionRequest(locator, revision, profile) + ); + } + + 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 ProfileAwareJavaEvent( + Object source, + NamedEntity user, + ExtractionContentProfileRef profile + ) { + super(source, user); + this.profile = profile; + } + + @Override + public IncrementalSource incrementalSource() { + throw new UnsupportedOperationException("Not needed by this compatibility test"); + } + + @Override + public ExtractionContentProfileRef profile() { + return 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/ExtractionProfileCompatibilityTest.kt b/dice/src/test/kotlin/com/embabel/dice/ExtractionProfileCompatibilityTest.kt new file mode 100644 index 00000000..9a2d1638 --- /dev/null +++ b/dice/src/test/kotlin/com/embabel/dice/ExtractionProfileCompatibilityTest.kt @@ -0,0 +1,171 @@ +/* + * 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.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, + ) = SourceAnalysisContext( + schema = DataDictionary.fromClasses("profile-compatibility"), + entityResolver = AlwaysCreateEntityResolver, + contextId = ContextId("profile-compatibility"), + profile = profile, + ) + + @Test + fun `legacy Kotlin source constructors and copy calls see no profile`() { + 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) + + // 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) + } + + @Test + fun `new Kotlin source constructors and copy calls carry a profile`() { + val profile = ExtractionContentProfileRef("house-style", "v3") + + val fromConstructor = context(profile = profile) + .copy(promptVariables = mapOf("profiled" to true)) + assertSame(profile, fromConstructor.profile) + assertEquals(true, fromConstructor.promptVariables["profiled"]) + + val fromHelpers = context().withProfile(profile) + assertSame(profile, fromHelpers.profile) + } + + @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"), + ) + + 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, + ) + 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 is what this slice adds, on the end. + assertTrue( + declared.take(12) + marker in published, + "constructor of 12 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(12), copyArities) + + val componentCount = SourceAnalysisContext::class.java.declaredMethods + .count { it.name.startsWith("component") } + assertEquals(12, 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..d056b430 --- /dev/null +++ b/dice/src/test/kotlin/com/embabel/dice/common/ExtractionContextIndependenceTest.kt @@ -0,0 +1,227 @@ +/* + * 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.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], + ) + + // 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)) + + 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, + ) + } + } +} 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..72ee2a70 --- /dev/null +++ b/dice/src/test/kotlin/com/embabel/dice/common/SourceAnalysisRequestEventProfileTest.kt @@ -0,0 +1,126 @@ +/* + * 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.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: 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") + + @Test + fun `a subclass written before profiles carries none`() { + val legacy = LegacyEvent(this, mock(NamedEntity::class.java)) + + assertNull(legacy.profile()) + assertNull(legacy.sourceLocator()) + assertNull(legacy.sourceRevision()) + } + + @Test + fun `the shipped conversation event defaults to no profile`() { + val event = ConversationAnalysisRequestEvent( + source = this, + user = mock(NamedEntity::class.java), + conversation = mock(Conversation::class.java), + ) + + assertNull(event.profile()) + } + + @Test + fun `the conversation event carries the exact profile 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, + ) + + assertSame(locator, event.sourceLocator()) + assertSame(revision, event.sourceRevision()) + assertSame(profile, event.profile()) + } + + @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()) + } + + @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.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..20e55e98 --- /dev/null +++ b/dice/src/test/kotlin/com/embabel/dice/proposition/extraction/ExtractionProfileContractTest.kt @@ -0,0 +1,90 @@ +/* + * 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 new reference type promises: stable name-and-version identity for a profile, a + * bounded string, and no interpretation of either component. + */ +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") + } +} 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 7967a23c..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 @@ -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 @@ -39,6 +40,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,113 +54,459 @@ 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 { @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 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, NamedEntity::class.java, ) + val legacyTextFull = legacyTextPrefix + listOf( + List::class.java, + ExtractionPerspective::class.java, + Boolean::class.javaObjectType, + ) + val requestOnly = listOf( + ExtractionRequest::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 + requestOnly, ), rememberTextParameters, ) - val sourceTextPrefix = legacyTextPrefix + SourceLocator::class.java - 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, - ), - sourceTextPrefix + listOf( - SourceRevisionRef::class.java, - List::class.java, - ExtractionPerspective::class.java, - Boolean::class.javaObjectType, - ), - ), - rememberTextFromSourceParameters, - ) - val legacyFile = listOf( InputStream::class.java, String::class.java, NamedEntity::class.java, ) - assertEquals(setOf(legacyFile), rememberFileParameters) assertEquals( - setOf( - legacyFile + SourceLocator::class.java, - legacyFile + listOf(SourceLocator::class.java, SourceRevisionRef::class.java), + setOf(legacyFile, legacyFile + requestOnly), + rememberFileParameters, + ) + + // 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( + requestOnly, + parameters.takeLast(1), + "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. + // 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 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 + // 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. + // 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, + ) + 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", + ) + } + } + } + + @Test + 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, 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, + ), + ) + 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 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() + requestOnly)) + assertFalse( + Modifier.isFinal(method.modifiers), + "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 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, + ), + "rememberText" to arrayOf>( + String::class.java, + String::class.java, + NamedEntity::class.java, + List::class.java, + ), + "rememberText" to arrayOf>( + String::class.java, + String::class.java, + NamedEntity::class.java, + List::class.java, + ExtractionPerspective::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 signatures that predate requests 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 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( + 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 rememberText( + text: String, + sourceId: String, + user: NamedEntity, + additionalGrounding: List, + perspective: ExtractionPerspective?, + mintNewEntities: Boolean?, + request: ExtractionRequest, + ) { + seen += "request:$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 request that carries nothing dispatches like the call it resembles. + extraction.rememberFile( + ByteArrayInputStream("legacy file text".toByteArray()), + "empty-request.txt", + user(), + ExtractionRequest.NONE, + ) + extraction.rememberFile( + ByteArrayInputStream("source file text".toByteArray()), + "source.txt", + user(), + ExtractionRequest(sourceLocator = locator, sourceRevision = revision), + ) + + assertEquals( + listOf( + "text:remember:legacy.txt", + "text:remember:empty-request.txt", + "request:remember:source.txt", ), - rememberFileFromSourceParameters, + seen, ) + verifyNoInteractions(pipeline) } @Test - fun `new text entry point retains exact typed and untyped inputs`() { + 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( + 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 rememberText( + text: String, + sourceId: String, + user: NamedEntity, + additionalGrounding: List, + perspective: ExtractionPerspective?, + mintNewEntities: Boolean?, + request: ExtractionRequest, + ) { + seen += "request:$sourceId:${request.profile?.name}" + } + } + + extraction.rememberFile( + ByteArrayInputStream("legacy file text".toByteArray()), + "profiled.txt", + user(), + ExtractionRequest(profile = ExtractionContentProfileRef("house-style", "v1")), + ) + + assertEquals(listOf("request:remember:profiled.txt:house-style"), seen) + verifyNoInteractions(pipeline) + } + + @Test + 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) } @@ -182,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"), ) } @@ -242,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() @@ -287,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(), @@ -364,15 +710,14 @@ class IncrementalPropositionExtractionTest { anyOrNull(), anyOrNull(), ) - doNothing().whenever(extraction).rememberTextFromSource( + doNothing().whenever(extraction).rememberText( any(), any(), any(), any(), anyOrNull(), - any(), - anyOrNull(), anyOrNull(), + any(), ) extraction.rememberFile( @@ -380,25 +725,33 @@ 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, ) - verify(extraction).rememberText("legacy dispatch", "remember:legacy-dispatch.txt", user) - verify(extraction).rememberTextFromSource( - "source dispatch", - "remember:source-dispatch.txt", + // 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", user, - locator, - revision, emptyList(), null, null, ) + verify(extraction).rememberText( + "request dispatch", + "remember:request-dispatch.txt", + user, + emptyList(), + null, + null, + request, + ) verifyNoInteractions(pipeline) } @@ -484,6 +837,246 @@ class IncrementalPropositionExtractionTest { assertTrue(extraction.isIdle, "a rejected event must not leave the extractor busy") } + @Test + 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() + val locator = UriLocator("https://example.com/profiled") + val revision = SourceRevisionRef(locator.key(), "r1") + val profile = ExtractionContentProfileRef("house-style", "v1") + + extraction.rememberText( + text = "untyped text", + sourceId = "untyped:profiled", + user = user, + additionalGrounding = emptyList(), + perspective = null, + mintNewEntities = null, + request = ExtractionRequest(profile = profile), + ) + extraction.rememberText( + text = "source text", + sourceId = "source:profiled", + user = user, + additionalGrounding = emptyList(), + perspective = null, + mintNewEntities = null, + request = ExtractionRequest( + sourceLocator = locator, + sourceRevision = revision, + profile = profile, + ), + ) + + // 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 `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("plain file text".toByteArray()), + filename = "profile-only.txt", + user = user, + request = ExtractionRequest(profile = profile), + ) + extraction.rememberFile( + inputStream = ByteArrayInputStream("source file text".toByteArray()), + filename = "source-profiled.txt", + user = user, + request = ExtractionRequest( + sourceLocator = locator, + sourceRevision = revision, + profile = profile, + ), + ) + // 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( + any(), + any(), + contextCaptor.capture(), + anyOrNull(), + any(), + eq(emptyList()), + ) + 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 `calls written without a request carry no profile`() { + 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) + } + } + + @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") + + 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(profile = profile), + ) + + 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 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, entityResolver = plain.entityResolver), + ) + assertSame(profile, profiled.profile) + } + + @Test + 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 profileCalls = AtomicInteger() + val event = object : SourceAnalysisRequestEvent(this, user()) { + override fun incrementalSource(): IncrementalSource = source + + override fun profile(): ExtractionContentProfileRef = + profile.also { profileCalls.incrementAndGet() } + } + + extraction.extractPropositions(event) + + // 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()) + val contextCaptor = argumentCaptor() + verify(pipeline).processChunk(any(), contextCaptor.capture()) + assertSame(profile, contextCaptor.firstValue.profile) + 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, @@ -518,11 +1111,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/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/INDEX.md b/docs/design/INDEX.md index c8f62f55..32f99796 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 + 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 new file mode 100644 index 00000000..67c679c8 --- /dev/null +++ b/docs/design/extraction-profiles.md @@ -0,0 +1,301 @@ +# 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: `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. + +## 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 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 + 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 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 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 +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. A reference is an +identifier the host mints, not a place to put a payload. + +## 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 -.-> 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 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 +× 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 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 + +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 / rememberFile
with an ExtractionRequest"] --> C + B["SourceAnalysisRequestEvent.profile()"] --> C + C["IncrementalPropositionExtraction.buildContext"] --> CTX + CTX["SourceAnalysisContext.profile"] --> X["carried, never consulted"] +``` + +| Path | How a profile arrives | +| --- | --- | +| `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 | + +### 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 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 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 | +| `rememberFile` | 3 args | 3 args, and 4 | + +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 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 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 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)"] -->|"request empty"| F3 + F4 -->|"request carries something"| T7 + T6["rememberText(6)"] --> T7["rememberText(7), terminal"] +``` + +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 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. + +## 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 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` 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 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. +- **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` 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: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 +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 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 + +- **No host profile catalog.** DICE holds no mapping from a reference to a policy and offers no + place to put one. +- **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 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. 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.