Manifest-driven validate(): the schema block is the spec, shared by the builders and a PR check - #126
Conversation
…he builders and a PR check (#119) builders/_validate.py enforces a manifest's `schema` block on a raw-shaped frame -- columns and `pattern` runs (#120), dtype families (#122), exact `known_nulls` and the `nulls:` placement rule (#121), row_count_floor, date_range -- and MEASURES the overlap window against the previous vintage (no series or period may disappear, no populated cell may go empty; revisions are returned for the builder to bound, never asserted equal). Two callers use it unchanged: - builders/business_cycle.py and builders/business_cycle_fred.py call validate(frame.reset_index(), manifest, previous) and keep only what a schema cannot say: the year grid, the economy set, value bands, recency, each FRED series' first observation, USREC 0/1, and the per-series revision bound. The FRED builder's MONTHLY_FROM / LAST_OBS / KNOWN_HOLES constants are gone -- the manifest's nulls block is the single source. _template.py shows the two-layer shape. - scripts/validate_datasets.py, run by .github/workflows/validate-datasets.yml on every PR: a conformance pass over all 44 manifests (canonical dtypes, name-xor-pattern with one capture group, exact known_nulls, a nulls block on every dynamic snapshot, known_nulls_total only inside header-less sheet reads) and the byte-level validate() over every CSV. Non-CSV formats are conformance-only and the log says so. Two contract fixes the validator surfaced, both in manifests: - us_business_cycle_monthly.csv.yml: `recent: 1` and no exact counts. The live frame on 2026-09-07 ran to 2026-08 with UMCSENT, CPILFESL and INDPRO unpublished for that month (they release mid-month); the old hand-written check asserted zero trailing nulls and would have failed the first monthly refresh on this lag. Every count in that file moves with the newest month, so placement is the contract. - countries.csv.yml: `Country code: 1` -- Namibia's ISO alpha-2 code is the string "NA", which a default read_csv parses as missing. Declared with the reason; a lecture that needs it must pass keep_default_na=False. Verified: 44/44 manifests pass under pandas 2.3.3 (CI) and 3.0.5 (the lectures' pin); both dynamic builders dry-run green against the live sources and reproduce the committed vintages (0 cells revised); 16 mutations of the committed frames are caught, and the four that are not are the builder layer's (grid, first observation) or permitted by design (a lagging newest month, a backfilled hole). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
The builders/business_cycle.py refactor drops a non-null assertion for the Country label column, which is not otherwise enforced by the shared validator for dynamic snapshots and can allow malformed refreshes to pass validation.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR makes the manifest schema block an executable validation contract by introducing a shared validator (builders/_validate.py) and a CI workflow that runs it on every PR, then refactors the dynamic-snapshot builders to delegate schema-derived checks to that shared implementation.
Changes:
- Added
builders/_validate.py(manifest-drivenread_raw()+validate()), and a new PR check (scripts/validate_datasets.py+.github/workflows/validate-datasets.yml) that runs conformance for all manifests and byte-level validation for CSVs. - Refactored dynamic builders (
builders/business_cycle.py,builders/business_cycle_fred.py) and the builder template to use the shared schema validator and keep only dataset-specific checks locally. - Updated manifests/docs to reflect the executable-schema contract and refine null placement rules for dynamic snapshots.
File summaries
| File | Description |
|---|---|
scripts/validate_datasets.py |
New CLI used by CI to enforce manifest conformance and CSV byte-level schema validation. |
PLAN.md |
Marks PR validation as landed and documents the new validator + workflow behavior. |
manifest-schema.yml |
Updates documentation to reflect that the schema block is now validated. |
lectures/us_business_cycle_monthly.csv.yml |
Adjusts null contract to placement-based (including recent: 1) to handle publication lag. |
lectures/countries.csv.yml |
Declares a known_nulls entry for Namibia’s NA being parsed as missing by default read_csv. |
builders/README.md |
Documents shared validator usage across builders and CI. |
builders/business_cycle.py |
Switches schema/invariant checks to the shared validator and keeps builder-specific validation. |
builders/business_cycle_fred.py |
Switches schema/invariant checks to the shared validator and keeps builder-specific validation. |
builders/_validate.py |
New shared, manifest-driven validator implementing schema rules and overlap-window measurement. |
builders/_template.py |
Updates the template to call the shared validator as the base validation layer. |
AGENTS.md |
Documents the shared validate stage and its role in builders and PR validation. |
.github/workflows/validate-datasets.yml |
New CI job running the schema validator on PRs and pushes to main. |
.github/workflows/consumed-file-check.yml |
Clarifies relationship to the new schema-validation workflow. |
Review details
- Files reviewed: 13/13 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…clared nulls The placement rule governs a snapshot's series; economy, Country and DATE are labels, and the old business_cycle.py assertion on Country was dropped in the refactor. Now enforced in the shared validator for every dynamic file rather than restored per builder. Three new mutations caught. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The manifest's
schemablock becomes the spec a validator executes, as #119 asked: one function,builders/_validate.py, used unchanged by the two dynamic builders and by a new PR check. The four decisions #124 wrote into the manifests (#120pattern, #121 nulls, #122 dtypes, #113 naming) are what make it executable; nothing here changes a data file.What lands
builders/_validate.pyvalidate(frame, manifest, previous): walkscolumnsin order (apatternclaims a contiguous run, at least one, exhaustive, capture group →date_range); compares dtypes by family (str/string/objectone family,datetime64any unit, a nullable bool read as object matchesbool, an all-null column matches anything); exactknown_nulls, with an undeclared column in a static file meaning zero; thenulls:placement rule (along,leading,recent,ended,inner);row_count_floor;date_range.start(and.endfor non-dynamic files). Withprevious, it measures the overlap window — no series or period may disappear, no populated cell may go empty — and returns revisions for the builder to bound.read_raw()reads a CSV as written:delimiterhonoured,parse_datesderived fromdatetime64declarations, no index column.scripts/validate_datasets.py+.github/workflows/validate-datasets.ymlknown_nulls, anulls:block on every dynamic snapshot,known_nulls_totalonly insideheader: nullsheet entries) and the byte-levelvalidate()over every CSV. Non-CSV formats get conformance only and the log says so. Runs on pandas 2.3.3, one major behind the lectures, as the standing check that family comparison keeps both green.business_cycle.py,business_cycle_fred.py,_template.pyframe.reset_index()and keep only what a schema cannot say: the year grid, the economy set, value bands, recency, each FRED series' first observation, USREC 0/1, the per-series revision bound. The FRED builder'sMONTHLY_FROM/LAST_OBS/KNOWN_HOLESconstants are removed; the manifest'snulls:block is now the single source.manifest-schema.ymlheader says the schema block is validated;AGENTS.mdBuilders andbuilders/README.mddescribe the two-layer shape;consumed-file-check.yml's header names its sibling instead of promising to be subsumed.Two contract fixes the validator found
The FRED refresh would have failed its first monthly run. Dry-running
business_cycle_fred.pyagainst live FRED on 2026-09-07 returned a frame to 2026-08 with only UNRATE and USREC populated for that month; UMCSENT, CPILFESL and INDPRO release around the 15th. The hand-written check asserted zero trailing nulls for every live series, so the September canary would have gone red on a publication lag, not a data problem.us_business_cycle_monthly.csv.ymlnow saysrecent: 1and carries no exact counts, since every count in that file moves with the newest month; placement is the contract, which is what #121 decided.Namibia.
countries.csvhas one null the manifest did not declare: the ISO alpha-2 codeNA, which a defaultread_csvparses as missing. The schema describes a default read (#122), so it is declared asCountry code: 1with the reason, and a note that a lecture wanting Namibia's code must passkeep_default_na=False.Verification
scripts/validate_datasets.py: 44 of 44 pass under pandas 2.3.3 and under 3.0.5.--out-dir, reproduce the committed vintages (0 cells revised in every overlap window), and the FRED summary feedssnapshots.py pr-bodyunchanged.FIRST_OBS), both confirmed caught there, or permitted by design (a lagging newest month; a backfilled hole).check_consumed_files.py: 44 manifests, 47 files hashed, 0 errors.CATALOG.mdregenerated with no diff.Not in this PR
Reading xlsx/xls/dta/npy/json ranges the way each lecture does, so those 13 manifests are conformance-only for now. Making
validate-datasetsa required status check is a branch-protection setting. The mutation script could become a committed test once the repo has a test runner.Closes #119. With it, every child of #14 is closed.
🤖 Generated with Claude Code