Python context layer: construction and frontmatter handling - #240
Open
jat255 wants to merge 4 commits into
Open
Conversation
jat255
changed the title
|
Preview deployed to Connect ( Deployed from commit 61e7d2c. |
|
Preview deployed to Connect ( Deployed from commit 61e7d2c. |
Files are read at construction so a bad path fails there rather than mid-conversation. Indexing is deferred; the store lands in the next commit. The frontmatter regex is anchored to the start of the document and consumes only the first fence, so a '---' thematic break in the body survives. Without the anchor a document would silently lose everything above its first break. tests/shared/context_layer.json pins the seven stripping cases. All seven were checked against the existing R strip_frontmatter() as well, so the fixture is satisfied by both suites as written; the R-side test that reads it arrives with the dictionary chunk work, which is what needs the cross-language guard.
Both regexes required a newline before the closing fence, so a file whose frontmatter keys had been deleted kept its "---\n---" and was indexed as literal fence text. The metadata block is now optional in both languages. Fixing only one side would have added a shared case the other fails. The fixture was previously read by the Python suite alone, which pins nothing. The R runner guards on a non-empty case list so an unread fixture cannot pass vacuously. The README still claimed the package exports nothing.
- Prose docstrings per house style; context_layer() names every reachable error (TypeError, FileNotFoundError, IsADirectoryError, UnicodeDecodeError), and the docs property is documented. - Drop the final line ending after reading, matching the R reader so both packages build the same document from the same file. - Assert the shared fixture's case list is non-empty, as tests/shared/README.md requires of every runner; the R runner already did. - The fixture description no longer claims it pins dictionary chunking, which it does not cover. - test_public_api_exposes_the_semantic_layer now expects the context layer exports, reconciling with the semantic layer merged on main.
jat255
force-pushed
the
jat255/context-layer-construction
branch
from
September 4, 2026 17:27
256030f to
cbdd674
Compare
jat255
marked this pull request as ready for review
September 4, 2026 17:52
simonpcouch
approved these changes
Sep 5, 2026
Comment on lines
+3
to
+4
| # An empty list would make the loop below vacuously succeed. | ||
| expect_gt(length(cases), 0) |
Collaborator
There was a problem hiding this comment.
Suggested change
| # An empty list would make the loop below vacuously succeed. | |
| expect_gt(length(cases), 0) |
Not really worried about this
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
2 participants
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.
First of two PRs for M4, the Python context layer (kata
63jx). This one adds construction and text handling; #241 adds retrieval on top.context_layer(files=...)reads its files eagerly and strips YAML frontmatter, so a bad path fails at construction rather than mid-conversation, and maintainer metadata such as a provenance URL never reaches the index. The returnedContextLayeris a plain class per D8.commons.__all__is no longer empty.The frontmatter regex is anchored to the start of the document and consumes only the first fence, so a
---thematic break in the body survives. The metadata block is optional, so a fence whose keys have been deleted is removed rather than indexed as literal---text.tests/shared/context_layer.jsonpins the nine stripping cases, and both suites run them. Retrieval ranking is deliberately not in the fixture: the two BM25 engines score differently, and per the D7 refinement that is implementation detail rather than observable behaviour.R changes
Only one functional change in
pkg-r/R/context-layer.Rto correct a small (potential) bug. Previously,strip_frontmatter()would not remove a document opening with---\n---(i.e. it had a frontmatter section, but it was empty).The regex was
^---\r?\n.*?\r?\n---(\r?\n|$)and is now^---\r?\n(.*?\r?\n)?---(\r?\n|$). Making the metadata block optional is the whole change. Previously a document opening with---\n---did not match, so the fence was left in place;context_layer()then saw non-empty text, kept the document, and indexed---\n---as a chunk. Now it matches and the fence is removed, which for a fence-only file means the document is dropped entirely.Blast radius is small.
strip_frontmatter()has one caller,context_layer(), and its only other reader is the new test. The change cannot widen what gets stripped, because adding an optional group to a lazy pattern only lets it match where it previously failed, and the anchor is unchanged.Evidence: the full R suite is green at 6649 passing, 0 failed, 0 errors, 28 skipped. The nine shared cases were run against the R implementation directly and against the Python one, and both satisfy all nine.
No hand-written R tests were deleted. The new test is additive.
pkg-r/tests/testthat/fixtures/shared/context_layer.jsonis generated byscripts/sync-shared-fixtures.shand needs no review.