Skip to content

Pin the measure schema block as a shared fixture, and fix the R array bug it exposes - #248

Draft
jat255 wants to merge 5 commits into
jat255/wwmt-measure-basicsfrom
jat255/wwmt-schema-fixture
Draft

Pin the measure schema block as a shared fixture, and fix the R array bug it exposes#248
jat255 wants to merge 5 commits into
jat255/wwmt-measure-basicsfrom
jat255/wwmt-schema-fixture

Conversation

@jat255

@jat255 jat255 commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Second of four stacked PRs building the Python semantic layer (M3). Based on #247.

search_pool shows the model one block per matching measure. That text is part of what the agent reads, so both packages must emit the same bytes. This adds measure_schema_text() and pins the format in tests/shared/measure-schema.json, with a runner in each suite.

Building the fixture exposed a live bug in the R package, which this PR also fixes. See the R section below for the detail. It is the only reason this PR touches pkg-r/.

Two Python defects surfaced the same way. An explicit heading="" was treated as absent where R passes it through, and that case is now in the fixture. And an argument typed with a bare enum.Enum, or any nullable argument, rendered as string with its vocabulary hidden from the model, because pydantic emits $ref and anyOf shapes the renderer did not handle.

The enum cases have Python-only tests rather than fixture cases. R has no counterpart to either: ellmer::type_enum() carries its values inline, and required = FALSE marks a type optional rather than producing a nullable union. There is no cross-language contract to pin.

Known gap, left deliberately: a multi-type union such as Region | OtherEnum | None still renders as string. No measure in the package constructs one, and a code comment records it.

R changes

arg_schema_line() renders one argument line of a measure's schema block. Its array branch called type_values() on the array's item type. type_values() reads the S7 values property, which ellmer::TypeEnum has and ellmer::TypeBasic does not. An array of a basic type therefore did not render poorly, it errored with Can't find property <ellmer::TypeBasic>@values. A new array_items_label() helper names a basic item type and still lists an enum item's vocabulary. The diff is five lines plus the helper.

Behaviour delta. Before, a measure with a string[] argument raised when its schema was rendered. Now it renders array of {string}. Enum arrays are unchanged byte for byte, because the helper reproduces the previous expression for that branch. Nothing else about the line moves.

Blast radius. arg_schema_line() has one caller, measure_schema_text(). That has one caller in the package, search_pool_text(), which backs the search_pool tool. So the only reachable effect is that a measure with a non-enum array argument can now appear in search_pool output instead of raising. Reaching it needs a @param with a string[]-style type code, which read_measures() parses into type_array(items = type_string()).

Why it is safe. The enum branch is textually the same expression as before, so no existing snapshot can move. The full package suite passes at 6649 tests with 0 failures. The new fixture runner in test-measures.R builds a measure from each shared case and compares the rendered text against it. It failed on exactly the string-array case before the fix and passes after it, which is the evidence that the fix targets the reported shape and nothing else.

What deserves scrutiny. Whether type_kind() is the right label for a basic item type. It returns the same words the scalar branch prints, so array of {string} reads consistently with (string, required), but a reviewer may prefer different wording. Worth a look because the fixture now pins it for both languages.

No hand-written R tests were deleted. pkg-r/tests/testthat/fixtures/shared/measure-schema.json is generated by scripts/sync-shared-fixtures.sh and needs no review.

An empty-string heading was falling back to the measure name, unlike
the R renderer which passes it through. Found by roborev review of
7b16ade.
Covers the fix in 9b5e8ca; both suites already pass it.
measure_schema_text() only recognized pydantic's inline enum key, which
Literal produces; a bare enum.Enum schemas as a $ref into $defs and
fell back to "string", hiding its vocabulary from the model. Resolve
the $ref before inspecting the property or array items.

Also narrows as_measure()'s Measure | None through a single test
helper instead of nine unchecked accesses, so pyrefly check src tests
actually runs clean once the dead ignore-file exclusion is bypassed.
…etail

pydantic wraps every nullable field or array item in an anyOf union
with a {"type": "null"} branch. _argument_detail and _items_label
only looked at the outer node, so a nullable bare enum, nullable
Literal, or nullable enum array all fell through to plain "string",
hiding their vocabulary from the model.

Unwrap anyOf before resolving $ref, in a single _resolve_node the two
callers share, so the same reduction handles nullable and $ref'd
shapes together instead of accreting one branch per shape found.
@jat255 jat255 changed the title jat255/wwmt schema fixture Pin the measure schema block as a shared fixture, and fix the R array bug it exposes Sep 2, 2026
@jat255 jat255 added the py Affects the Python implementation label Sep 2, 2026
@jat255
jat255 force-pushed the jat255/wwmt-schema-fixture branch from 6923b6a to 1676857 Compare September 2, 2026 03:14
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown

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

Deployed from commit 6923b6a.

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown

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

Deployed from commit 6923b6a.

@jat255
jat255 force-pushed the jat255/wwmt-schema-fixture branch from 1676857 to 6923b6a Compare September 2, 2026 04:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

py Affects the Python implementation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant