-
Notifications
You must be signed in to change notification settings - Fork 1
Python context layer: construction and frontmatter handling #240
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
Open
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
7db588d
feat(py): context_layer() with eager read and frontmatter stripping
jat255 1fbbf40
fix: strip an emptied-out frontmatter fence, and read the fixture from R
jat255 cbdd674
fix(py): context layer review follow-ups
jat255 61e7d2c
Fix comment grammar in frontmatter stripping function
jat255 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| """A context layer: text an agent retrieves from to interpret its data source. | ||
|
|
||
| Context is retrieved when relevant. Facts needed in every conversation belong | ||
| in the agent's instructions, not here. ``pkg-r/R/context-layer.R`` implements | ||
| the same behaviour for R, and ``tests/shared/context_layer.json`` pins the | ||
| parts that must agree. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import os | ||
| import re | ||
| from collections.abc import Iterable | ||
|
|
||
| __all__ = ["ContextLayer", "context_layer"] | ||
|
|
||
| # Frontmatter carries file metadata (e.g. provenance) meant for maintainers, | ||
| # not the model; drop it so retrieval can't surface it. Anchored to the start | ||
| # so a '---' thematic break in the body survives. The metadata block is | ||
| # optional so an emptied-out fence is removed rather than indexed as text. | ||
| _FRONTMATTER = re.compile(r"\A---\r?\n(.*?\r?\n)?---(\r?\n|\Z)", re.DOTALL) | ||
|
|
||
|
|
||
| def strip_frontmatter(md: str) -> str: | ||
| return _FRONTMATTER.sub("", md, count=1) | ||
|
|
||
|
|
||
| class ContextLayer: | ||
| """Text that helps an agent interpret its data source. | ||
|
|
||
| Construct one with :func:`context_layer`. Internals are private and may | ||
| change without notice. | ||
| """ | ||
|
|
||
| def __init__(self, docs: Iterable[str] = ()) -> None: | ||
| self._docs = tuple(docs) | ||
|
|
||
| @property | ||
| def docs(self) -> tuple[str, ...]: | ||
| """The documents as read from their files, frontmatter stripped.""" | ||
| return self._docs | ||
|
|
||
| def __repr__(self) -> str: | ||
| n = len(self._docs) | ||
| return f"<ContextLayer: {n} document{'' if n == 1 else 's'}>" | ||
|
|
||
|
|
||
| def context_layer( | ||
| files: Iterable[str | os.PathLike[str]] = (), | ||
| ) -> ContextLayer: | ||
| """Create a context layer from text or Markdown files. | ||
|
|
||
| ``files`` must be a collection of paths; a bare string or path raises | ||
| ``TypeError``. Files are read eagerly and decoded as UTF-8, so a missing | ||
| path (``FileNotFoundError``), a directory (``IsADirectoryError``), or a | ||
| file in another encoding (``UnicodeDecodeError``) fails here rather than | ||
| mid-conversation. | ||
| """ | ||
| if isinstance(files, (str, bytes, os.PathLike)): | ||
| raise TypeError( | ||
| f"`files` must be a collection of paths, not {type(files).__name__}. " | ||
| f"Pass a list: files=[{files!r}]." | ||
| ) | ||
|
|
||
| # Read eagerly so a bad path fails at construction; index lazily (see | ||
| # ContextLayer.search). | ||
| docs: list[str] = [] | ||
| for path in files: | ||
| with open(path, encoding="utf-8") as handle: | ||
| md = strip_frontmatter(handle.read()) | ||
| # readLines() in pkg-r/R/context-layer.R drops the final line ending; | ||
| # do the same so both read the same document from the same file. | ||
| md = md.removesuffix("\n") | ||
| if md.strip(): | ||
| docs.append(md) | ||
|
|
||
| return ContextLayer(docs) |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| import pytest | ||
|
|
||
| from commons import ContextLayer, context_layer | ||
| from commons._context_layer import strip_frontmatter | ||
|
|
||
| from ._shared import load_shared_fixture | ||
|
|
||
| SHARED = load_shared_fixture("context_layer") | ||
|
|
||
|
|
||
| # An empty case list would make the parametrized test below vacuously pass. | ||
| def test_the_fixture_is_not_empty(): | ||
| assert SHARED["strip_frontmatter"]["cases"] | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "case", SHARED["strip_frontmatter"]["cases"], ids=lambda c: c["name"] | ||
| ) | ||
| def test_strip_frontmatter_shared_cases(case): | ||
| assert strip_frontmatter(case["input"]) == case["expected"] | ||
|
|
||
|
|
||
| def test_context_layer_reads_files_and_strips_frontmatter(tmp_path): | ||
| path = tmp_path / "notes.md" | ||
| path.write_text("---\nprovenance: abc1234\n---\n# Revenue\nRevenue excludes tax.") | ||
|
|
||
| layer = context_layer(files=[path]) | ||
|
|
||
| assert layer.docs == ("# Revenue\nRevenue excludes tax.",) | ||
|
|
||
|
|
||
| def test_context_layer_skips_a_frontmatter_only_file(tmp_path): | ||
| path = tmp_path / "meta.md" | ||
| path.write_text("---\nprovenance: some-source\n---\n") | ||
|
|
||
| assert context_layer(files=[path]).docs == () | ||
|
|
||
|
|
||
| def test_context_layer_drops_the_final_line_ending(tmp_path): | ||
| path = tmp_path / "notes.md" | ||
| path.write_text("# Revenue\n") | ||
|
|
||
| assert context_layer(files=[path]).docs == ("# Revenue",) | ||
|
|
||
|
|
||
| def test_context_layer_defaults_to_no_documents(): | ||
| assert context_layer().docs == () | ||
| assert isinstance(context_layer(), ContextLayer) | ||
|
|
||
|
|
||
| def test_context_layer_fails_at_construction_on_a_bad_path(tmp_path): | ||
| missing = tmp_path / "nope.md" | ||
|
|
||
| with pytest.raises(FileNotFoundError, match=str(missing)): | ||
| context_layer(files=[missing]) | ||
|
|
||
|
|
||
| def test_context_layer_rejects_a_bare_string(tmp_path): | ||
| path = tmp_path / "notes.md" | ||
| path.write_text("# Revenue") | ||
|
|
||
| with pytest.raises(TypeError, match="files"): | ||
| context_layer(files=str(path)) | ||
|
|
||
|
|
||
| def test_context_layer_repr_counts_documents(tmp_path): | ||
| path = tmp_path / "notes.md" | ||
| path.write_text("# Revenue") | ||
|
|
||
| assert repr(context_layer()) == "<ContextLayer: 0 documents>" | ||
| assert repr(context_layer(files=[path])) == "<ContextLayer: 1 document>" |
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| { | ||
| "description": "The context layer's text handling: what frontmatter is stripped before indexing. Shared by pkg-r and pkg-py. The source is tests/shared/context_layer.json; the copy under pkg-r/tests/testthat/fixtures/shared/ is generated by scripts/sync-shared-fixtures.sh. Edit the source and re-run that script. Retrieval ranking is deliberately absent: the two BM25 engines score differently, and that difference is implementation detail.", | ||
| "strip_frontmatter": { | ||
| "description": "Frontmatter carries file metadata meant for maintainers, not the model, so it is removed before the document reaches the store. Only a fence that opens on the very first line counts. A '---' in the body is a thematic break and must survive, or a document would lose everything above it.", | ||
| "cases": [ | ||
| { | ||
| "name": "leading frontmatter is removed", | ||
| "input": "---\nprovenance: abc1234\n---\n# Revenue\nRevenue excludes tax.", | ||
| "expected": "# Revenue\nRevenue excludes tax." | ||
| }, | ||
| { | ||
| "name": "a document without frontmatter is unchanged", | ||
| "input": "# Revenue\nRevenue excludes tax.", | ||
| "expected": "# Revenue\nRevenue excludes tax." | ||
| }, | ||
| { | ||
| "name": "a body thematic break survives", | ||
| "input": "# Intro\nRevenue excludes tax.\n\n---\n\n# Details\nDiscounts are applied before tax.", | ||
| "expected": "# Intro\nRevenue excludes tax.\n\n---\n\n# Details\nDiscounts are applied before tax." | ||
| }, | ||
| { | ||
| "name": "only the first fence is removed", | ||
| "input": "---\na: 1\n---\nbody\n---\nb: 2\n---\n", | ||
| "expected": "body\n---\nb: 2\n---\n" | ||
| }, | ||
| { | ||
| "name": "a frontmatter-only document becomes empty", | ||
| "input": "---\nprovenance: some-source\n---\n", | ||
| "expected": "" | ||
| }, | ||
| { | ||
| "name": "an empty fence is removed rather than kept as text", | ||
| "input": "---\n---\n# Revenue", | ||
| "expected": "# Revenue" | ||
| }, | ||
| { | ||
| "name": "an empty fence with no body becomes empty", | ||
| "input": "---\n---\n", | ||
| "expected": "" | ||
| }, | ||
| { | ||
| "name": "CRLF line endings are handled", | ||
| "input": "---\r\nprovenance: abc1234\r\n---\r\n# Revenue", | ||
| "expected": "# Revenue" | ||
| }, | ||
| { | ||
| "name": "a fence that does not start on the first line is left alone", | ||
| "input": "intro\n---\na: 1\n---\nbody", | ||
| "expected": "intro\n---\na: 1\n---\nbody" | ||
| } | ||
| ] | ||
| } | ||
| } |
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| { | ||
| "description": "The context layer's text handling: what frontmatter is stripped before indexing. Shared by pkg-r and pkg-py. The source is tests/shared/context_layer.json; the copy under pkg-r/tests/testthat/fixtures/shared/ is generated by scripts/sync-shared-fixtures.sh. Edit the source and re-run that script. Retrieval ranking is deliberately absent: the two BM25 engines score differently, and that difference is implementation detail.", | ||
| "strip_frontmatter": { | ||
| "description": "Frontmatter carries file metadata meant for maintainers, not the model, so it is removed before the document reaches the store. Only a fence that opens on the very first line counts. A '---' in the body is a thematic break and must survive, or a document would lose everything above it.", | ||
| "cases": [ | ||
| { | ||
| "name": "leading frontmatter is removed", | ||
| "input": "---\nprovenance: abc1234\n---\n# Revenue\nRevenue excludes tax.", | ||
| "expected": "# Revenue\nRevenue excludes tax." | ||
| }, | ||
| { | ||
| "name": "a document without frontmatter is unchanged", | ||
| "input": "# Revenue\nRevenue excludes tax.", | ||
| "expected": "# Revenue\nRevenue excludes tax." | ||
| }, | ||
| { | ||
| "name": "a body thematic break survives", | ||
| "input": "# Intro\nRevenue excludes tax.\n\n---\n\n# Details\nDiscounts are applied before tax.", | ||
| "expected": "# Intro\nRevenue excludes tax.\n\n---\n\n# Details\nDiscounts are applied before tax." | ||
| }, | ||
| { | ||
| "name": "only the first fence is removed", | ||
| "input": "---\na: 1\n---\nbody\n---\nb: 2\n---\n", | ||
| "expected": "body\n---\nb: 2\n---\n" | ||
| }, | ||
| { | ||
| "name": "a frontmatter-only document becomes empty", | ||
| "input": "---\nprovenance: some-source\n---\n", | ||
| "expected": "" | ||
| }, | ||
| { | ||
| "name": "an empty fence is removed rather than kept as text", | ||
| "input": "---\n---\n# Revenue", | ||
| "expected": "# Revenue" | ||
| }, | ||
| { | ||
| "name": "an empty fence with no body becomes empty", | ||
| "input": "---\n---\n", | ||
| "expected": "" | ||
| }, | ||
| { | ||
| "name": "CRLF line endings are handled", | ||
| "input": "---\r\nprovenance: abc1234\r\n---\r\n# Revenue", | ||
| "expected": "# Revenue" | ||
| }, | ||
| { | ||
| "name": "a fence that does not start on the first line is left alone", | ||
| "input": "intro\n---\na: 1\n---\nbody", | ||
| "expected": "intro\n---\na: 1\n---\nbody" | ||
| } | ||
| ] | ||
| } | ||
| } |
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really worried about this