Python definitions registry: token expansion, the prompt index, and grain - #261
Merged
jat255 merged 4 commits intoSep 4, 2026
Conversation
jat255
force-pushed
the
jat255/m2-definitions-registry
branch
from
September 3, 2026 18:16
ea16360 to
8b3ea63
Compare
jat255
force-pushed
the
jat255/m2-definitions-registry
branch
from
September 3, 2026 19:30
8b3ea63 to
106bfdc
Compare
jat255
changed the base branch from
main
to
jat255/r-260-definition-token-scope-and-gist
September 3, 2026 19:31
|
Preview deployed to Connect ( Deployed from commit 501a729. |
|
Preview deployed to Connect ( Deployed from commit 501a729. |
The registry surface commons owns whatever data-dict eventually provides:
{{name}} and {{table::name}} expansion, the kind index for the prompt,
grain metadata for call_metrics' mixed-grain guard, and the check that a
definition's table is one the source exposes.
Everything consumes ExportRecord and nothing reaches into an expression
parser or a typed IR. That rule is what makes replacing this with a shared
data-dict interface a deletion rather than a rewrite, and it is why the
registry can be built and tested before the compiler exists.
The dictionary's first-touch entry and its retrieval chunks now render a
table's definitions, which the reader left as a marked seam. Both show
compiled SQL rather than the authored expression, since the expression is
in data-dict's language and the model writes SQL.
Tests run the registry against tests/shared/definitions.json as well as
against hand-built records: every definition data-dict produced expands to
its own compiled SQL, and each kind is one the index groups. This is the
first Python code to consume that contract.
…e gist Four defects, each reproduced first. Token resolution ran against SQL that earlier expansions had already rewritten, so a definition whose compiled SQL named another table brought that table into scope for a later bare token. Every token now resolves against the query as written, and substitution happens afterwards. Retrieval chunks carried only the kind and type, so a retrieved definition named a token without saying what it expands to. They carry the same gist as the first-touch entry now, which is compiled SQL and notes, never the authored expression. An absent type reached the prompt as the word "None". data-dict omits the type when no single one is inferred, so it is left out rather than printed. pkg-r loses the entire gist in this case, filed as #259. The index cap counted line lengths but not the newlines joining them, so the result could exceed the cap. It measures the joined text.
The fixture landed in #265 with only the R suite executing it, and tests/shared/README.md asks for both runners to land with a fixture. The registry exists here, so Python runs the same 13 cases from the same file: which queries expand and to what, the gist, and the index under a cap. Replaces the integrity-only test that stood in for this. Refusals assert the reason the fixture names, through a map to this package's message wording, which is what the R runner does with its own. Every case passes without touching the registry. Checked by perturbing one expansion, one gist and one overflow flag in the fixture and confirming the three matching cases fail.
Self-review found the gist collapsing every whitespace run while the shared contract keeps authored whitespace; the fixture now pins the rule with a double-space case. Also from that pass: - pin the qualified-token refusals (table absent, unknown name) in tests/shared/definition-rendering.json, synced copy included - restore the fixture-integrity guards dropped with test_definition_rendering_fixture.py: both sides of the index cap and a typeless gist case - cover the qualified and legacy-dotted refusal paths and multi-source build_registry/for_source - build_registry: tolerate tables = None instead of crashing past the getattr default - correct stale docstrings: the compiler is not present yet, and the registry now runs the definitions.json records
jat255
force-pushed
the
jat255/m2-definitions-registry
branch
from
September 4, 2026 03:44
1840df1 to
501a729
Compare
jat255
added a commit
that referenced
this pull request
Sep 4, 2026
…ll three as a shared fixture (#265) * fix(r): resolve definition tokens against the query as written Three defects in pkg-r/R/definitions.R, each found while building the Python registry and each pinned here as a shared fixture rather than as an R-only test. expand_definitions() resolved each token against SQL that earlier expansions had already rewritten, so a definition whose compiled SQL named another table brought that table into scope for the tokens that followed. That defeats the check keeping a bare token bound to its own table. Every token now resolves against the query as written, and substitution happens afterwards. Closes #260. definition_index_text() capped on cumsum(nchar(index)) but joined the kept lines with newlines, so the rendered index could exceed cap_chars by one character per line beyond the first, and definitions_overflow() measured the same way and could report no overflow. Both measure the joined text. definition_gist() built its head with sprintf("(%s, %s)", kind, type), which returns character(0) for a definition data-dict inferred no type for, so c() dropped the element and the gist rendered without its "(kind)" prefix. The type is omitted and the kind kept. Note this is narrower than #259 claims: the description and the compiled SQL did survive, so the model was not left with a bare token. Closes #259. tests/shared/definition-rendering.json pins all three as behaviour both packages must agree on: which queries expand and to what, the gist text, and the index under a cap. Refusals pin the fact of refusal and a reason slug rather than the message, which is idiomatic to each language. The two index cases sit on the exact boundary, so they fail against the old cumsum rule rather than passing by accident. The R suite runs the cases; the Python suite checks the fixture's integrity until #261 lands its registry, at which point it runs them too. All 13 cases already pass against that registry unchanged. * refactor(r): make definition_gist() type check NULL-only Both call sites pass dictionary lists, where an absent type is NULL and length(def$type) == 0 already catches it. The is.na() arm was only reachable from registry rows, which never reach definition_gist(), so drop it rather than answer the same question two ways. * Simplify comments to be more concise and reference issue numbers * Simplify comment in test helper definitions
|
Cleaned up 3 preview bundle(s) on https://dogfood.team.pct.posit.it: 367500, 367722, 367741 |
|
Cleaned up 3 preview bundle(s) on https://connect.staging.pct.posit.it: 2521, 2546, 2547 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fifth PR of M2. Adds the data-dict definitions registry, which commons owns until data-dict provides one. Closes the registry issue plus the two items that moved to it from the dictionary reader.
What it does
{{name}}and{{table::name}}tokens inrun_sqlqueries.call_metrics' mixed-grain guard needs.Everything consumes
ExportRecord; nothing reaches into an expression parser or a typed IR. When a shared data-dict interface arrives, swapping it in is a deletion rather than a rewrite. That is why the registry exists, and why it is tested before the compiler is.First Python code to assert the shared contract
Tests run the registry over
tests/shared/definitions.jsonas well as hand-built records. They check that every definition data-dict produced expands to its own compiled SQL, that every kind is one the index groups, and that the grain flags reach the records. #255 landed that fixture with only R asserting it; this is the Python half.The R bugs this found, and where they went
Building this on the Python side turned up three defects in
pkg-r/R/definitions.R: token scope widening through already-expanded SQL, an index cap that excluded the joining newlines, and a typeless definition losing its(kind)prefix. They are fixed in #265, which this PR now stacks on. That is also wheretests/shared/definition-rendering.jsonlives.This PR adds the Python runner for that fixture. Both suites now run the same 13 cases, which is what
tests/shared/README.mdasks for. Every case passed against this registry before the runner was written, so nothing here changed to satisfy it.Verification
339 tests, ruff and pyrefly clean. The shared runner was checked by perturbing one expansion, one gist and one overflow flag in the fixture and confirming the three matching cases fail.