From 7afa74852adb09af6d717e52453e0eaf0931a27e Mon Sep 17 00:00:00 2001 From: Josh Taillon Date: Mon, 7 Sep 2026 10:16:00 -0600 Subject: [PATCH 1/5] feat(py): citation corpus, provenance marker, and turn reminders The four agent helpers that no milestone owned, ahead of the Commons class that assembles them (kata g9yd, under M5). build_citation_corpus() collects the text a citation can be verified against: each measure's search_pool block, each source's dictionary prose and per-table entries, then the context layer's documents. Order is the contract, since matching returns the first entry holding the quote, so prose that is both a dictionary's and a document's keeps the dictionary's label. Python keys its sources, so every dictionary has a name to be labelled with and R's unnamed "data dictionary" fallback has no counterpart here. collect_appended_tags() reads back the tags tool results set, and provenance_aside() renders the marker for the outcomes that show one. The marker names no icon: that URL comes from the served asset bundle, as citation_aside_html already records. The reminders travel as their own content type, so a UI can leave them out. The restored-conversation reminder names Python and run_python where R's names R and run_r; the shared fixture holds the wording once and each package renders it with its own two values. Nothing constructs any of this yet, as planned: that wiring is kata pvrd. --- pkg-py/src/commons/_citations.py | 66 +++++- pkg-py/src/commons/_provenance.py | 79 ++++++- pkg-py/src/commons/_reminders.py | 53 +++++ pkg-py/tests/test_citation_corpus.py | 96 +++++++++ pkg-py/tests/test_provenance.py | 108 +++++++++- pkg-py/tests/test_reminders.py | 81 +++++++ .../fixtures/shared/citation-corpus.json | 154 +++++++++++++ .../testthat/fixtures/shared/provenance.json | 47 ++++ .../fixtures/shared/turn-reminders.json | 40 ++++ pkg-r/tests/testthat/test-citations.R | 204 +++++------------- pkg-r/tests/testthat/test-provenance.R | 42 ++-- pkg-r/tests/testthat/test-turn-reminder.R | 43 ++++ tests/shared/citation-corpus.json | 154 +++++++++++++ tests/shared/provenance.json | 47 ++++ tests/shared/turn-reminders.json | 40 ++++ 15 files changed, 1070 insertions(+), 184 deletions(-) create mode 100644 pkg-py/src/commons/_reminders.py create mode 100644 pkg-py/tests/test_citation_corpus.py create mode 100644 pkg-py/tests/test_reminders.py create mode 100644 pkg-r/tests/testthat/fixtures/shared/citation-corpus.json create mode 100644 pkg-r/tests/testthat/fixtures/shared/turn-reminders.json create mode 100644 pkg-r/tests/testthat/test-turn-reminder.R create mode 100644 tests/shared/citation-corpus.json create mode 100644 tests/shared/turn-reminders.json diff --git a/pkg-py/src/commons/_citations.py b/pkg-py/src/commons/_citations.py index c13dccbf..90fa6ecb 100644 --- a/pkg-py/src/commons/_citations.py +++ b/pkg-py/src/commons/_citations.py @@ -10,15 +10,20 @@ import html import re -from collections.abc import Sequence +from collections.abc import Iterable, Mapping, Sequence from dataclasses import dataclass, field -from typing import Any, Literal, get_args +from typing import TYPE_CHECKING, Any, Literal, get_args from chatlas import ContentToolResult, Turn from chatlas.types import ContentText +from ._measures import Measure, measure_schema_text from ._prompt import read_prompt -from ._provenance import Tag +from ._provenance import TAG_EXTRA_KEY, Tag, escape_attr + +if TYPE_CHECKING: + from ._context_layer import ContextLayer + from ._data_source import DataSource __all__ = [ "CitationDecision", @@ -187,6 +192,52 @@ def match_citation(quote: str, corpus: Sequence[CorpusEntry]) -> CorpusEntry | N ) +def build_citation_corpus( + context_layer: ContextLayer | None, + measures: Iterable[Measure], + sources: Mapping[str, DataSource], +) -> list[CorpusEntry]: + """Collect the trusted text an answer's citations are verified against. + + Which text is citable and under which label is a cross-language contract + pinned by ``tests/shared/citation-corpus.json``. Matching returns the + first entry containing the quote, so the specific sources are added ahead + of the general documentation corpus. + """ + entries: list[CorpusEntry] = [] + + def add(label: str, kind: CitationKind, texts: Iterable[str | None]) -> None: + entries.extend( + CorpusEntry(label=label, kind=kind, text=text) for text in texts if text + ) + + # A measure block names its sources only for an agent that has several, so + # the corpus holds the block search_pool actually presented. + source_names = tuple(sources) if len(sources) > 1 else () + for record in measures: + add( + f"{record.name} definition", + "definition", + [measure_schema_text(record, source_names=source_names)], + ) + + for name, source in sources.items(): + dictionary = source.dictionary + if dictionary is None: + continue + add( + f"{name} dictionary", + "schema", + [dictionary.description, dictionary.details], + ) + for table in dictionary.tables: + add(f"{table} table", "schema", [dictionary.entry_text(table)]) + + docs = context_layer.docs if context_layer is not None else () + add("documentation", "prose", docs) + return entries + + def citation_aside_html(quote: str, explanation: str, label: str, kind: str) -> str: """Render a verified citation as the aside shinychat displays. @@ -204,23 +255,18 @@ def citation_aside_html(quote: str, explanation: str, label: str, kind: str) -> f"{html.escape(label, quote=False)}\n\n" ) return ( - f'' + f'' f"{title}{reason}{blockquote}" ) -# Ampersands first, so the entities this generates are not escaped again. -def _escape_attr(text: str) -> str: - return text.replace("&", "&").replace('"', """) - - def tool_result(value: Any, tag: Tag | None = None) -> ContentToolResult: """A tool result carrying the provenance tag of the output it holds. The tag is read back off ``extra`` when the turn is classified, so it is set here rather than at the point a result is added to the conversation. """ - return ContentToolResult(value=value, extra={"commons_tag": tag}) + return ContentToolResult(value=value, extra={TAG_EXTRA_KEY: tag}) def citation_reminder_text() -> str: diff --git a/pkg-py/src/commons/_provenance.py b/pkg-py/src/commons/_provenance.py index c62a5752..687f71b0 100644 --- a/pkg-py/src/commons/_provenance.py +++ b/pkg-py/src/commons/_provenance.py @@ -1,8 +1,9 @@ """A/B/C provenance: how much an answer can be trusted. -The truth table and the display copy are a cross-language contract pinned by -``tests/shared/provenance.json``; change that fixture, not just this file. -``pkg-r/R/provenance.R`` implements the same contract for R. +The truth table, the display copy, and which outcomes render a marker are a +cross-language contract pinned by ``tests/shared/provenance.json``; change +that fixture, not just this file. ``pkg-r/R/provenance.R`` implements the same +contract for R. """ from __future__ import annotations @@ -10,8 +11,24 @@ import enum from collections.abc import Sequence from dataclasses import dataclass +from typing import Final -__all__ = ["PROVENANCE_DISPLAY", "ProvenanceDisplay", "Tag", "derive_provenance_tag"] +from chatlas import ContentToolResult, Turn + +__all__ = [ + "PROVENANCE_DISPLAY", + "TAG_EXTRA_KEY", + "ProvenanceDisplay", + "Tag", + "collect_appended_tags", + "derive_provenance_tag", + "escape_attr", + "provenance_aside", +] + +# Where a tool records how much its result can be trusted. R writes the same +# key, so a trajectory written by either package classifies in both. +TAG_EXTRA_KEY: Final = "commons_tag" class Tag(enum.StrEnum): @@ -80,3 +97,57 @@ def derive_provenance_tag(tags: Sequence[Tag], verified: bool) -> Tag | None: if Tag.A in tags: return Tag.A return None + + +def collect_appended_tags(turns: Sequence[Turn], from_index: int) -> list[Tag]: + """Gather the tags the tools of one exchange set on their results. + + ``from_index`` is the turn count read before the exchange started, so a + tag an earlier answer earned cannot classify this one. + """ + tags: list[Tag] = [] + for turn in turns[from_index:]: + for content in turn.contents: + if not isinstance(content, ContentToolResult): + continue + value = (content.extra or {}).get(TAG_EXTRA_KEY) + try: + tags.append(Tag(value)) + # Nothing validates `extra`, and a restored conversation arrives + # as JSON: an unreadable tag must not cost the exchange the tags + # that are readable. + except ValueError: + continue + return tags + + +# Ampersands first, so the entities this generates are not escaped again. +def escape_attr(text: str) -> str: + return text.replace("&", "&").replace('"', """) + + +# Upgrades when the UI mounts the aside, and renders as nothing until then. +_INFO_CONTROL: Final = ( + '' + "" +) + + +def provenance_aside(tag: Tag | None, *, include_cited: bool = False) -> str: + """Render the marker that follows a classified answer. + + Which outcomes render is a cross-language contract pinned by + ``tests/shared/provenance.json``. A live answer omits the "Cited" marker, + because the verified citation's own aside already says as much; a review + context passes ``include_cited`` to see every outcome. + + No icon: its URL comes from the served asset bundle, which arrives with + the Python UI (see ``citation_aside_html``). + """ + if tag is None or (tag is Tag.B and not include_cited): + return "" + display = PROVENANCE_DISPLAY[tag] + return ( + f'' + f"{display.body} {_INFO_CONTROL}" + ) diff --git a/pkg-py/src/commons/_reminders.py b/pkg-py/src/commons/_reminders.py new file mode 100644 index 00000000..1a0ad520 --- /dev/null +++ b/pkg-py/src/commons/_reminders.py @@ -0,0 +1,53 @@ +"""Reminders appended to a user's turn. + +The wording and which models earn the concise reminder are a cross-language +contract pinned by ``tests/shared/turn-reminders.json``; change that fixture, +not just this file. ``pkg-r/R/turn-reminder.R`` holds the same contract for R. +""" + +from __future__ import annotations + +from collections.abc import Sequence +from typing import Any, Final + +from chatlas.types import ContentText + +from ._prompt import is_claude_5_model + +__all__ = [ + "CLAUDE_5_TURN_REMINDER", + "RESTORED_CONVERSATION_REMINDER", + "ContentTurnReminder", + "append_restored_conversation_reminder", + "append_turn_reminder", +] + + +class ContentTurnReminder(ContentText): + """Text the model reads and the UI leaves out. + + A provider only accepts content it knows, so a reminder travels as text; + its own type is what lets a UI recognize and skip it. + """ + + +CLAUDE_5_TURN_REMINDER: Final = "Be concise as a default." + +RESTORED_CONVERSATION_REMINDER: Final = ( + "The Python state associated with this restored conversation is " + "unavailable. Do not assume that objects, loaded packages, or result " + "handles from earlier run_python calls still exist. Re-run needed tools, " + "recreate objects, and reload packages before continuing." +) + + +def append_turn_reminder(inputs: Sequence[Any], model: str | None) -> list[Any]: + """Add the concise reminder for the models that need it.""" + if not is_claude_5_model(model): + return list(inputs) + return [*inputs, ContentTurnReminder(text=CLAUDE_5_TURN_REMINDER)] + + +def append_restored_conversation_reminder(inputs: Sequence[Any]) -> list[Any]: + """Tell the model that the session behind its earlier results is gone.""" + return [*inputs, ContentTurnReminder(text=RESTORED_CONVERSATION_REMINDER)] diff --git a/pkg-py/tests/test_citation_corpus.py b/pkg-py/tests/test_citation_corpus.py new file mode 100644 index 00000000..aabcd766 --- /dev/null +++ b/pkg-py/tests/test_citation_corpus.py @@ -0,0 +1,96 @@ +"""What an agent's answers can be cited against, driven by the shared fixture. + +The labels, the kinds, and the order entries are added in are a cross-language +contract, so the cases live in ``tests/shared/citation-corpus.json`` and the R +suite runs the same ones. Do not restate a case here; add it to the fixture. +""" + +import inspect +from typing import Any + +import pandas as pd +import pytest + +from commons import ContextLayer, data_source +from commons._citations import build_citation_corpus, match_citation +from commons._data_dictionary import DataDictionary +from commons._data_source import DataSource +from commons._measures import Injected, Measure, as_measure, measure + +from ._shared import load_shared_fixture + +CASES: list[dict[str, Any]] = load_shared_fixture("citation-corpus")[ + "build_citation_corpus" +]["cases"] + + +def _fixture_measure(spec: dict[str, Any]) -> Measure: + """Build a measure from a fixture spec through the production decorator. + + Every argument a case declares is commons-supplied, so the generated + function carries an ``Injected`` annotation per name and no model-visible + arguments at all: what the corpus needs from a measure is the block + ``search_pool`` renders, and measure-schema.json pins the arguments half + of that block already. + """ + + def func(*args: Any, **kwargs: Any) -> None: + return None + + injected: list[str] = spec.get("injected") or [] + func.__name__ = spec["name"] + func.__annotations__ = {name: Injected[Any] for name in injected} + func.__signature__ = inspect.Signature( # type: ignore[attr-defined] + [ + inspect.Parameter( + name, inspect.Parameter.POSITIONAL_OR_KEYWORD, annotation=Injected[Any] + ) + for name in injected + ] + ) + record = as_measure(measure(description=spec["description"])(func)) + assert record is not None + return record + + +def _fixture_source(spec: dict[str, Any]) -> DataSource: + """A real source over an in-memory DuckDB, carrying the case's dictionary.""" + dictionary = ( + None + if spec["dictionary"] is None + else DataDictionary.model_validate(spec["dictionary"]) + ) + return data_source(sales=pd.DataFrame({"revenue": [1.0]}), dictionary=dictionary) + + +def test_the_fixture_is_not_empty() -> None: + # An empty case list would make the parametrized test below vacuously pass. + assert CASES + + +@pytest.mark.parametrize("case", CASES, ids=lambda case: case["name"]) +def test_the_corpus_matches_the_shared_fixture(case: dict[str, Any]) -> None: + layer = None if case["docs"] is None else ContextLayer(case["docs"]) + measures = [_fixture_measure(spec) for spec in case["measures"]] + sources = {spec["name"]: _fixture_source(spec) for spec in case["sources"]} + + corpus = build_citation_corpus(layer, measures, sources) + + assert [(entry.label, entry.kind) for entry in corpus] == [ + (entry["label"], entry["kind"]) for entry in case["expected"] + ] + for want in case["matches"]: + found = match_citation(want["quote"], corpus) + label = found.label if found is not None else None + assert label == want["label"], want["quote"] + + +def test_every_entry_carries_the_text_it_was_built_from() -> None: + # The fixture pins labels and order; an entry with no text would still + # satisfy it, and would silently make its source uncitable. + dictionary = DataDictionary.model_validate({"details": "Revenue excludes tax."}) + source = data_source(sales=pd.DataFrame({"revenue": [1.0]}), dictionary=dictionary) + + corpus = build_citation_corpus(ContextLayer(["A note."]), [], {"sales_db": source}) + + assert [entry.text for entry in corpus] == ["Revenue excludes tax.", "A note."] diff --git a/pkg-py/tests/test_provenance.py b/pkg-py/tests/test_provenance.py index f7b87878..eb261df7 100644 --- a/pkg-py/tests/test_provenance.py +++ b/pkg-py/tests/test_provenance.py @@ -9,14 +9,23 @@ from typing import Any import pytest +from chatlas import AssistantTurn, ContentToolResult, UserTurn +from chatlas.types import ContentText -from commons._provenance import PROVENANCE_DISPLAY, Tag, derive_provenance_tag +from commons._provenance import ( + PROVENANCE_DISPLAY, + Tag, + collect_appended_tags, + derive_provenance_tag, + provenance_aside, +) from ._shared import load_shared_fixture SPEC = load_shared_fixture("provenance") DERIVATION_CASES: list[dict[str, Any]] = SPEC["derive_provenance_tag"]["cases"] DISPLAY: dict[str, Any] = SPEC["provenance_display"]["tags"] +ASIDE_CASES: list[dict[str, Any]] = SPEC["provenance_aside"]["cases"] def test_shared_fixture_covers_every_outcome() -> None: @@ -70,3 +79,100 @@ def test_tag_formats_as_the_bare_string() -> None: def test_display_copy_is_immutable() -> None: with pytest.raises(FrozenInstanceError): PROVENANCE_DISPLAY[Tag.A].label = "Something else" # type: ignore[misc] + + +def _tool_result(tag: Any) -> ContentToolResult: + return ContentToolResult(value="42", extra={"commons_tag": tag}) + + +def test_collects_the_tags_tool_results_set_in_the_appended_turns() -> None: + turns = [ + UserTurn([_tool_result(Tag.A)]), + UserTurn([_tool_result(Tag.B)]), + ] + + assert collect_appended_tags(turns, 0) == [Tag.A, Tag.B] + + +def test_ignores_turns_before_the_index() -> None: + # The index is taken before a turn starts, so tags an earlier exchange set + # must not classify this one. + turns = [ + UserTurn([_tool_result(Tag.B)]), + UserTurn([_tool_result(Tag.A)]), + ] + + assert collect_appended_tags(turns, 1) == [Tag.A] + + +def test_an_index_past_the_last_turn_collects_nothing() -> None: + assert collect_appended_tags([AssistantTurn([ContentText(text="hi")])], 1) == [] + + +def test_ignores_content_without_a_tag() -> None: + turns = [ + UserTurn( + [ + ContentText(text="Revenue was 42."), + ContentToolResult(value="42"), + _tool_result(None), + _tool_result(Tag.A), + ], + ) + ] + + assert collect_appended_tags(turns, 0) == [Tag.A] + + +def test_reads_a_tag_that_deserialized_to_a_plain_string() -> None: + # A restored conversation arrives as JSON, so `extra` holds "A" rather + # than the enum member the tool set. + turns = [UserTurn([_tool_result("A")])] + + assert collect_appended_tags(turns, 0) == [Tag.A] + + +def test_ignores_a_tag_value_that_is_not_an_outcome() -> None: + # Nothing validates `extra`, and an unrecognized tag must not abort the + # turn it appears in: the other tags still classify the answer. + turns = [UserTurn([_tool_result("Z"), _tool_result(Tag.B)])] + + assert collect_appended_tags(turns, 0) == [Tag.B] + + +@pytest.mark.parametrize("case", ASIDE_CASES, ids=lambda case: case["name"]) +def test_the_marker_follows_the_shared_fixture(case: dict[str, Any]) -> None: + tag = None if case["tag"] is None else Tag(case["tag"]) + + aside = provenance_aside(tag, include_cited=case["include_cited"]) + + if not case["emits"]: + assert aside == "" + return + assert tag is not None + display = PROVENANCE_DISPLAY[tag] + assert aside.startswith(f'') + assert display.body in aside + + +def test_the_shared_fixture_covers_both_outcomes_of_every_tag() -> None: + # A fixture that lost its emits: false cases would still pass every + # assertion above. + assert {case["emits"] for case in ASIDE_CASES} == {True, False} + assert {case["tag"] for case in ASIDE_CASES} == {"A", "B", "C", None} + + +def test_the_marker_carries_the_info_control() -> None: + # The custom element upgrades when the UI mounts the aside, and renders as + # nothing until then, so it is emitted before that UI exists. + assert ( + '' + "" in provenance_aside(Tag.A) + ) + + +def test_the_marker_names_no_icon_yet() -> None: + # The icon URL comes from the served asset bundle, which arrives with the + # Python UI. R emits one; a bare filename here would 404. + assert "icon=" not in provenance_aside(Tag.A) + assert "data:image" not in provenance_aside(Tag.A) diff --git a/pkg-py/tests/test_reminders.py b/pkg-py/tests/test_reminders.py new file mode 100644 index 00000000..3dcb6ce2 --- /dev/null +++ b/pkg-py/tests/test_reminders.py @@ -0,0 +1,81 @@ +"""Reminders appended to a user turn, driven by the shared fixture. + +Which models earn the concise reminder, and the wording of both reminders, are +cross-language contracts, so the cases live in ``tests/shared/turn-reminders.json`` +and the R suite runs the same ones. Do not restate a case here; add it to the +fixture. +""" + +from typing import Any + +import pytest +from chatlas.types import ContentText + +from commons._reminders import ( + CLAUDE_5_TURN_REMINDER, + RESTORED_CONVERSATION_REMINDER, + ContentTurnReminder, + append_restored_conversation_reminder, + append_turn_reminder, +) + +from ._shared import load_shared_fixture + +SPEC = load_shared_fixture("turn-reminders") +CONCISE: dict[str, Any] = SPEC["claude_5_turn_reminder"] +RESTORED: dict[str, Any] = SPEC["restored_conversation_reminder"] + + +def test_the_fixture_covers_models_that_do_and_do_not_earn_the_reminder() -> None: + assert {case["appended"] for case in CONCISE["cases"]} == {True, False} + + +@pytest.mark.parametrize("case", CONCISE["cases"], ids=lambda case: case["name"]) +def test_the_concise_reminder_follows_the_shared_fixture(case: dict[str, Any]) -> None: + inputs = append_turn_reminder(["What was revenue?"], case["model"]) + + if not case["appended"]: + assert inputs == ["What was revenue?"] + return + assert len(inputs) == 2 + assert isinstance(inputs[1], ContentTurnReminder) + assert inputs[1].text == CONCISE["text"] + + +def test_the_concise_reminder_text_matches_the_shared_fixture() -> None: + assert CLAUDE_5_TURN_REMINDER == CONCISE["text"] + + +def test_the_restored_reminder_renders_the_shared_wording() -> None: + expected = RESTORED["template"].format(**RESTORED["substitutions"]["python"]) + + assert RESTORED_CONVERSATION_REMINDER == expected + + +def test_the_restored_reminder_is_appended_after_the_prompt() -> None: + inputs = append_restored_conversation_reminder(["What was revenue?"]) + + assert len(inputs) == 2 + assert inputs[0] == "What was revenue?" + assert isinstance(inputs[1], ContentTurnReminder) + assert inputs[1].text == RESTORED_CONVERSATION_REMINDER + + +def test_appending_leaves_the_caller_s_inputs_alone() -> None: + # The caller's list is the turn's own contents; appending in place would + # add a second reminder on every retry of that turn. + inputs: list[Any] = ["What was revenue?"] + + append_turn_reminder(inputs, "claude-sonnet-5") + append_restored_conversation_reminder(inputs) + + assert inputs == ["What was revenue?"] + + +def test_a_reminder_is_text_the_model_reads() -> None: + # Providers only accept content they know, so the reminder has to be a + # kind of text; being its own type is what lets a UI leave it out. + reminder = ContentTurnReminder(text=CLAUDE_5_TURN_REMINDER) + + assert isinstance(reminder, ContentText) + assert reminder.content_type == "text" diff --git a/pkg-r/tests/testthat/fixtures/shared/citation-corpus.json b/pkg-r/tests/testthat/fixtures/shared/citation-corpus.json new file mode 100644 index 00000000..ddb4f55a --- /dev/null +++ b/pkg-r/tests/testthat/fixtures/shared/citation-corpus.json @@ -0,0 +1,154 @@ +{ + "description": "The trusted text an answer's citations are verified against. The source is tests/shared/citation-corpus.json; the copy under pkg-r/tests/testthat/fixtures/shared/ is generated by scripts/sync-shared.sh. Edit the source and re-run that script.", + "build_citation_corpus": { + "description": "Every case builds a corpus from a context layer's documents, a list of measures, and named data sources whose dictionaries describe a table called `sales`. `expected` names every entry the corpus holds, in order, because matching returns the first entry whose text contains the quote: specific sources are added before the general documentation corpus, so prose that is both a dictionary's and a document's keeps the dictionary's label. `matches` quotes text from the inputs and names the entry that quote must resolve to, or null when nothing may match it. A measure contributes the same block search_pool shows the model, so a quote from that block is citable; measure text itself is pinned by measure-schema.json.", + "cases": [ + { + "name": "each kind of trusted text carries its own label and kind", + "docs": ["Fiscal year starts in February."], + "measures": [ + { + "name": "order_count", + "description": "Count orders, optionally filtered by region." + } + ], + "sources": [ + { + "name": "sales_db", + "dictionary": { + "description": "Order and revenue data for a small retailer.", + "details": "Revenue excludes tax collected at checkout.", + "tables": [ + { + "name": "sales", + "description": "One row per booked order." + } + ] + } + } + ], + "expected": [ + { "label": "order_count definition", "kind": "definition" }, + { "label": "sales_db dictionary", "kind": "schema" }, + { "label": "sales_db dictionary", "kind": "schema" }, + { "label": "sales table", "kind": "schema" }, + { "label": "documentation", "kind": "prose" } + ], + "matches": [ + { + "quote": "Count orders, optionally filtered by region.", + "label": "order_count definition" + }, + { + "quote": "Order and revenue data for a small retailer.", + "label": "sales_db dictionary" + }, + { + "quote": "Revenue excludes tax collected at checkout.", + "label": "sales_db dictionary" + }, + { "quote": "One row per booked order.", "label": "sales table" }, + { + "quote": "Fiscal year starts in February.", + "label": "documentation" + }, + { "quote": "revenue", "label": null } + ] + }, + { + "name": "a measure block names no source for a single-source agent", + "docs": [], + "measures": [ + { + "name": "order_count", + "description": "Count orders.", + "injected": ["sales_db"] + } + ], + "sources": [{ "name": "sales_db", "dictionary": null }], + "expected": [ + { "label": "order_count definition", "kind": "definition" } + ], + "matches": [ + { "quote": "sources: sales_db", "label": null }, + { "quote": "### order_count\nCount orders.", "label": "order_count definition" } + ] + }, + { + "name": "a measure block names the source it takes once there are several", + "docs": [], + "measures": [ + { + "name": "order_count", + "description": "Count orders.", + "injected": ["sales_db"] + } + ], + "sources": [ + { "name": "sales_db", "dictionary": null }, + { "name": "returns_db", "dictionary": null } + ], + "expected": [ + { "label": "order_count definition", "kind": "definition" } + ], + "matches": [ + { + "quote": "Count orders.\n\nsources: sales_db", + "label": "order_count definition" + } + ] + }, + { + "name": "prose that is also a document keeps its dictionary label", + "docs": ["Revenue excludes tax collected at checkout."], + "measures": [], + "sources": [ + { + "name": "sales_db", + "dictionary": { + "details": "Revenue excludes tax collected at checkout." + } + } + ], + "expected": [ + { "label": "sales_db dictionary", "kind": "schema" }, + { "label": "documentation", "kind": "prose" } + ], + "matches": [ + { + "quote": "Revenue excludes tax collected at checkout.", + "label": "sales_db dictionary" + } + ] + }, + { + "name": "a source with no prose and no documents contributes nothing", + "docs": [], + "measures": [], + "sources": [{ "name": "sales_db", "dictionary": null }], + "expected": [], + "matches": [{ "quote": "Revenue excludes tax.", "label": null }] + }, + { + "name": "an agent with no context layer still cites its dictionary", + "docs": null, + "measures": [], + "sources": [ + { + "name": "sales_db", + "dictionary": { + "details": "Revenue excludes tax collected at checkout." + } + } + ], + "expected": [{ "label": "sales_db dictionary", "kind": "schema" }], + "matches": [ + { + "quote": "Revenue excludes tax collected at checkout.", + "label": "sales_db dictionary" + } + ] + } + ] + } +} diff --git a/pkg-r/tests/testthat/fixtures/shared/provenance.json b/pkg-r/tests/testthat/fixtures/shared/provenance.json index befa6a8d..36631894 100644 --- a/pkg-r/tests/testthat/fixtures/shared/provenance.json +++ b/pkg-r/tests/testthat/fixtures/shared/provenance.json @@ -94,5 +94,52 @@ "pill_class": "caution" } } + }, + "provenance_aside": { + "description": "The provenance marker appended after a classified answer. A live answer omits the \"Cited\" marker, because the verified citation's own aside already says as much; a review context asks for every outcome by passing include_cited. `emits` is false when nothing is rendered at all. A rendered marker carries that tag's label and body from provenance_display verbatim. The icon is not pinned here: its URL comes from each package's own served asset bundle.", + "cases": [ + { + "name": "a verified answer renders its marker", + "tag": "A", + "include_cited": false, + "emits": true + }, + { + "name": "an untrusted answer renders its marker", + "tag": "C", + "include_cited": false, + "emits": true + }, + { + "name": "a cited answer renders nothing beside its own citation", + "tag": "B", + "include_cited": false, + "emits": false + }, + { + "name": "a cited answer renders its marker in a review context", + "tag": "B", + "include_cited": true, + "emits": true + }, + { + "name": "including cited markers leaves the other outcomes alone", + "tag": "A", + "include_cited": true, + "emits": true + }, + { + "name": "an unclassified answer renders nothing", + "tag": null, + "include_cited": false, + "emits": false + }, + { + "name": "an unclassified answer renders nothing in a review context", + "tag": null, + "include_cited": true, + "emits": false + } + ] } } diff --git a/pkg-r/tests/testthat/fixtures/shared/turn-reminders.json b/pkg-r/tests/testthat/fixtures/shared/turn-reminders.json new file mode 100644 index 00000000..00dca076 --- /dev/null +++ b/pkg-r/tests/testthat/fixtures/shared/turn-reminders.json @@ -0,0 +1,40 @@ +{ + "description": "Reminders appended to a user's turn, which the model reads and the UI does not show. The source is tests/shared/turn-reminders.json; the copy under pkg-r/tests/testthat/fixtures/shared/ is generated by scripts/sync-shared.sh. Edit the source and re-run that script.", + "claude_5_turn_reminder": { + "description": "Claude 5 models are verbose without it, and every other model is not, so the reminder is appended for those models alone. `appended` is what the model id in the same case earns; `text` is what gets appended, and is identical in both packages.", + "text": "Be concise as a default.", + "cases": [ + { "name": "a bare Claude 5 model", "model": "claude-sonnet-5", "appended": true }, + { + "name": "a Claude 5 model behind a provider prefix", + "model": "anthropic/claude-opus-5", + "appended": true + }, + { + "name": "a Claude 5 model on Bedrock", + "model": "us.anthropic.claude-fable-5", + "appended": true + }, + { + "name": "a Claude 5 model on Databricks", + "model": "databricks-claude-sonnet-5", + "appended": true + }, + { + "name": "an earlier Claude whose version ends in 5", + "model": "claude-sonnet-4-5", + "appended": false + }, + { "name": "another vendor's model", "model": "gpt-5.4", "appended": false }, + { "name": "an unknown model", "model": null, "appended": false } + ] + }, + "restored_conversation_reminder": { + "description": "Appended when a conversation is restored from a trajectory: the worker session that produced its earlier tool results is gone, so nothing the model built in that session still exists. The wording is shared and the two packages differ only in the language and the execution tool they name, which `substitutions` records; each package renders `template` with its own values and must produce its reminder exactly.", + "template": "The {language} state associated with this restored conversation is unavailable. Do not assume that objects, loaded packages, or result handles from earlier {tool} calls still exist. Re-run needed tools, recreate objects, and reload packages before continuing.", + "substitutions": { + "r": { "language": "R", "tool": "run_r" }, + "python": { "language": "Python", "tool": "run_python" } + } + } +} diff --git a/pkg-r/tests/testthat/test-citations.R b/pkg-r/tests/testthat/test-citations.R index 58fcce7f..b96e3eef 100644 --- a/pkg-r/tests/testthat/test-citations.R +++ b/pkg-r/tests/testthat/test-citations.R @@ -75,169 +75,61 @@ test_that("citation asides leave trust copy to their markers", { ) }) -test_that("corpus entries carry a kind and a reader-facing label", { - skip_if_not_installed("yaml") - doc <- withr::local_tempfile(fileext = ".md") - writeLines("Fiscal year starts in February.", doc) - path <- withr::local_tempfile(fileext = ".yaml") - writeLines( - c( - '$version: "0.1.0"', - "name: retail sales", - "description: Order and revenue data for a small retailer.", - "tables:", - " - name: sales", - " columns:", - " - name: revenue", - " description: Booked revenue, net of discounts." - ), - path - ) - source <- data_source(sales = test_sales(), dictionary = path) - - corpus <- build_citation_corpus( - augment_context_layer(context_layer(files = doc), list(source)), - list(order_count = count_measure_tool()), - list(sales_db = source) - ) - - expect_equal( - match_citation("Fiscal year starts in February.", corpus), - list(label = "documentation", kind = "prose") - ) - expect_equal( - match_citation( - "Count orders, optionally filtered by region and a revenue ceiling.", - corpus - ), - list(label = "order_count definition", kind = "definition") - ) - expect_equal( - match_citation("Booked revenue, net of discounts.", corpus), - list(label = "sales table", kind = "schema") - ) - expect_equal( - match_citation("Order and revenue data for a small retailer.", corpus), - list(label = "sales_db dictionary", kind = "schema") - ) - expect_null(match_citation("tax", corpus)) -}) - -test_that("the citation corpus spans context, measures, and dictionaries", { - skip_if_not_installed("yaml") - doc <- withr::local_tempfile(fileext = ".md") - writeLines("Fiscal year starts in February.", doc) - layer <- context_layer(files = doc) - registry <- list(order_count = count_measure_tool()) - path <- withr::local_tempfile(fileext = ".yaml") - writeLines( - c( - '$version: "0.1.0"', - "name: retail sales", - "tables:", - " - name: sales", - " description: One row per order line.", - " columns:", - " - name: revenue", - " description: Booked revenue, net of discounts." - ), - path - ) - source <- data_source(sales = test_sales(), dictionary = path) - - corpus <- build_citation_corpus( - augment_context_layer(layer, list(source)), - registry, - list(source) - ) - - expect_false(is.null(match_citation( - "Fiscal year starts in February.", - corpus - ))) - expect_equal( - match_citation( - "Count orders, optionally filtered by region and a revenue ceiling.", - corpus - )$label, - "order_count definition" - ) - expect_equal( - match_citation("Booked revenue, net of discounts.", corpus)$label, - "sales table" - ) -}) +# A measure the fixture describes: its arguments are all commons-supplied, so +# `arguments` stays empty and every formal is an injected source. +fixture_corpus_measure <- function(spec) { + injected <- as.character(unlist(spec$injected) %||% character()) + fn <- if (length(injected) == 0) { + function() NULL + } else { + formals <- rep(list(rlang::missing_arg()), length(injected)) + names(formals) <- injected + rlang::new_function(formals, quote(NULL)) + } + measure(spec$name, spec$description, fn) +} -test_that("dictionary prose keeps its specific label once it is also context", { - skip_if_not_installed("yaml") - path <- withr::local_tempfile(fileext = ".yaml") - writeLines( - c( - '$version: "0.1.0"', - "name: retail sales", - "details: Revenue figures exclude tax collected at checkout.", - "tables:", - " - name: sales", - " description: One row per order line.", - " details: Refunds appear as negative-revenue rows." - ), - path - ) - source <- data_source(sales = test_sales(), dictionary = path) - own_doc <- withr::local_tempfile(fileext = ".md") - writeLines("Fiscal year starts in February.", own_doc) +test_that("build_citation_corpus matches the shared fixture", { + cases <- shared_fixture("citation-corpus")$build_citation_corpus$cases + # An empty fixture would make the loop below vacuously succeed. + expect_gt(length(cases), 0) - corpus <- build_citation_corpus( - augment_context_layer(context_layer(files = own_doc), list(source)), - list(), - list(source) - ) + for (case in cases) { + layer <- if (is.null(case$docs)) { + NULL + } else { + new_context_layer(as.character(unlist(case$docs))) + } + registry <- lapply(case$measures, fixture_corpus_measure) + sources <- lapply(case$sources, function(spec) { + dictionary <- if (is.null(spec$dictionary)) { + NULL + } else { + new_data_dictionary(spec$dictionary) + } + suppressMessages(data_source(sales = test_sales(), dictionary = dictionary)) + }) + names(sources) <- vapply(case$sources, function(spec) spec$name, character(1)) - expect_equal( - match_citation("One row per order line.", corpus)$label, - "sales table" - ) - expect_equal( - match_citation("Refunds appear as negative-revenue rows.", corpus)$label, - "sales table" - ) - expect_equal( - match_citation( - "Revenue figures exclude tax collected at checkout.", - corpus - )$label, - "data dictionary" - ) - expect_equal( - match_citation("Fiscal year starts in February.", corpus)$label, - "documentation" - ) -}) + corpus <- build_citation_corpus(layer, registry, sources) -test_that("corpus measure text matches multi-source presentation", { - registry <- list( - region_revenue = measure( - "region_revenue", - "Total revenue for a region.", - function(region, sales_db) NULL, - arguments = list(region = ellmer::type_string("The sales region.")) + expect_identical( + lapply(corpus, function(entry) entry[c("label", "kind")]), + lapply(case$expected, function(entry) entry[c("label", "kind")]), + info = case$name ) - ) - sources <- list(sales_db = test_source(), crm = test_source()) - - corpus <- build_citation_corpus(NULL, registry, sources) - - # search_pool presents a `sources:` line to multi-source agents; a - # verbatim quote spanning it must verify. - expect_equal( - match_citation( - "Total revenue for a region.\n\nsources: sales_db", - corpus - )$label, - "region_revenue definition" - ) + for (want in case$matches) { + found <- match_citation(want$quote, corpus) + expect_identical( + found$label, + want$label, + info = paste(case$name, want$quote, sep = ": ") + ) + } + } }) + test_that("dataset-level dictionary prose is citable", { skip_if_not_installed("yaml") path <- withr::local_tempfile(fileext = ".yaml") diff --git a/pkg-r/tests/testthat/test-provenance.R b/pkg-r/tests/testthat/test-provenance.R index d2c0f913..6b650ec3 100644 --- a/pkg-r/tests/testthat/test-provenance.R +++ b/pkg-r/tests/testthat/test-provenance.R @@ -38,31 +38,47 @@ test_that("provenance_display uses R display copy", { } }) -test_that("provenance_aside renders A and C, nothing for B/NA", { +test_that("provenance_aside matches the shared fixture", { + cases <- shared_fixture("provenance")$provenance_aside$cases + # An empty fixture would make the loop below vacuously succeed. + expect_gt(length(cases), 0) + + for (case in cases) { + tag <- case$tag %||% NA_character_ + aside <- provenance_aside(tag, include_cited = case$include_cited) + + if (!isTRUE(case$emits)) { + expect_identical(aside, "", info = case$name) + next + } + entry <- provenance_display[[tag]] + expect_match( + aside, + paste0('^Be concise as a default.", + "cases": [ + { "name": "a bare Claude 5 model", "model": "claude-sonnet-5", "appended": true }, + { + "name": "a Claude 5 model behind a provider prefix", + "model": "anthropic/claude-opus-5", + "appended": true + }, + { + "name": "a Claude 5 model on Bedrock", + "model": "us.anthropic.claude-fable-5", + "appended": true + }, + { + "name": "a Claude 5 model on Databricks", + "model": "databricks-claude-sonnet-5", + "appended": true + }, + { + "name": "an earlier Claude whose version ends in 5", + "model": "claude-sonnet-4-5", + "appended": false + }, + { "name": "another vendor's model", "model": "gpt-5.4", "appended": false }, + { "name": "an unknown model", "model": null, "appended": false } + ] + }, + "restored_conversation_reminder": { + "description": "Appended when a conversation is restored from a trajectory: the worker session that produced its earlier tool results is gone, so nothing the model built in that session still exists. The wording is shared and the two packages differ only in the language and the execution tool they name, which `substitutions` records; each package renders `template` with its own values and must produce its reminder exactly.", + "template": "The {language} state associated with this restored conversation is unavailable. Do not assume that objects, loaded packages, or result handles from earlier {tool} calls still exist. Re-run needed tools, recreate objects, and reload packages before continuing.", + "substitutions": { + "r": { "language": "R", "tool": "run_r" }, + "python": { "language": "Python", "tool": "run_python" } + } + } +} From 53333fc53578a2c3db9f2971d2a1693348e8c62c Mon Sep 17 00:00:00 2001 From: Josh Taillon Date: Mon, 7 Sep 2026 13:54:21 -0600 Subject: [PATCH 2/5] test: pin collect_appended_tags collection in the shared fixture The contract was hand-written in both suites: tags gather only from turns at or after the index, content without a tag is skipped. A per-language copy of a shared behavior drifts, so provenance.json gains a collect_appended_tags section both suites run. The fixture's skip counts the turns present when the exchange began, since from_index is 0-based in Python and 1-based in R. A tag value outside A, B, and C stays deliberately unpinned: Python drops it at collection, R returns it, and derive_provenance_tag ignores anything but A and B either way. --- pkg-py/tests/test_provenance.py | 68 ++++++++----------- .../testthat/fixtures/shared/provenance.json | 47 +++++++++++++ pkg-r/tests/testthat/test-commons.R | 51 -------------- pkg-r/tests/testthat/test-provenance.R | 34 ++++++++++ tests/shared/provenance.json | 47 +++++++++++++ 5 files changed, 156 insertions(+), 91 deletions(-) diff --git a/pkg-py/tests/test_provenance.py b/pkg-py/tests/test_provenance.py index eb261df7..9a83799b 100644 --- a/pkg-py/tests/test_provenance.py +++ b/pkg-py/tests/test_provenance.py @@ -14,6 +14,7 @@ from commons._provenance import ( PROVENANCE_DISPLAY, + TAG_EXTRA_KEY, Tag, collect_appended_tags, derive_provenance_tag, @@ -26,6 +27,7 @@ DERIVATION_CASES: list[dict[str, Any]] = SPEC["derive_provenance_tag"]["cases"] DISPLAY: dict[str, Any] = SPEC["provenance_display"]["tags"] ASIDE_CASES: list[dict[str, Any]] = SPEC["provenance_aside"]["cases"] +COLLECT_CASES: list[dict[str, Any]] = SPEC["collect_appended_tags"]["cases"] def test_shared_fixture_covers_every_outcome() -> None: @@ -81,62 +83,48 @@ def test_display_copy_is_immutable() -> None: PROVENANCE_DISPLAY[Tag.A].label = "Something else" # type: ignore[misc] -def _tool_result(tag: Any) -> ContentToolResult: - return ContentToolResult(value="42", extra={"commons_tag": tag}) +def _content(spec: dict[str, Any]) -> Any: + if spec["type"] == "text": + return ContentText(text=spec["text"]) + if "tag" not in spec: + return ContentToolResult(value="42") + return ContentToolResult(value="42", extra={TAG_EXTRA_KEY: spec["tag"]}) -def test_collects_the_tags_tool_results_set_in_the_appended_turns() -> None: +@pytest.mark.parametrize("case", COLLECT_CASES, ids=lambda case: case["name"]) +def test_collection_matches_the_shared_fixture(case: dict[str, Any]) -> None: turns = [ - UserTurn([_tool_result(Tag.A)]), - UserTurn([_tool_result(Tag.B)]), + (AssistantTurn if turn["role"] == "assistant" else UserTurn)( + [_content(content) for content in turn["contents"]] + ) + for turn in case["turns"] ] - assert collect_appended_tags(turns, 0) == [Tag.A, Tag.B] - - -def test_ignores_turns_before_the_index() -> None: - # The index is taken before a turn starts, so tags an earlier exchange set - # must not classify this one. - turns = [ - UserTurn([_tool_result(Tag.B)]), - UserTurn([_tool_result(Tag.A)]), + assert collect_appended_tags(turns, case["skip"]) == [ + Tag(tag) for tag in case["expected"] ] - assert collect_appended_tags(turns, 1) == [Tag.A] +def test_the_shared_fixture_covers_collection_edges() -> None: + # A truncated fixture would still pass every parametrized case above. + assert any(case["skip"] > 0 for case in COLLECT_CASES) + assert any(case["expected"] == [] for case in COLLECT_CASES) -def test_an_index_past_the_last_turn_collects_nothing() -> None: - assert collect_appended_tags([AssistantTurn([ContentText(text="hi")])], 1) == [] - -def test_ignores_content_without_a_tag() -> None: +def test_ignores_a_tag_value_that_is_not_an_outcome() -> None: + # Deliberately per-language, so the fixture does not pin it: Python drops + # an unreadable tag at collection, so it cannot cost the exchange the + # tags that are readable. R returns it; derive_provenance_tag ignores + # anything but A and B either way. turns = [ UserTurn( [ - ContentText(text="Revenue was 42."), - ContentToolResult(value="42"), - _tool_result(None), - _tool_result(Tag.A), - ], + ContentToolResult(value="1", extra={TAG_EXTRA_KEY: "Z"}), + ContentToolResult(value="2", extra={TAG_EXTRA_KEY: "B"}), + ] ) ] - assert collect_appended_tags(turns, 0) == [Tag.A] - - -def test_reads_a_tag_that_deserialized_to_a_plain_string() -> None: - # A restored conversation arrives as JSON, so `extra` holds "A" rather - # than the enum member the tool set. - turns = [UserTurn([_tool_result("A")])] - - assert collect_appended_tags(turns, 0) == [Tag.A] - - -def test_ignores_a_tag_value_that_is_not_an_outcome() -> None: - # Nothing validates `extra`, and an unrecognized tag must not abort the - # turn it appears in: the other tags still classify the answer. - turns = [UserTurn([_tool_result("Z"), _tool_result(Tag.B)])] - assert collect_appended_tags(turns, 0) == [Tag.B] diff --git a/pkg-r/tests/testthat/fixtures/shared/provenance.json b/pkg-r/tests/testthat/fixtures/shared/provenance.json index 36631894..09e956c7 100644 --- a/pkg-r/tests/testthat/fixtures/shared/provenance.json +++ b/pkg-r/tests/testthat/fixtures/shared/provenance.json @@ -72,6 +72,53 @@ } ] }, + "collect_appended_tags": { + "description": "Which tags collect_appended_tags(turns, from_index) gathers from one exchange. Each case builds a conversation from `turns`: a turn has a `role` (`user` or `assistant`) and `contents`, where a content item is {\"type\": \"tool_result\", \"tag\": ...} or {\"type\": \"text\", \"text\": ...}. A tool result's `tag` is the commons_tag value the tool set; an absent `tag` key or a null value means the result carries no tag. `skip` counts the turns already present when the exchange began: Python passes it as from_index, R passes skip + 1. `expected` lists the tags collected, in turn order. A tag value outside A, B, and C is deliberately not pinned: Python drops it at collection, R returns it, and the only consumer, derive_provenance_tag, ignores anything but A and B either way.", + "cases": [ + { + "name": "tags are collected across the appended turns", + "turns": [ + {"role": "user", "contents": [{"type": "tool_result", "tag": "A"}]}, + {"role": "user", "contents": [{"type": "tool_result", "tag": "B"}]} + ], + "skip": 0, + "expected": ["A", "B"] + }, + { + "name": "turns already present when the exchange began are ignored", + "turns": [ + {"role": "user", "contents": [{"type": "tool_result", "tag": "B"}]}, + {"role": "user", "contents": [{"type": "tool_result", "tag": "A"}]} + ], + "skip": 1, + "expected": ["A"] + }, + { + "name": "an exchange with no new turns collects nothing", + "turns": [ + {"role": "assistant", "contents": [{"type": "text", "text": "hi"}]} + ], + "skip": 1, + "expected": [] + }, + { + "name": "content without a tag is skipped", + "turns": [ + { + "role": "user", + "contents": [ + {"type": "text", "text": "Revenue was 42."}, + {"type": "tool_result"}, + {"type": "tool_result", "tag": null}, + {"type": "tool_result", "tag": "A"} + ] + } + ], + "skip": 0, + "expected": ["A"] + } + ] + }, "provenance_display": { "description": "Word-for-word copy both UIs render. `icon` is null when the provenance marker has no icon.", "tags": { diff --git a/pkg-r/tests/testthat/test-commons.R b/pkg-r/tests/testthat/test-commons.R index fb68592a..0c7d640b 100644 --- a/pkg-r/tests/testthat/test-commons.R +++ b/pkg-r/tests/testthat/test-commons.R @@ -547,57 +547,6 @@ test_that("commons() records an agent-creation span", { expect_equal(span$attributes[["commons.agent.has_context_layer"]], FALSE) }) -test_that("collect_appended_tags reads commons_tag across tool-calling turns", { - turns <- list( - ellmer::AssistantTurn( - contents = list( - ellmer::ContentToolRequest( - id = "1", - name = "run_sql", - arguments = list() - ) - ) - ), - ellmer::UserTurn( - contents = list( - ellmer::ContentToolResult( - value = "42", - request = NULL, - extra = list(commons_tag = "B") - ) - ) - ), - ellmer::AssistantTurn( - contents = list(ellmer::ContentText(text = "Answer.")) - ) - ) - expect_identical(collect_appended_tags(turns, from_index = 1L), "B") -}) - -test_that("collect_appended_tags ignores turns before from_index", { - turns <- list( - ellmer::UserTurn( - contents = list( - ellmer::ContentToolResult( - value = "1", - request = NULL, - extra = list(commons_tag = "A") - ) - ) - ), - ellmer::UserTurn( - contents = list( - ellmer::ContentToolResult( - value = "2", - request = NULL, - extra = list(commons_tag = "B") - ) - ) - ) - ) - expect_identical(collect_appended_tags(turns, from_index = 2L), "B") -}) - # Split a real ellmer response inside reserved markup to test chunk invariance. stream_citations_fixture <- function(agent, raw, split_at) { skip_if_ellmer_streaming_hooks_unavailable() diff --git a/pkg-r/tests/testthat/test-provenance.R b/pkg-r/tests/testthat/test-provenance.R index 6b650ec3..ceee76f3 100644 --- a/pkg-r/tests/testthat/test-provenance.R +++ b/pkg-r/tests/testthat/test-provenance.R @@ -24,6 +24,40 @@ test_that("derive_provenance_tag matches the shared truth table", { } }) +test_that("collect_appended_tags matches the shared fixture", { + cases <- shared_fixture("provenance")$collect_appended_tags$cases + # An empty fixture would make the loop below vacuously succeed. + expect_gt(length(cases), 0) + + for (case in cases) { + turns <- lapply(case$turns, function(turn) { + contents <- lapply(turn$contents, function(content) { + if (content$type == "text") { + return(ellmer::ContentText(text = content$text)) + } + ellmer::ContentToolResult( + value = "42", + request = NULL, + extra = drop_nulls(list(commons_tag = content$tag)) + ) + }) + if (turn$role == "assistant") { + ellmer::AssistantTurn(contents = contents) + } else { + ellmer::UserTurn(contents = contents) + } + }) + + # `skip` counts the turns present when the exchange began; from_index is + # 1-based here and 0-based in Python. + expect_identical( + collect_appended_tags(turns, from_index = case$skip + 1), + as.character(unlist(case$expected)), + info = case$name + ) + } +}) + test_that("provenance_display uses R display copy", { display <- shared_fixture("provenance")$provenance_display$tags expect_setequal(names(display), names(provenance_display)) diff --git a/tests/shared/provenance.json b/tests/shared/provenance.json index 36631894..09e956c7 100644 --- a/tests/shared/provenance.json +++ b/tests/shared/provenance.json @@ -72,6 +72,53 @@ } ] }, + "collect_appended_tags": { + "description": "Which tags collect_appended_tags(turns, from_index) gathers from one exchange. Each case builds a conversation from `turns`: a turn has a `role` (`user` or `assistant`) and `contents`, where a content item is {\"type\": \"tool_result\", \"tag\": ...} or {\"type\": \"text\", \"text\": ...}. A tool result's `tag` is the commons_tag value the tool set; an absent `tag` key or a null value means the result carries no tag. `skip` counts the turns already present when the exchange began: Python passes it as from_index, R passes skip + 1. `expected` lists the tags collected, in turn order. A tag value outside A, B, and C is deliberately not pinned: Python drops it at collection, R returns it, and the only consumer, derive_provenance_tag, ignores anything but A and B either way.", + "cases": [ + { + "name": "tags are collected across the appended turns", + "turns": [ + {"role": "user", "contents": [{"type": "tool_result", "tag": "A"}]}, + {"role": "user", "contents": [{"type": "tool_result", "tag": "B"}]} + ], + "skip": 0, + "expected": ["A", "B"] + }, + { + "name": "turns already present when the exchange began are ignored", + "turns": [ + {"role": "user", "contents": [{"type": "tool_result", "tag": "B"}]}, + {"role": "user", "contents": [{"type": "tool_result", "tag": "A"}]} + ], + "skip": 1, + "expected": ["A"] + }, + { + "name": "an exchange with no new turns collects nothing", + "turns": [ + {"role": "assistant", "contents": [{"type": "text", "text": "hi"}]} + ], + "skip": 1, + "expected": [] + }, + { + "name": "content without a tag is skipped", + "turns": [ + { + "role": "user", + "contents": [ + {"type": "text", "text": "Revenue was 42."}, + {"type": "tool_result"}, + {"type": "tool_result", "tag": null}, + {"type": "tool_result", "tag": "A"} + ] + } + ], + "skip": 0, + "expected": ["A"] + } + ] + }, "provenance_display": { "description": "Word-for-word copy both UIs render. `icon` is null when the provenance marker has no icon.", "tags": { From 020f8878cf46419d5c06bc05640d8f6360a26508 Mon Sep 17 00:00:00 2001 From: Josh Taillon Date: Mon, 7 Sep 2026 13:54:22 -0600 Subject: [PATCH 3/5] fix(py): list build_citation_corpus in __all__ and document escape_attr Every other public function in _citations.py was exported; the corpus builder was the one omission. escape_attr gained a docstring when it stopped being private. --- pkg-py/src/commons/_citations.py | 1 + pkg-py/src/commons/_provenance.py | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg-py/src/commons/_citations.py b/pkg-py/src/commons/_citations.py index 90fa6ecb..883bd10e 100644 --- a/pkg-py/src/commons/_citations.py +++ b/pkg-py/src/commons/_citations.py @@ -30,6 +30,7 @@ "CitationRequest", "CorpusEntry", "ParsedCitation", + "build_citation_corpus", "citation_aside_html", "citation_reminder_text", "match_citation", diff --git a/pkg-py/src/commons/_provenance.py b/pkg-py/src/commons/_provenance.py index 687f71b0..ffca6a7b 100644 --- a/pkg-py/src/commons/_provenance.py +++ b/pkg-py/src/commons/_provenance.py @@ -121,8 +121,9 @@ def collect_appended_tags(turns: Sequence[Turn], from_index: int) -> list[Tag]: return tags -# Ampersands first, so the entities this generates are not escaped again. def escape_attr(text: str) -> str: + """Escape ``&`` and ``"`` for use inside an HTML attribute value.""" + # Ampersands first, so the entities this generates are not escaped again. return text.replace("&", "&").replace('"', """) From f2d37450348547d8b96aa3d4039c26d5298bd3f1 Mon Sep 17 00:00:00 2001 From: Josh Taillon Date: Mon, 7 Sep 2026 13:54:23 -0600 Subject: [PATCH 4/5] docs: add the new fixtures to the shared-fixture inventory turn-reminders.json had no bullet, and the provenance bullet now names collect_appended_tags alongside the truth table. --- tests/shared/README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/shared/README.md b/tests/shared/README.md index bc15fecb..571c0613 100644 --- a/tests/shared/README.md +++ b/tests/shared/README.md @@ -23,10 +23,11 @@ The Python suite reads this directory directly. The R suite cannot. `testthat` n ## What sort of fixtures are here - **Span names and attributes.** `commons_conversation_turn`, `commons_agent_create`, `commons_data_source_create`, and friends. Also `gen_ai.conversation.id`, `commons.provenance.tag`, and the exact JSON shape of `commons.citation.candidates`. This contract lets the R trajectory reviewer read Python traces. Write it so that it survives the conversation-id ownership moving upstream to shinychat. -- **The provenance and citation behavior.** The `derive_provenance_tag()` truth table, `normalize_citation()` input/output pairs, `match_citation()` verdicts including both guards (10-character minimum, only-the-quote-verifies), `parse_commons_citation()` well-formed and malformed bodies. Also the chunk-invariance cases of the streaming scanner. The scanner is a pure chunks-in/string-out function, so it is ideal fixture material. +- **The provenance and citation behavior.** The `derive_provenance_tag()` truth table, `collect_appended_tags()` collection from an exchange's turns, `normalize_citation()` input/output pairs, `match_citation()` verdicts including both guards (10-character minimum, only-the-quote-verifies), `parse_commons_citation()` well-formed and malformed bodies. Also the chunk-invariance cases of the streaming scanner. The scanner is a pure chunks-in/string-out function, so it is ideal fixture material. - **System prompt rendering.** The template lives in `prompts/`, not here, but what each renderer makes of it does: `prompt-render.json` holds the data a renderer receives and the prompt it must produce. A shared template pins the words, not the rendering. - **Where the citation request lands.** `citation-request.json` pins the reminder to the first tool result of a user turn whose output has to be cited: which value shapes it joins and how, and which turns start a new request. The wording lives in `prompts/citation-request.md`, so the fixture uses a stand-in for it. - **Handles.** `handles.json` pins the ids a conversation hands out for tool results (`r1`, `r2`, ...), that a value which is not a frame gets one too, the note's opening sentence, and the row cap with the sentence that states it. The opening sentence is pinned as a template with the tool name left as a placeholder, since the tool that reaches a handle is `run_r` in R and `run_python` in Python. +- **Turn reminders.** `turn-reminders.json` pins the wording appended to a user's turn: the concise reminder's text and which model ids earn it, and the restored-conversation reminder as a template plus each package's two substitutions (its language and its execution tool), so the sentence is held once rather than copied per language. - **The citation dialect and display copy.** The `` grammar and the `PROVENANCE_DISPLAY` strings, so that both UIs say the same words. - **The definitions interface.** `definition-export/` holds the shared fixtures: 14 data dictionaries in data-dict's YAML format, each declaring table-level `definitions` whose expressions use data-dict's expression language — 3 valid files (42 definitions) and 11 invalid ones — read by both commons implementations. `definitions.json` pins what both packages agree to produce from them, in three sections: - `export_records` — the expected export for each valid definition: its SQL translation, its inferred kind and type, and the columns and definitions it references. Generated from the data-dict binary at the pinned commit by `scripts/generate-definitions-fixture.sh`, which refuses to run against a binary built from anything else. Never hand-edit. From 2f3f622a592aa6618be02d424ab6f83031e1c736 Mon Sep 17 00:00:00 2001 From: Josh Taillon Date: Mon, 7 Sep 2026 22:29:56 -0600 Subject: [PATCH 5/5] test: pin R's unreadable-tag collection and an arguments-spanning quote Review follow-ups on the shared fixtures. citation-corpus.json gains a case quoting across a measure's sources line and arguments block, and the spec-to-measure builders move into tests/_shared.py and helper-shared-fixtures.R so both runners share them instead of keeping an injected-only copy. R gains a per-language test for its side of the collection divergence the fixture leaves unpinned: R returns an unrecognized tag value where Python drops it. The shared-fixture inventory names citation-corpus.json, a Python test stops calling a tag value an outcome, and escape_attr's docstring states its double-quoted-attribute-only constraint. --- pkg-py/src/commons/_provenance.py | 6 +- pkg-py/tests/_shared.py | 79 ++++++++++++++++++- pkg-py/tests/test_citation_corpus.py | 35 +------- pkg-py/tests/test_measures.py | 74 +---------------- pkg-py/tests/test_provenance.py | 2 +- .../fixtures/shared/citation-corpus.json | 34 +++++++- pkg-r/tests/testthat/helper-shared-fixtures.R | 52 ++++++++++++ pkg-r/tests/testthat/test-citations.R | 16 +--- pkg-r/tests/testthat/test-measures.R | 49 ------------ pkg-r/tests/testthat/test-provenance.R | 24 ++++++ tests/shared/README.md | 2 +- tests/shared/citation-corpus.json | 34 +++++++- 12 files changed, 232 insertions(+), 175 deletions(-) diff --git a/pkg-py/src/commons/_provenance.py b/pkg-py/src/commons/_provenance.py index ffca6a7b..af386dbc 100644 --- a/pkg-py/src/commons/_provenance.py +++ b/pkg-py/src/commons/_provenance.py @@ -122,7 +122,11 @@ def collect_appended_tags(turns: Sequence[Turn], from_index: int) -> list[Tag]: def escape_attr(text: str) -> str: - """Escape ``&`` and ``"`` for use inside an HTML attribute value.""" + """Escape ``&`` and ``"`` for a double-quoted HTML attribute value. + + Nothing else is escaped, so the result belongs in a double-quoted + attribute only: never a text node, never a single-quoted attribute. + """ # Ampersands first, so the entities this generates are not escaped again. return text.replace("&", "&").replace('"', """) diff --git a/pkg-py/tests/_shared.py b/pkg-py/tests/_shared.py index 9ee81269..934e86af 100644 --- a/pkg-py/tests/_shared.py +++ b/pkg-py/tests/_shared.py @@ -7,7 +7,11 @@ import json from pathlib import Path -from typing import Any +from typing import Annotated, Any, Literal + +from pydantic import Field + +from commons._measures import Injected, Measure, as_measure, measure SHARED_DIR = Path(__file__).resolve().parents[2] / "tests" / "shared" @@ -21,3 +25,76 @@ def load_shared_fixture(name: str) -> Any: f"installed wheel." ) return json.loads(path.read_text(encoding="utf-8")) + + +_SCALARS: dict[str, Any] = { + "string": str, + "integer": int, + "number": float, + "boolean": bool, +} + + +def _fixture_annotation(spec: dict[str, Any]) -> Any: + kind = spec["type"] + if kind == "enum": + return Literal[tuple(spec["values"])] # type: ignore[misc] + if kind == "array": + return list[_fixture_annotation(spec["items"])] + return _SCALARS[kind] + + +def fixture_measure(spec: dict[str, Any]) -> Measure: + """Build a measure from a fixture spec through the production ``@measure``. + + A real function is generated because ``@measure`` inspects a signature, + not a spec; each described argument keeps its position, required arguments + and injected arguments come first (Python requires that), and defaulted + arguments follow. A case declaring a required argument after an optional + one cannot be built without reordering, which would render a different + argument order than the R runner, so it is rejected rather than built. + """ + arguments = spec.get("arguments") or [] + injected = spec.get("injected") or [] + + optional_seen = False + for argument in arguments: + if argument["required"] and optional_seen: + raise ValueError( + f"Fixture case {spec['name']!r} declares required argument " + f"{argument['name']!r} after an optional one; Python requires " + "defaulted parameters last, so this runner cannot preserve " + "declaration order for that case." + ) + optional_seen = optional_seen or not argument["required"] + + namespace: dict[str, Any] = {} + required_params: list[str] = [] + optional_params: list[str] = [] + + for argument in arguments: + type_name = f"_{argument['name']}_type" + namespace[type_name] = Annotated[ + _fixture_annotation(argument), Field(description=argument["description"]) + ] + if argument["required"]: + required_params.append(f"{argument['name']}: {type_name}") + else: + default_name = f"_{argument['name']}_default" + namespace[default_name] = argument["default"] + optional_params.append(f"{argument['name']}: {type_name} = {default_name}") + + injected_params: list[str] = [] + for injected_name in injected: + type_name = f"_{injected_name}_type" + namespace[type_name] = Injected[Any] + injected_params.append(f"{injected_name}: {type_name}") + + params = ", ".join(required_params + injected_params + optional_params) + exec(f"def {spec['name']}({params}) -> None: ...", namespace) # noqa: S102 + func = namespace[spec["name"]] + + decorated = measure(description=spec["description"], name=spec["name"])(func) + record = as_measure(decorated) + assert record is not None + return record diff --git a/pkg-py/tests/test_citation_corpus.py b/pkg-py/tests/test_citation_corpus.py index aabcd766..59420394 100644 --- a/pkg-py/tests/test_citation_corpus.py +++ b/pkg-py/tests/test_citation_corpus.py @@ -5,7 +5,6 @@ suite runs the same ones. Do not restate a case here; add it to the fixture. """ -import inspect from typing import Any import pandas as pd @@ -15,44 +14,14 @@ from commons._citations import build_citation_corpus, match_citation from commons._data_dictionary import DataDictionary from commons._data_source import DataSource -from commons._measures import Injected, Measure, as_measure, measure -from ._shared import load_shared_fixture +from ._shared import fixture_measure, load_shared_fixture CASES: list[dict[str, Any]] = load_shared_fixture("citation-corpus")[ "build_citation_corpus" ]["cases"] -def _fixture_measure(spec: dict[str, Any]) -> Measure: - """Build a measure from a fixture spec through the production decorator. - - Every argument a case declares is commons-supplied, so the generated - function carries an ``Injected`` annotation per name and no model-visible - arguments at all: what the corpus needs from a measure is the block - ``search_pool`` renders, and measure-schema.json pins the arguments half - of that block already. - """ - - def func(*args: Any, **kwargs: Any) -> None: - return None - - injected: list[str] = spec.get("injected") or [] - func.__name__ = spec["name"] - func.__annotations__ = {name: Injected[Any] for name in injected} - func.__signature__ = inspect.Signature( # type: ignore[attr-defined] - [ - inspect.Parameter( - name, inspect.Parameter.POSITIONAL_OR_KEYWORD, annotation=Injected[Any] - ) - for name in injected - ] - ) - record = as_measure(measure(description=spec["description"])(func)) - assert record is not None - return record - - def _fixture_source(spec: dict[str, Any]) -> DataSource: """A real source over an in-memory DuckDB, carrying the case's dictionary.""" dictionary = ( @@ -71,7 +40,7 @@ def test_the_fixture_is_not_empty() -> None: @pytest.mark.parametrize("case", CASES, ids=lambda case: case["name"]) def test_the_corpus_matches_the_shared_fixture(case: dict[str, Any]) -> None: layer = None if case["docs"] is None else ContextLayer(case["docs"]) - measures = [_fixture_measure(spec) for spec in case["measures"]] + measures = [fixture_measure(spec) for spec in case["measures"]] sources = {spec["name"]: _fixture_source(spec) for spec in case["sources"]} corpus = build_citation_corpus(layer, measures, sources) diff --git a/pkg-py/tests/test_measures.py b/pkg-py/tests/test_measures.py index 9b4e397b..7cf2de00 100644 --- a/pkg-py/tests/test_measures.py +++ b/pkg-py/tests/test_measures.py @@ -31,7 +31,7 @@ semantic_layer, ) -from ._shared import load_shared_fixture +from ._shared import fixture_measure, load_shared_fixture def test_injected_alias_carries_the_marker() -> None: @@ -391,76 +391,6 @@ def test_measure_is_frozen() -> None: "measure_schema_text" ]["cases"] -_SCALARS: dict[str, Any] = { - "string": str, - "integer": int, - "number": float, - "boolean": bool, -} - - -def _fixture_annotation(spec: dict[str, Any]) -> Any: - kind = spec["type"] - if kind == "enum": - return Literal[tuple(spec["values"])] # type: ignore[misc] - if kind == "array": - return list[_fixture_annotation(spec["items"])] - return _SCALARS[kind] - - -def _fixture_measure(spec: dict[str, Any]) -> Measure: - """Build a measure from a fixture spec and decorate it with the production - @measure, so this runner enters through the same front door as - test-measures.R's runner enters measure(). - - A real function is generated because @measure inspects a signature, not a - spec; each described argument keeps its position, required arguments and - injected arguments come first (Python requires that), and defaulted - arguments follow. A case declaring a required argument after an optional - one cannot be built without reordering, which would render a different - argument order than the R runner, so it is rejected rather than built. - """ - optional_seen = False - for argument in spec["arguments"]: - if argument["required"] and optional_seen: - raise ValueError( - f"Fixture case {spec['name']!r} declares required argument " - f"{argument['name']!r} after an optional one; Python requires " - "defaulted parameters last, so this runner cannot preserve " - "declaration order for that case." - ) - optional_seen = optional_seen or not argument["required"] - - namespace: dict[str, Any] = {} - required_params: list[str] = [] - optional_params: list[str] = [] - - for argument in spec["arguments"]: - type_name = f"_{argument['name']}_type" - namespace[type_name] = Annotated[ - _fixture_annotation(argument), Field(description=argument["description"]) - ] - if argument["required"]: - required_params.append(f"{argument['name']}: {type_name}") - else: - default_name = f"_{argument['name']}_default" - namespace[default_name] = argument["default"] - optional_params.append(f"{argument['name']}: {type_name} = {default_name}") - - injected_params: list[str] = [] - for injected_name in spec["injected"]: - type_name = f"_{injected_name}_type" - namespace[type_name] = Injected[Any] - injected_params.append(f"{injected_name}: {type_name}") - - params = ", ".join(required_params + injected_params + optional_params) - exec(f"def {spec['name']}({params}) -> None: ...", namespace) # noqa: S102 - func = namespace[spec["name"]] - - decorated = measure(description=spec["description"], name=spec["name"])(func) - return _as_measure(decorated) - - def test_schema_fixture_is_not_empty() -> None: assert SCHEMA_CASES @@ -468,7 +398,7 @@ def test_schema_fixture_is_not_empty() -> None: @pytest.mark.parametrize("case", SCHEMA_CASES, ids=lambda case: case["name"]) def test_measure_schema_text_matches_the_shared_fixture(case: dict[str, Any]) -> None: rendered = measure_schema_text( - _fixture_measure(case["measure"]), + fixture_measure(case["measure"]), source_names=case["source_names"], heading=case.get("heading"), ) diff --git a/pkg-py/tests/test_provenance.py b/pkg-py/tests/test_provenance.py index 9a83799b..12dd915f 100644 --- a/pkg-py/tests/test_provenance.py +++ b/pkg-py/tests/test_provenance.py @@ -111,7 +111,7 @@ def test_the_shared_fixture_covers_collection_edges() -> None: assert any(case["expected"] == [] for case in COLLECT_CASES) -def test_ignores_a_tag_value_that_is_not_an_outcome() -> None: +def test_ignores_a_tag_value_that_is_not_a_valid_tag() -> None: # Deliberately per-language, so the fixture does not pin it: Python drops # an unreadable tag at collection, so it cannot cost the exchange the # tags that are readable. R returns it; derive_provenance_tag ignores diff --git a/pkg-r/tests/testthat/fixtures/shared/citation-corpus.json b/pkg-r/tests/testthat/fixtures/shared/citation-corpus.json index ddb4f55a..42bfb806 100644 --- a/pkg-r/tests/testthat/fixtures/shared/citation-corpus.json +++ b/pkg-r/tests/testthat/fixtures/shared/citation-corpus.json @@ -1,7 +1,7 @@ { "description": "The trusted text an answer's citations are verified against. The source is tests/shared/citation-corpus.json; the copy under pkg-r/tests/testthat/fixtures/shared/ is generated by scripts/sync-shared.sh. Edit the source and re-run that script.", "build_citation_corpus": { - "description": "Every case builds a corpus from a context layer's documents, a list of measures, and named data sources whose dictionaries describe a table called `sales`. `expected` names every entry the corpus holds, in order, because matching returns the first entry whose text contains the quote: specific sources are added before the general documentation corpus, so prose that is both a dictionary's and a document's keeps the dictionary's label. `matches` quotes text from the inputs and names the entry that quote must resolve to, or null when nothing may match it. A measure contributes the same block search_pool shows the model, so a quote from that block is citable; measure text itself is pinned by measure-schema.json.", + "description": "Every case builds a corpus from a context layer's documents, a list of measures, and named data sources whose dictionaries describe a table called `sales`. `expected` names every entry the corpus holds, in order, because matching returns the first entry whose text contains the quote: specific sources are added before the general documentation corpus, so prose that is both a dictionary's and a document's keeps the dictionary's label. `matches` quotes text from the inputs and names the entry that quote must resolve to, or null when nothing may match it. A measure contributes the same block search_pool shows the model, so a quote from that block is citable; measure text itself is pinned by measure-schema.json, whose spec shape a measure's `arguments` and `injected` fields reuse.", "cases": [ { "name": "each kind of trusted text carries its own label and kind", @@ -148,6 +148,38 @@ "label": "sales_db dictionary" } ] + }, + { + "name": "a quote spans a measure's sources line and arguments block", + "docs": [], + "measures": [ + { + "name": "region_revenue", + "description": "Total revenue for a region.", + "arguments": [ + { + "name": "region", + "type": "string", + "required": true, + "description": "The sales region." + } + ], + "injected": ["sales_db"] + } + ], + "sources": [ + { "name": "sales_db", "dictionary": null }, + { "name": "returns_db", "dictionary": null } + ], + "expected": [ + { "label": "region_revenue definition", "kind": "definition" } + ], + "matches": [ + { + "quote": "Total revenue for a region.\n\nsources: sales_db\narguments:\n - region (string, required) The sales region.", + "label": "region_revenue definition" + } + ] } ] } diff --git a/pkg-r/tests/testthat/helper-shared-fixtures.R b/pkg-r/tests/testthat/helper-shared-fixtures.R index 2ae44517..1e8009df 100644 --- a/pkg-r/tests/testthat/helper-shared-fixtures.R +++ b/pkg-r/tests/testthat/helper-shared-fixtures.R @@ -14,3 +14,55 @@ shared_fixture <- function(name) { } jsonlite::fromJSON(path, simplifyVector = FALSE) } + +# Build the ellmer type a fixture spec describes. Shared by every runner +# whose fixture declares measure arguments (measure-schema.json, +# citation-corpus.json). +fixture_scalar_type <- function(kind, description = "", required = TRUE) { + switch( + kind, + integer = ellmer::type_integer(description, required = required), + number = ellmer::type_number(description, required = required), + boolean = ellmer::type_boolean(description, required = required), + ellmer::type_string(description, required = required) + ) +} + +fixture_type <- function(arg) { + required <- isTRUE(arg$required) + if (identical(arg$type, "enum")) { + return(ellmer::type_enum( + values = unlist(arg$values), + description = arg$description, + required = required + )) + } + if (identical(arg$type, "array")) { + items <- if (identical(arg$items$type, "enum")) { + ellmer::type_enum(values = unlist(arg$items$values)) + } else { + fixture_scalar_type(arg$items$type) + } + return(ellmer::type_array( + items = items, + description = arg$description, + required = required + )) + } + fixture_scalar_type(arg$type, arg$description, required) +} + +# Build a measure from a fixture spec. The injected arguments only have to +# exist as formals; measure() marks them as hidden from the model. +fixture_measure <- function(spec) { + arguments <- list() + for (arg in spec$arguments) { + arguments[[arg$name]] <- fixture_type(arg) + } + formal_names <- c(names(arguments), unlist(spec$injected)) + fn <- as.function(c( + stats::setNames(rep(list(quote(expr = )), length(formal_names)), formal_names), + list(NULL) + )) + measure(spec$name, spec$description, fn, arguments = arguments) +} diff --git a/pkg-r/tests/testthat/test-citations.R b/pkg-r/tests/testthat/test-citations.R index b96e3eef..4290c0f1 100644 --- a/pkg-r/tests/testthat/test-citations.R +++ b/pkg-r/tests/testthat/test-citations.R @@ -75,20 +75,6 @@ test_that("citation asides leave trust copy to their markers", { ) }) -# A measure the fixture describes: its arguments are all commons-supplied, so -# `arguments` stays empty and every formal is an injected source. -fixture_corpus_measure <- function(spec) { - injected <- as.character(unlist(spec$injected) %||% character()) - fn <- if (length(injected) == 0) { - function() NULL - } else { - formals <- rep(list(rlang::missing_arg()), length(injected)) - names(formals) <- injected - rlang::new_function(formals, quote(NULL)) - } - measure(spec$name, spec$description, fn) -} - test_that("build_citation_corpus matches the shared fixture", { cases <- shared_fixture("citation-corpus")$build_citation_corpus$cases # An empty fixture would make the loop below vacuously succeed. @@ -100,7 +86,7 @@ test_that("build_citation_corpus matches the shared fixture", { } else { new_context_layer(as.character(unlist(case$docs))) } - registry <- lapply(case$measures, fixture_corpus_measure) + registry <- lapply(case$measures, fixture_measure) sources <- lapply(case$sources, function(spec) { dictionary <- if (is.null(spec$dictionary)) { NULL diff --git a/pkg-r/tests/testthat/test-measures.R b/pkg-r/tests/testthat/test-measures.R index 2aab56b6..50ea0aff 100644 --- a/pkg-r/tests/testthat/test-measures.R +++ b/pkg-r/tests/testthat/test-measures.R @@ -152,55 +152,6 @@ test_that("search_pool_text reports when nothing matches", { ) }) -fixture_scalar_type <- function(kind, description = "", required = TRUE) { - switch( - kind, - integer = ellmer::type_integer(description, required = required), - number = ellmer::type_number(description, required = required), - boolean = ellmer::type_boolean(description, required = required), - ellmer::type_string(description, required = required) - ) -} - -fixture_type <- function(arg) { - required <- isTRUE(arg$required) - if (identical(arg$type, "enum")) { - return(ellmer::type_enum( - values = unlist(arg$values), - description = arg$description, - required = required - )) - } - if (identical(arg$type, "array")) { - items <- if (identical(arg$items$type, "enum")) { - ellmer::type_enum(values = unlist(arg$items$values)) - } else { - fixture_scalar_type(arg$items$type) - } - return(ellmer::type_array( - items = items, - description = arg$description, - required = required - )) - } - fixture_scalar_type(arg$type, arg$description, required) -} - -# Build a measure from a fixture spec. The injected arguments only have to -# exist as formals; measure() marks them as hidden from the model. -fixture_measure <- function(spec) { - arguments <- list() - for (arg in spec$arguments) { - arguments[[arg$name]] <- fixture_type(arg) - } - formal_names <- c(names(arguments), unlist(spec$injected)) - fn <- as.function(c( - stats::setNames(rep(list(quote(expr = )), length(formal_names)), formal_names), - list(NULL) - )) - measure(spec$name, spec$description, fn, arguments = arguments) -} - test_that("measure_schema_text matches the shared fixture", { cases <- shared_fixture("measure-schema")$measure_schema_text$cases expect_gt(length(cases), 0) diff --git a/pkg-r/tests/testthat/test-provenance.R b/pkg-r/tests/testthat/test-provenance.R index ceee76f3..4e193867 100644 --- a/pkg-r/tests/testthat/test-provenance.R +++ b/pkg-r/tests/testthat/test-provenance.R @@ -58,6 +58,30 @@ test_that("collect_appended_tags matches the shared fixture", { } }) +test_that("collect_appended_tags keeps a tag value that is not a valid tag", { + # Deliberately per-language, so the fixture does not pin it: R returns an + # unreadable tag at collection and Python drops it; derive_provenance_tag + # ignores anything but A and B either way. + turns <- list( + ellmer::UserTurn( + contents = list( + ellmer::ContentToolResult( + value = "1", + request = NULL, + extra = list(commons_tag = "Z") + ), + ellmer::ContentToolResult( + value = "2", + request = NULL, + extra = list(commons_tag = "B") + ) + ) + ) + ) + + expect_identical(collect_appended_tags(turns, from_index = 1L), c("Z", "B")) +}) + test_that("provenance_display uses R display copy", { display <- shared_fixture("provenance")$provenance_display$tags expect_setequal(names(display), names(provenance_display)) diff --git a/tests/shared/README.md b/tests/shared/README.md index 571c0613..77bdb746 100644 --- a/tests/shared/README.md +++ b/tests/shared/README.md @@ -23,7 +23,7 @@ The Python suite reads this directory directly. The R suite cannot. `testthat` n ## What sort of fixtures are here - **Span names and attributes.** `commons_conversation_turn`, `commons_agent_create`, `commons_data_source_create`, and friends. Also `gen_ai.conversation.id`, `commons.provenance.tag`, and the exact JSON shape of `commons.citation.candidates`. This contract lets the R trajectory reviewer read Python traces. Write it so that it survives the conversation-id ownership moving upstream to shinychat. -- **The provenance and citation behavior.** The `derive_provenance_tag()` truth table, `collect_appended_tags()` collection from an exchange's turns, `normalize_citation()` input/output pairs, `match_citation()` verdicts including both guards (10-character minimum, only-the-quote-verifies), `parse_commons_citation()` well-formed and malformed bodies. Also the chunk-invariance cases of the streaming scanner. The scanner is a pure chunks-in/string-out function, so it is ideal fixture material. +- **The provenance and citation behavior.** The `derive_provenance_tag()` truth table, `collect_appended_tags()` collection from an exchange's turns, the `build_citation_corpus()` corpus — which text is citable, under which label, and in which order, since matching returns the first entry holding the quote — `normalize_citation()` input/output pairs, `match_citation()` verdicts including both guards (10-character minimum, only-the-quote-verifies), `parse_commons_citation()` well-formed and malformed bodies. Also the chunk-invariance cases of the streaming scanner. The scanner is a pure chunks-in/string-out function, so it is ideal fixture material. - **System prompt rendering.** The template lives in `prompts/`, not here, but what each renderer makes of it does: `prompt-render.json` holds the data a renderer receives and the prompt it must produce. A shared template pins the words, not the rendering. - **Where the citation request lands.** `citation-request.json` pins the reminder to the first tool result of a user turn whose output has to be cited: which value shapes it joins and how, and which turns start a new request. The wording lives in `prompts/citation-request.md`, so the fixture uses a stand-in for it. - **Handles.** `handles.json` pins the ids a conversation hands out for tool results (`r1`, `r2`, ...), that a value which is not a frame gets one too, the note's opening sentence, and the row cap with the sentence that states it. The opening sentence is pinned as a template with the tool name left as a placeholder, since the tool that reaches a handle is `run_r` in R and `run_python` in Python. diff --git a/tests/shared/citation-corpus.json b/tests/shared/citation-corpus.json index ddb4f55a..42bfb806 100644 --- a/tests/shared/citation-corpus.json +++ b/tests/shared/citation-corpus.json @@ -1,7 +1,7 @@ { "description": "The trusted text an answer's citations are verified against. The source is tests/shared/citation-corpus.json; the copy under pkg-r/tests/testthat/fixtures/shared/ is generated by scripts/sync-shared.sh. Edit the source and re-run that script.", "build_citation_corpus": { - "description": "Every case builds a corpus from a context layer's documents, a list of measures, and named data sources whose dictionaries describe a table called `sales`. `expected` names every entry the corpus holds, in order, because matching returns the first entry whose text contains the quote: specific sources are added before the general documentation corpus, so prose that is both a dictionary's and a document's keeps the dictionary's label. `matches` quotes text from the inputs and names the entry that quote must resolve to, or null when nothing may match it. A measure contributes the same block search_pool shows the model, so a quote from that block is citable; measure text itself is pinned by measure-schema.json.", + "description": "Every case builds a corpus from a context layer's documents, a list of measures, and named data sources whose dictionaries describe a table called `sales`. `expected` names every entry the corpus holds, in order, because matching returns the first entry whose text contains the quote: specific sources are added before the general documentation corpus, so prose that is both a dictionary's and a document's keeps the dictionary's label. `matches` quotes text from the inputs and names the entry that quote must resolve to, or null when nothing may match it. A measure contributes the same block search_pool shows the model, so a quote from that block is citable; measure text itself is pinned by measure-schema.json, whose spec shape a measure's `arguments` and `injected` fields reuse.", "cases": [ { "name": "each kind of trusted text carries its own label and kind", @@ -148,6 +148,38 @@ "label": "sales_db dictionary" } ] + }, + { + "name": "a quote spans a measure's sources line and arguments block", + "docs": [], + "measures": [ + { + "name": "region_revenue", + "description": "Total revenue for a region.", + "arguments": [ + { + "name": "region", + "type": "string", + "required": true, + "description": "The sales region." + } + ], + "injected": ["sales_db"] + } + ], + "sources": [ + { "name": "sales_db", "dictionary": null }, + { "name": "returns_db", "dictionary": null } + ], + "expected": [ + { "label": "region_revenue definition", "kind": "definition" } + ], + "matches": [ + { + "quote": "Total revenue for a region.\n\nsources: sales_db\narguments:\n - region (string, required) The sales region.", + "label": "region_revenue definition" + } + ] } ] }