Python context layer: fold source prose into retrieval - #285
Open
jat255 wants to merge 2 commits into
Open
Conversation
An agent's sources describe themselves, and that prose is worth retrieving. augment_context_layer() appends each source's dictionary chunks and hands back a new layer, so a dictionary alone is searchable context even when the caller passed no context files at all. The new layer is always a new object. Source enrichment belongs to the agent that owns the sources; mutating the caller's layer would leak one agent's sources into the next agent built from the same layer. With nothing to add the argument comes back unchanged, None included, so an agent with neither context nor a dictionary has no layer rather than an empty one. The milestone's acceptance check is a test rather than a one-off script, and asserts equality rather than a match: the glossary term comes back whole, not as a fragment. pkg-r also folds in a warehouse's own semantic models here. This package has no surface for those yet, so that half is a comment at the point where it will go.
Both packages fold a source's dictionary prose into an agent's context layer, and each was checking that on its own. Add an augment_context_layer section to the shared context_layer fixture and a runner in each suite. Each case lists one entry per source, so the cases cover where the sources' chunks land relative to the caller's own documents, that every source contributes in the order given, and that a source carrying no dictionary is skipped rather than counted. An absent layer, an empty layer, and a dictionary whose prose is all empty are separate cases, because they are separate outcomes. Whether the returned layer is a new object with a fresh index stays out of the fixture and out of R. It is what keeps one agent's sources from leaking into the next, but it is not observable text, so it is asserted in the Python suite only. Two R tests in test-data-dictionary.R covered the same ground more loosely, one asserting only that the document count grew. The fixture runner replaces both.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Last of four PRs for M4, the Python context layer (kata
c8a0). Stacked on #284, which adds the chunks this folds in.augment_context_layer(layer, sources)appends each source's dictionary chunks and returns a new layer, so a dictionary alone is searchable context even when the caller passed no context files. It is internal, as it is inpkg-r; the agent constructor in M5 is what calls it.The returned layer is always a new object with a fresh index. Source enrichment belongs to the agent that owns the sources, so mutating the caller's layer would leak one agent's sources into the next agent built from the same layer. With nothing to add the argument comes back unchanged,
Noneincluded, so an agent with neither context files nor dictionary prose gets no layer rather than an empty one.The milestone's acceptance criterion is a test rather than a one-off script, asserting equality rather than a match, so it fails if a glossary term ever comes back as a fragment instead of whole.
One deliberate gap.
pkg-ralso folds in each source's warehouse semantic models here, the Snowflake semantic views and Databricks metric views. This package has no surface for those yet (katagcgjcovers them, and notes that porting the reader alone would produce records nothing consumes), so that half is a comment at the point where it will go rather than code. The milestone plan attributed those models to M3, which was wrong: M3's semantic layer is measures written in Python, and these are the warehouse's own definitions.R changes
Test-only, no change to any function under
pkg-r/R/.The behaviour this PR adds on the Python side already existed in R, and each package was checking it separately.
tests/shared/context_layer.jsongains anaugment_context_layersection andtest-context-layer.Rgains a runner for it. Each case lists one entry per source, so the cases cover where the sources' chunks land relative to the caller's own documents, that every source contributes in the order given, and that a source carrying no dictionary is skipped rather than counted. An absent layer, an empty layer, and a dictionary whose prose is all empty are separate cases, because they produce separate outcomes.Two hand-written tests in
test-data-dictionary.Rwere deleted, and the fixture runner replaces both. "augmenting keeps existing context docs" asserted that one known document survived and that the document count grew, which the runner now pins as an exact expected vector. "augmenting without dictionaries is a no-op" assertedexpect_null(augment_context_layer(NULL, list(test_source()))), which is the runner's "no sources and no layer stays absent" case plus the stronger "a source with no dictionary leaves the layer alone". A comment at the deletion site points at the fixture, so the next reader looking for that coverage finds it. "dictionary prose is searchable via the context layer" is kept and untouched: it goes through BM25 retrieval, and ranking is engine-specific rather than a cross-language contract.Whether the returned layer is a new object carrying a fresh index is asserted in the Python suite only, and the fixture says so. It is what keeps one agent's sources out of another's, but it is not observable text, and R holds its documents in an R6 object where the equivalent assertion would be about object identity rather than behaviour.
Why it is safe: no R source file changed, so R behaviour cannot have changed. What needs checking is that the runner is not vacuous, and I checked that rather than trusting the pass. The case list is guarded with
expect_gt(length(cases), 0). I confirmed separately that the three-source case really constructs three sources and that its middle, dictionary-less source is skipped, because a runner that silently built one source would still have passed the other cases.test-context-layer.Ris at 33 passing, andtest-data-dictionary.R(68 passing, 3 skipped on CRAN),test-definitions.R(114 passing) andtest-citations.R(84 passing) were run for the deletions and the otheraugment_context_layer()callers. Not the full R suite, for the #268 hang.pkg-r/tests/testthat/fixtures/shared/context_layer.jsonis generated byscripts/sync-shared-fixtures.shand needs no review.