Python context layer: construction and frontmatter handling - #240
Draft
jat255 wants to merge 2 commits into
Draft
Conversation
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.
|
Preview deployed to Connect ( Deployed from commit 256030f. |
|
Preview deployed to Connect ( Deployed from commit 256030f. |
jat255
marked this pull request as draft
September 2, 2026 00:04
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
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
pkg-r/R/context-layer.Rgets a one-line change tostrip_frontmatter(), andtest-context-layer.Rgets a fixture-driven test.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.The behaviour delta is confined to documents whose first fence has no content lines. Every other input produces exactly what it did before, including a fence with content, a document with no fence, a body thematic break, and a fence that does not start on the first line. All four are fixture cases.
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.
Worth your scrutiny: whether dropping a fence-only document is the right call, or whether such a file should surface a warning instead. Silently indexing
---\n---was clearly wrong; silently dropping the file is my judgement and is easy to reverse.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.