diff --git a/pkg-py/src/commons/_context_layer.py b/pkg-py/src/commons/_context_layer.py index a9251a30..07386987 100644 --- a/pkg-py/src/commons/_context_layer.py +++ b/pkg-py/src/commons/_context_layer.py @@ -12,11 +12,15 @@ import re import threading from collections.abc import Iterable +from typing import TYPE_CHECKING from raghilda.chunker import MarkdownChunker from raghilda.document import MarkdownDocument from raghilda.store import DuckDBStore +if TYPE_CHECKING: + from ._data_dictionary import DataDictionary + __all__ = ["ContextLayer", "context_layer"] # Frontmatter carries file metadata (e.g. provenance) meant for maintainers, @@ -111,6 +115,17 @@ def search(self, query: str, top_k: int = 3) -> list[str]: ] +def _dictionary_chunks(dictionary: DataDictionary | None) -> list[str]: + """The dictionary's retrievable prose, or nothing when there is none. + + A source without a dictionary is ordinary, so the absence is handled + here rather than by every caller. + """ + if dictionary is None: + return [] + return dictionary.context_chunks() + + def context_layer( files: Iterable[str | os.PathLike[str]] = (), ) -> ContextLayer: diff --git a/pkg-py/tests/test_context_layer.py b/pkg-py/tests/test_context_layer.py index 3e816d13..4be2dcc9 100644 --- a/pkg-py/tests/test_context_layer.py +++ b/pkg-py/tests/test_context_layer.py @@ -5,7 +5,8 @@ from raghilda.store import DuckDBStore from commons import ContextLayer, context_layer -from commons._context_layer import strip_frontmatter +from commons._context_layer import _dictionary_chunks, strip_frontmatter +from commons._data_dictionary import DataDictionary from ._shared import load_shared_fixture @@ -232,3 +233,18 @@ def counting_create(*args, **kwargs): assert builds == 1 assert all(result == results[0] for result in results) + + +# An empty case list would make the parametrized test below vacuously pass. +def test_the_dictionary_chunk_fixture_is_not_empty(): + assert SHARED["dictionary_context_chunks"]["cases"] + + +@pytest.mark.parametrize( + "case", SHARED["dictionary_context_chunks"]["cases"], ids=lambda c: c["name"] +) +def test_dictionary_context_chunks_shared_cases(case): + spec = case["dictionary"] + dictionary = None if spec is None else DataDictionary.model_validate(spec) + + assert _dictionary_chunks(dictionary) == case["expected"] diff --git a/pkg-r/R/context-layer.R b/pkg-r/R/context-layer.R index a0dab84d..581c31d4 100644 --- a/pkg-r/R/context-layer.R +++ b/pkg-r/R/context-layer.R @@ -97,7 +97,9 @@ dictionary_context_chunks <- function(dictionary) { glossary, definition_context_chunks(dictionary) ) - chunks[nzchar(chunks)] + # vapply() names the table chunks after their tables; the chunks are just + # text, and the names would ride along into the layer's documents. + unname(chunks[nzchar(chunks)]) } # Frontmatter carries file metadata (e.g. provenance) meant for maintainers, diff --git a/pkg-r/tests/testthat/fixtures/shared/context_layer.json b/pkg-r/tests/testthat/fixtures/shared/context_layer.json index c9cf2f7c..250fe39c 100644 --- a/pkg-r/tests/testthat/fixtures/shared/context_layer.json +++ b/pkg-r/tests/testthat/fixtures/shared/context_layer.json @@ -1,5 +1,5 @@ { - "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.", + "description": "The context layer's text handling: what frontmatter is stripped before indexing, and how a data dictionary's prose becomes retrievable chunks. 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": [ @@ -49,5 +49,127 @@ "expected": "intro\n---\na: 1\n---\nbody" } ] + }, + "dictionary_context_chunks": { + "description": "Dictionary prose doubles as searchable context, cut at natural YAML boundaries (the dataset details, each table's prose, each glossary term) so retrieval returns coherent units rather than fragments. Order is dataset details, then tables in declaration order, then glossary terms, then governed definitions. A table with no prose contributes no chunk, and empty strings are dropped. Column-level content stays out: first touch owns it, and indexing it would pay for a second copy the agent already has. Governed definitions carry no case here, because the two implementations compile them at different constructors and so disagree on whether an unattached dictionary has any; definitions.json pins the chunk text itself.", + "cases": [ + { + "name": "details, tables, and glossary in order", + "dictionary": { + "details": "Orders from the retail system.", + "tables": { + "orders": { + "description": "One row per order.", + "details": "Excludes cancellations." + }, + "customers": { + "description": "One row per customer.", + "details": null + } + }, + "glossary": { + "AOV": "Average order value.", + "GMV": "Gross merchandise value." + } + }, + "expected": [ + "Orders from the retail system.", + "Table `orders`: One row per order.\n\nExcludes cancellations.", + "Table `customers`: One row per customer.", + "AOV: Average order value.", + "GMV: Gross merchandise value." + ] + }, + { + "name": "a table with no prose contributes no chunk", + "dictionary": { + "details": null, + "tables": { + "orders": { + "description": null, + "details": null + } + }, + "glossary": {} + }, + "expected": [] + }, + { + "name": "column-level prose is not indexed", + "dictionary": { + "details": null, + "tables": { + "orders": { + "description": "One row per order.", + "details": null, + "columns": { + "revenue": { + "description": "Line revenue, net of returns." + } + } + } + }, + "glossary": {} + }, + "expected": [ + "Table `orders`: One row per order." + ] + }, + { + "name": "an absent dictionary yields no chunks", + "dictionary": null, + "expected": [] + }, + { + "name": "details alone is enough", + "dictionary": { + "details": "Orders from the retail system.", + "tables": {}, + "glossary": {} + }, + "expected": [ + "Orders from the retail system." + ] + }, + { + "name": "empty strings are dropped", + "dictionary": { + "details": "", + "tables": { + "orders": { + "description": "One row per order.", + "details": null + } + }, + "glossary": {} + }, + "expected": [ + "Table `orders`: One row per order." + ] + }, + { + "name": "tables as a sequence keep declaration order", + "dictionary": { + "details": null, + "tables": [ + { + "name": "orders", + "description": "One row per order.", + "details": null + }, + { + "name": "customers", + "description": "One row per customer.", + "details": null + } + ], + "glossary": {} + }, + "expected": [ + "Table `orders`: One row per order.", + "Table `customers`: One row per customer." + ] + } + ] } } diff --git a/pkg-r/tests/testthat/test-context-layer.R b/pkg-r/tests/testthat/test-context-layer.R index 7ebec0ed..2b5560fc 100644 --- a/pkg-r/tests/testthat/test-context-layer.R +++ b/pkg-r/tests/testthat/test-context-layer.R @@ -10,6 +10,23 @@ test_that("strip_frontmatter matches the shared cases", { } }) +test_that("dictionary_context_chunks matches the shared cases", { + cases <- shared_fixture("context_layer")$dictionary_context_chunks$cases + + for (case in cases) { + dictionary <- if (is.null(case$dictionary)) { + NULL + } else { + new_data_dictionary(case$dictionary) + } + expect_identical( + dictionary_context_chunks(dictionary), + as.character(unlist(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 index c9cf2f7c..250fe39c 100644 --- a/tests/shared/context_layer.json +++ b/tests/shared/context_layer.json @@ -1,5 +1,5 @@ { - "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.", + "description": "The context layer's text handling: what frontmatter is stripped before indexing, and how a data dictionary's prose becomes retrievable chunks. 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": [ @@ -49,5 +49,127 @@ "expected": "intro\n---\na: 1\n---\nbody" } ] + }, + "dictionary_context_chunks": { + "description": "Dictionary prose doubles as searchable context, cut at natural YAML boundaries (the dataset details, each table's prose, each glossary term) so retrieval returns coherent units rather than fragments. Order is dataset details, then tables in declaration order, then glossary terms, then governed definitions. A table with no prose contributes no chunk, and empty strings are dropped. Column-level content stays out: first touch owns it, and indexing it would pay for a second copy the agent already has. Governed definitions carry no case here, because the two implementations compile them at different constructors and so disagree on whether an unattached dictionary has any; definitions.json pins the chunk text itself.", + "cases": [ + { + "name": "details, tables, and glossary in order", + "dictionary": { + "details": "Orders from the retail system.", + "tables": { + "orders": { + "description": "One row per order.", + "details": "Excludes cancellations." + }, + "customers": { + "description": "One row per customer.", + "details": null + } + }, + "glossary": { + "AOV": "Average order value.", + "GMV": "Gross merchandise value." + } + }, + "expected": [ + "Orders from the retail system.", + "Table `orders`: One row per order.\n\nExcludes cancellations.", + "Table `customers`: One row per customer.", + "AOV: Average order value.", + "GMV: Gross merchandise value." + ] + }, + { + "name": "a table with no prose contributes no chunk", + "dictionary": { + "details": null, + "tables": { + "orders": { + "description": null, + "details": null + } + }, + "glossary": {} + }, + "expected": [] + }, + { + "name": "column-level prose is not indexed", + "dictionary": { + "details": null, + "tables": { + "orders": { + "description": "One row per order.", + "details": null, + "columns": { + "revenue": { + "description": "Line revenue, net of returns." + } + } + } + }, + "glossary": {} + }, + "expected": [ + "Table `orders`: One row per order." + ] + }, + { + "name": "an absent dictionary yields no chunks", + "dictionary": null, + "expected": [] + }, + { + "name": "details alone is enough", + "dictionary": { + "details": "Orders from the retail system.", + "tables": {}, + "glossary": {} + }, + "expected": [ + "Orders from the retail system." + ] + }, + { + "name": "empty strings are dropped", + "dictionary": { + "details": "", + "tables": { + "orders": { + "description": "One row per order.", + "details": null + } + }, + "glossary": {} + }, + "expected": [ + "Table `orders`: One row per order." + ] + }, + { + "name": "tables as a sequence keep declaration order", + "dictionary": { + "details": null, + "tables": [ + { + "name": "orders", + "description": "One row per order.", + "details": null + }, + { + "name": "customers", + "description": "One row per customer.", + "details": null + } + ], + "glossary": {} + }, + "expected": [ + "Table `orders`: One row per order.", + "Table `customers`: One row per customer." + ] + } + ] } }