Skip to content

Python context layer: construction and frontmatter handling - #240

Merged
jat255 merged 5 commits into
mainfrom
jat255/context-layer-construction
Sep 7, 2026
Merged

Python context layer: construction and frontmatter handling#240
jat255 merged 5 commits into
mainfrom
jat255/context-layer-construction

Conversation

@jat255

@jat255 jat255 commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

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 returned ContextLayer is 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.json pins 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.R to 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.json is generated by scripts/sync-shared-fixtures.sh and needs no review.

@jat255 jat255 changed the title jat255/context layer construction Python context layer: construction and frontmatter handling Sep 2, 2026
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown

Preview deployed to Connect (dogfood.team.pct.posit.it): https://dogfood.team.pct.posit.it/connect/#/apps/d7a36cae-8f27-448b-a478-61b81fbe3942/draft/368295

Deployed from commit 59fb1dc.

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown

Preview deployed to Connect (connect.staging.pct.posit.it): https://connect.staging.pct.posit.it/connect/#/apps/ad662e1b-5048-4acc-9ad7-f9478c92274e/draft/2651

Deployed from commit 59fb1dc.

@jat255
jat255 marked this pull request as draft September 2, 2026 00:04
@jat255 jat255 added the py Affects the Python implementation label Sep 2, 2026
@jat255 jat255 added this to the py-M4: context layer milestone Sep 4, 2026
@jat255
jat255 force-pushed the jat255/context-layer-construction branch from 256030f to cbdd674 Compare September 4, 2026 17:27
@jat255
jat255 marked this pull request as ready for review September 4, 2026 17:52
@jat255
jat255 requested a review from simonpcouch September 4, 2026 17:59

@simonpcouch simonpcouch left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🏄

Comment thread pkg-r/tests/testthat/test-context-layer.R Outdated
@jat255
jat255 force-pushed the jat255/context-layer-construction branch from 2524b4a to 499f6ce Compare September 6, 2026 01:39
jat255 and others added 5 commits September 6, 2026 17:52
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.
Co-authored-by: Simon P. Couch <simonpatrickcouch@gmail.com>
@jat255
jat255 force-pushed the jat255/context-layer-construction branch from 499f6ce to 59fb1dc Compare September 6, 2026 23:52
jat255 added a commit that referenced this pull request Sep 7, 2026
Simon pushed back on the strip_frontmatter runner's guard on #240, and
the dictionary_context_chunks runner re-added the same pattern; drop it
from both so the file stays consistent with the reviewed state. The
Python suite keeps its standalone not-empty tests, the shape #240
approved.
@jat255
jat255 merged commit ae0eeee into main Sep 7, 2026
13 checks passed
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown

Cleaned up 6 preview bundle(s) on https://dogfood.team.pct.posit.it: 366790, 367865, 367869, 368156, 368157, 368295

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown

Cleaned up 6 preview bundle(s) on https://connect.staging.pct.posit.it: 2456, 2573, 2575, 2634, 2635, 2651

@jat255
jat255 deleted the jat255/context-layer-construction branch September 7, 2026 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

py Affects the Python implementation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants