Python context layer: pin dictionary prose as retrievable chunks - #284
Open
jat255 wants to merge 3 commits into
Open
Python context layer: pin dictionary prose as retrievable chunks#284jat255 wants to merge 3 commits into
jat255 wants to merge 3 commits into
Conversation
jat255
added
py
The dictionary already turns its prose into retrieval chunks in both packages, but nothing held the two to the same answer. Add a dictionary_context_chunks section to the shared context_layer fixture and a runner in each suite, driving each package's own reader from the same parsed shape. The fixture exposed one divergence: R named its table chunks after their tables, because vapply() over names() carries them, and those names rode into the layer's documents. Chunks are text, so drop the names. Neither call site read them. Governed definitions carry no case. The two packages compile them at different constructors, so they disagree on whether a dictionary that is not attached to a source has any; definitions.json pins the chunk text. Python needs no new chunking function: the dictionary owns it as DataDictionary.context_chunks(). What was missing was the guard for a source with no dictionary at all, which is what _dictionary_chunks() adds and what folding source prose into a layer needs next.
jat255
force-pushed
the
jat255/context-layer-dictionary-chunks
branch
from
September 7, 2026 00:40
7551a43 to
da628ae
Compare
|
Preview deployed to Connect ( Deployed from commit 6cbba98. |
|
Preview deployed to Connect ( Deployed from commit 6cbba98. |
Review follow-ups. Restore the empty-fixture guard on the strip_frontmatter runner: tests/shared/README.md requires it of every runner that enumerates cases. Pin that empty strings are dropped from dictionary chunks, and pin table declaration order through the sequence shape that authored data-dict.yaml files use, rather than only the pre-keyed mapping.
jat255
removed
the
needs-manual-review
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.
Third of four PRs for M4, the Python context layer (kata
vp7e). Stacked on #241; #285 follows this and wires the chunks into an agent's layer on top.This PR adds shared fixtures defining the expected way prose should be transformed into retrieval chunks.
tests/shared/context_layer.jsongains adictionary_context_chunkssection and both suites run it against their own reader: dataset details, then tables in declaration order, then glossary terms, then governed definitions, with a table that has no prose contributing nothing and column-level content staying out.Implementing this PR found that python needed a guard for a source carrying no dictionary at all.
Governed definitions get no fixture case, deliberately. Both packages render the chunk text the same way and
definitions.jsonpins it, but they compile definitions at different constructors, so they disagree on whether a dictionary not yet attached to a source has any. A case here would pin that disagreement rather than the contract.R changes
@simonpcouch
One functional change in
pkg-r/R/context-layer.R, required by an error found by the new fixture:dictionary_context_chunks()returned a named character vector and now returns an unnamed one.The names came from
vapply(names(dictionary$tables), ...), which labels each chunk with its table name, andc()propagated them to the whole vector (with""for the dataset details and glossary entries). Those names rode intoaugment_context_layer()and onto the layer'sdocsvector. Nothing ever read them, but the R side was carrying metadata Python can't represent, which made the shared fixture unsatisfiable for both suites. The fix is a singleunname(); chunk text and order are unchanged.Blast radius: two call sites, neither reads names.
augment_context_layer()concatenates the vector into documents, and the one test caller (test-definitions.R:401) usesgrepl()and positional subsetting.Verified by running the four test files that touch this path:
test-context-layer.R(65 passing),test-data-dictionary.R(71 passing, 3 skipped on CRAN),test-definitions.R(114 passing),test-citations.R(84 passing). I skipped the full R suite — it hangs ~10 minutes onrun_r's uncancelledlatercallbacks, fixed in #268.One thing worth scrutiny: the new runner in
test-context-layer.Rfeeds each fixture case through the realnew_data_dictionary()reader, which works because both languages parse the same authoreddata-dict.yaml. If that ever diverges, this runner is where it will surface.No hand-written tests were deleted; the new test is additive. The generated fixture
pkg-r/tests/testthat/fixtures/shared/context_layer.jsoncomes fromscripts/sync-shared-fixtures.shand needs no review.