diff --git a/pkg-py/README.md b/pkg-py/README.md index 01c05fbd..fbaa1179 100644 --- a/pkg-py/README.md +++ b/pkg-py/README.md @@ -2,6 +2,6 @@ `commons` is a constructor for trustworthy data agents. Once implemented, this package will give an LLM data, semantic, and context layers to work with, tools for querying them, and A/B/C provenance tags so every answer carries a classification as to its trustworthiness. -**Status: pre-alpha.** The package exports the data and semantic layers (`data_source`, `list_tables`, `measure`, `semantic_layer`, and their types); the agent constructor that ties them together is not implemented yet. Python 3.11 or later is required. +**Status: pre-alpha.** The package exports the data and semantic layers (`data_source`, `list_tables`, `measure`, `semantic_layer`, and their types) and the context layer (`context_layer()` and the `ContextLayer` it returns) for reading text or Markdown files; the agent constructor that ties them together is not implemented yet. Python 3.11 or later is required. -Behavior that both implementations must agree on belongs in [`tests/shared/`](https://github.com/posit-dev/commons/tree/main/tests/shared) at the repository root, which that directory's README defines as the authority. The provenance tag rules and display copy are the first behavior governed that way; both suites run those cases. +Behavior that both implementations must agree on belongs in [`tests/shared/`](https://github.com/posit-dev/commons/tree/main/tests/shared) at the repository root, which that directory's README defines as the authority. The provenance tag rules and display copy, the citation dialect, and the context layer's frontmatter handling are governed that way; both suites run those cases. diff --git a/pkg-py/src/commons/__init__.py b/pkg-py/src/commons/__init__.py index 87ee2f27..483dbbed 100644 --- a/pkg-py/src/commons/__init__.py +++ b/pkg-py/src/commons/__init__.py @@ -5,14 +5,17 @@ classification as to how much it can be trusted. """ +from ._context_layer import ContextLayer, context_layer from ._data_source import DataSource, data_source, list_tables from ._measures import Injected, Measure, SemanticLayer, measure, semantic_layer __all__: list[str] = [ + "ContextLayer", "DataSource", "Injected", "Measure", "SemanticLayer", + "context_layer", "data_source", "list_tables", "measure", diff --git a/pkg-py/src/commons/_context_layer.py b/pkg-py/src/commons/_context_layer.py new file mode 100644 index 00000000..ef54d216 --- /dev/null +++ b/pkg-py/src/commons/_context_layer.py @@ -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"" + + +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) diff --git a/pkg-py/tests/test_context_layer.py b/pkg-py/tests/test_context_layer.py new file mode 100644 index 00000000..865b7631 --- /dev/null +++ b/pkg-py/tests/test_context_layer.py @@ -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()) == "" + assert repr(context_layer(files=[path])) == "" diff --git a/pkg-py/tests/test_measures.py b/pkg-py/tests/test_measures.py index 3f53f54b..9b4e397b 100644 --- a/pkg-py/tests/test_measures.py +++ b/pkg-py/tests/test_measures.py @@ -1088,10 +1088,12 @@ def test_public_api_exposes_the_semantic_layer() -> None: import commons assert set(commons.__all__) == { + "ContextLayer", "DataSource", "Injected", "Measure", "SemanticLayer", + "context_layer", "data_source", "list_tables", "measure", diff --git a/pkg-r/R/context-layer.R b/pkg-r/R/context-layer.R index 2c352ccd..b372de56 100644 --- a/pkg-r/R/context-layer.R +++ b/pkg-r/R/context-layer.R @@ -101,9 +101,10 @@ dictionary_context_chunks <- function(dictionary) { } # Frontmatter carries file metadata (e.g. provenance) meant for maintainers, -# not the model; drop it so retrieval can't surface it. +# not the model; drop it so retrieval can't surface it. The metadata block is +# optional so an empty frontmatter section is removed rather than indexed as text. strip_frontmatter <- function(md) { - sub("(?s)^---\r?\n.*?\r?\n---(\r?\n|$)", "", md, perl = TRUE) + sub("(?s)^---\r?\n(.*?\r?\n)?---(\r?\n|$)", "", md, perl = TRUE) } # Store setup (duckdb creation, chunk insertion, FTS indexing) is the most diff --git a/pkg-r/tests/testthat/fixtures/shared/context_layer.json b/pkg-r/tests/testthat/fixtures/shared/context_layer.json new file mode 100644 index 00000000..c9cf2f7c --- /dev/null +++ b/pkg-r/tests/testthat/fixtures/shared/context_layer.json @@ -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" + } + ] + } +} diff --git a/pkg-r/tests/testthat/test-context-layer.R b/pkg-r/tests/testthat/test-context-layer.R index d4f2019b..ac83d8a9 100644 --- a/pkg-r/tests/testthat/test-context-layer.R +++ b/pkg-r/tests/testthat/test-context-layer.R @@ -1,3 +1,17 @@ +test_that("strip_frontmatter matches the shared cases", { + cases <- shared_fixture("context_layer")$strip_frontmatter$cases + # An empty list would make the loop below vacuously succeed. + expect_gt(length(cases), 0) + + for (case in cases) { + expect_identical( + strip_frontmatter(case$input), + case$expected, + info = case$name + ) + } +}) + test_that("context_layer indexes files and finds relevant chunks", { path <- withr::local_tempfile(fileext = ".md") writeLines( diff --git a/tests/shared/context_layer.json b/tests/shared/context_layer.json new file mode 100644 index 00000000..c9cf2f7c --- /dev/null +++ b/tests/shared/context_layer.json @@ -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" + } + ] + } +}