You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
.dev/tools/check-cookbook-snippets.py puts one shared preamble in front of every # cookbook:partial snippet. The preamble imports every alias any recipe uses, so a snippet that uses a name its own recipe never imports still passes, and fails with a NameError when a reader runs the recipe. #74 hit this twice (dfc in query.md, QueryParams / PV / bc in ingestion.md).
A third checker pass (pure ast, no new dependency). For each recipe file, every name a partial snippet uses that the preamble imports must be bound somewhere in that file's own checked blocks (imports, assignments, defs, …). The import list is parsed from the preamble itself, so the preamble's fixtures (client, params, begin/end, t0/t1, carried-forward ids, acquire()) are out of scope automatically. One error per name per file, at its first use.
Unskip the six "Imports used by the examples" blocks.Triage finding not in the draft: all six are # cookbook:skip, so the imports block itself is never checked. A misspelled import there (PvQuerry as PV) passes today. With the skip removed, mypy catches it and the current cookbook still passes. The unskipped blocks also supply the bindings for (1), with no new directive.
conventions.md and connecting.md get imports. The draft's open question resolves to "real hits": both uses are in checked code blocks. conventions.md gets its own imports block (Q, SavePvMetadataRequestParams, to_timestamp), and connecting.md's "Sub-clients can be None" block gets an inline import (QueryParams, PV). No opt-out directive.
Self-test alongside the existing canary: a synthetic recipe using dfc without importing it must fail, and the same recipe with the import must pass.
A prototype of (1) reports exactly the two #74 misses against the pre-fix files, and nothing in today's six recipes beyond the conventions.md / connecting.md hits.
Changed from the draft
ruff --select F821 is not used. mypy already reports any name the preamble does not define, so the only gap is names the preamble does define. F821 would double-report those and need a fixture stub header.
The draft's prose hits need no handling, because the checker only sees fenced ```python blocks.
Out of scope
Values carried between snippets (dataset_id, provider_id, …). Those still need the recipe run as one script, with only its own imports.
Original AI-drafted description
Problem
.dev/tools/check-cookbook-snippets.py puts one shared preamble in front of every # cookbook:partial block. The preamble imports every alias any recipe uses (qc, ssc, dfb, dfc, bc, PV, CFG, DS, AQ, …) and defines client, params, begin/end, t0/t1, and so on. So a snippet that uses a name its own recipe never imports still passes. Run on its own, the same recipe fails with a NameError.
ingestion.md's imports block lacked QueryParams, PV, and bc. Running the recipe as a script caught it.
query.md used dfc and never imported it. This one survived the continuous-script run, because that run appended query.md after ingestion.md, which does import dfc. Copilot's review caught it. Fixed in eedae6a.
The cookbook promises that each recipe's "Imports used by the examples" block covers its examples, and nothing currently checks that.
Proposal
Add a per-recipe import check to the checker, alongside the existing syntax and type checks:
For each recipe, collect the names its own code blocks bind. That means the imports block plus any inline import / from … import, assignments, and defs.
Flag any module alias or imported name a block uses that the recipe never binds. The full list comes from the preamble's own import statements, so it stays in sync with the preamble automatically.
Leave the preamble's fixtures out of this check (client, params, begin/end, t0/t1, the carried-forward ids, acquire()). Those stand in for values a recipe sets up in earlier prose or snippets, which is the reason the preamble exists.
One way to do this is to concatenate a recipe's blocks into one file, with only its own imports, and run ruff --select F821 (undefined name) over it. Fixture names would come from a stub header. Done by hand for the #74 fix, this found 4 undefined names before the fix and 0 after. A pure-ast walk would also work and adds no dependency.
Include a self-test case, matching the existing self-test style: a known-bad recipe that uses an alias it never imports must fail. That way, if the rule stops matching, the checker fails loudly instead of quietly passing everything.
Notes
A one-off scan of today's recipes found no other real misses. The other hits were aliases mentioned in prose (Q.attributes(...) in a callout, dfb.serialized_column() in a bullet), so the check should look only at code blocks.
connecting.md and conventions.md use PV / CFG / Q without importing them. Check whether those uses are in code blocks, and if so whether they are meant to rely on another recipe's imports. If they are, the new rule needs an explicit, documented opt-out rather than a silent exemption.
Summary
.dev/tools/check-cookbook-snippets.pyputs one shared preamble in front of every# cookbook:partialsnippet. The preamble imports every alias any recipe uses, so a snippet that uses a name its own recipe never imports still passes, and fails with aNameErrorwhen a reader runs the recipe. #74 hit this twice (dfcinquery.md,QueryParams/PV/bciningestion.md).Plan:
plan/tickets/75/plan.md(lands with the implementation, in one PR).Scope (after triage, 2026-10-02)
ast, no new dependency). For each recipe file, every name a partial snippet uses that the preamble imports must be bound somewhere in that file's own checked blocks (imports, assignments,defs, …). The import list is parsed from the preamble itself, so the preamble's fixtures (client,params,begin/end,t0/t1, carried-forward ids,acquire()) are out of scope automatically. One error per name per file, at its first use.# cookbook:skip, so the imports block itself is never checked. A misspelled import there (PvQuerry as PV) passes today. With the skip removed, mypy catches it and the current cookbook still passes. The unskipped blocks also supply the bindings for (1), with no new directive.conventions.mdandconnecting.mdget imports. The draft's open question resolves to "real hits": both uses are in checked code blocks.conventions.mdgets its own imports block (Q,SavePvMetadataRequestParams,to_timestamp), andconnecting.md's "Sub-clients can be None" block gets an inline import (QueryParams,PV). No opt-out directive.dfcwithout importing it must fail, and the same recipe with the import must pass.doc/cookbook/README.md"Verifying the examples", and one sentence inCLAUDE.md. NoNEXT.mdentry, since this is tooling only (same call as Release notes checker: add link rules and fix the main-pinned link in rel-1.16.0 (data-platform#98) #70).A prototype of (1) reports exactly the two #74 misses against the pre-fix files, and nothing in today's six recipes beyond the
conventions.md/connecting.mdhits.Changed from the draft
ruff --select F821is not used. mypy already reports any name the preamble does not define, so the only gap is names the preamble does define. F821 would double-report those and need a fixture stub header.```pythonblocks.Out of scope
dataset_id,provider_id, …). Those still need the recipe run as one script, with only its own imports.Original AI-drafted description
Problem
.dev/tools/check-cookbook-snippets.pyputs one shared preamble in front of every# cookbook:partialblock. The preamble imports every alias any recipe uses (qc,ssc,dfb,dfc,bc,PV,CFG,DS,AQ, …) and definesclient,params,begin/end,t0/t1, and so on. So a snippet that uses a name its own recipe never imports still passes. Run on its own, the same recipe fails with aNameError.This has now been hit twice in one PR (#74):
ingestion.md's imports block lackedQueryParams,PV, andbc. Running the recipe as a script caught it.query.mduseddfcand never imported it. This one survived the continuous-script run, because that run appendedquery.mdafteringestion.md, which does importdfc. Copilot's review caught it. Fixed in eedae6a.The cookbook promises that each recipe's "Imports used by the examples" block covers its examples, and nothing currently checks that.
Proposal
Add a per-recipe import check to the checker, alongside the existing syntax and type checks:
import/from … import, assignments, anddefs.client,params,begin/end,t0/t1, the carried-forward ids,acquire()). Those stand in for values a recipe sets up in earlier prose or snippets, which is the reason the preamble exists.One way to do this is to concatenate a recipe's blocks into one file, with only its own imports, and run
ruff --select F821(undefined name) over it. Fixture names would come from a stub header. Done by hand for the #74 fix, this found 4 undefined names before the fix and 0 after. A pure-astwalk would also work and adds no dependency.Include a self-test case, matching the existing self-test style: a known-bad recipe that uses an alias it never imports must fail. That way, if the rule stops matching, the checker fails loudly instead of quietly passing everything.
Notes
Q.attributes(...)in a callout,dfb.serialized_column()in a bullet), so the check should look only at code blocks.connecting.mdandconventions.mdusePV/CFG/Qwithout importing them. Check whether those uses are in code blocks, and if so whether they are meant to rely on another recipe's imports. If they are, the new rule needs an explicit, documented opt-out rather than a silent exemption.