Retire 8 stale annotations and make the manifest-or-annotation rule enforceable - #90
Merged
Conversation
…nforceable audit_annotations.yml's header says a migrated dataset does not get a `datasets:` entry — the manifest is its source of truth. Eight datasets violated it: assignat.xlsx, caron.npy, chapter_3.xlsx, dette.xlsx, fig_3.xlsx, longprices.xls, mpd2020.xlsx and nom_balances.npy. Each annotation was diffed against its manifest before deletion. All eight are strictly subsumed: the annotations carry a one-line description and a `provenance` word, while the manifests carry a title, a full description, class, builder_status and a source note. The two hand-prepared .npy files say "no construction record" in prose; their manifests say the same thing machine-readably as `class: constructed` + `builder_status: unrecovered`. The two hand-transcribed spreadsheets say "hand-transcribed by authors"; their manifests quote the consuming lecture saying so. One entry disagreed with its manifest rather than merely duplicating it: mpd2020.xlsx was annotated `provenance: verbatim`, while its manifest records `class: constructed` with `integrity.upstream.status: diverged` and a `local-edit` delta — three header labels on the `Regional data` sheet were renamed locally, and the lecture reads that sheet with header=(0,1,2), so the edits are load-bearing. The annotation was the stale half. That is exactly the rot the rule exists to prevent, and it is why deleting these rather than leaving them inert is worth doing. The rule is now enforced. `dual_recorded` is a fifth key in the audit's problems dict: any filename holding both a lectures/<f>.yml manifest and a datasets: entry. It is computed from the two static sources rather than from the scan, so it fires even for a dataset no lecture currently references, and --strict already fails on any non-empty problems value. Measured in both directions rather than asserted. As committed: exit 0, all five problem keys empty. With a single stale entry re-added: exit 1, one `dual_recorded` warning naming the file and the fix. Restored: exit 0. Not rendered on the dashboard. render_audit.py surfaces only migration_inconsistencies, and a --strict failure skips the deploy entirely, so the workflow run is the surface that carries this signal. Closes #86 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR enforces the repository’s “manifest-or-annotation” invariant for migrated datasets by (1) removing stale datasets: entries from scripts/audit_annotations.yml where manifests already exist, and (2) adding a new strict-audit failure condition when a filename appears in both sources.
Changes:
- Delete 8 stale
datasets:entries fromscripts/audit_annotations.ymlthat were duplicated (or contradicted) by correspondinglectures/*.ymlmanifests. - Add a
dual_recordedproblem category inscripts/build_audit.pythat flags any dataset present in both manifests andaudit_annotations.yml’sdatasets:mapping, causing--strictto fail.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| scripts/build_audit.py | Adds dual_recorded detection and emits it under problems so --strict fails when both a manifest and a datasets: annotation exist for the same filename. |
| scripts/audit_annotations.yml | Removes 8 migrated dataset entries from datasets: to restore the “manifest is the source of truth” invariant. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
mmcky
added a commit
that referenced
this pull request
Aug 17, 2026
… (Track C) (#92) * Land wave C1 — the three advanced.myst datasets that needed no rename (Track C) Moves bbh_macro_quarterly.csv, bbh_michigan_monthly.csv and hansen_jagannathan_1991_data.json out of lecture-python-advanced.myst. All three land byte-identical to the blobs the lecture reads today, so the repoint that follows cannot change a figure: sha256 f14f4256…, 567efe5a… and c81333f3…, each checked against `git show HEAD:<path>` in advanced.myst at 00057ba and recorded in the manifests' integrity.sha256. Status is `landed`, not `repointed` — the lecture still reads its own copies. The repoint PR follows this one, in that order: advanced.myst sets execute_notebooks: "cache", so the repointed cells re-execute against whatever is on this repo's main at that moment. TWO BUILDERS RECOVERED, refuting the annotations they replace. Both bbh files were recorded as `constructed-lost` with "extraction not scripted". Both are in fact fully reconstructible from the authors' Zenodo deposit (doi:10.5281/zenodo.10194324), and the builders here reproduce their committed bytes BYTE FOR BYTE — verified by three end-to-end runs each, independently re-run in a clean clone and again in this branch, with `cmp` reporting no difference every time. They therefore land as `builder_status: committed`, not `unrecovered`. The macro builder pulls the 169 KB workbook out of the 198.8 MB archive with HTTP range requests — four requests, ~296 KB — rather than downloading the lot. Neither builder reads live FRED, and the reason is not the obvious one. The bytes are on a 2012 base and FRED has since rebased to 2017, but the lecture takes log first differences and ratios, which are rebasing-invariant — under a pure rebasing 8 of the 9 VAR inputs are bit-identical. The operative risk is upstream REVISIONS, which is why the builders pin the immutable versioned DOI. hansen_jagannathan_1991_data.json stays `unrecovered`, and the distinction is worth keeping. Its construction IS fully recovered — all 2,462 cells re-derive from Shiller's workbooks and seven FRED series to within 4 ULP, and six of eleven columns are bitwise identical end to end — but the BYTES are not reproducible, so no builder can claim to rebuild it. The manifest records the full specification so Phase 9 is a finishing job rather than an investigation, including the unexplained 0.8938191876245914 level factor on annual.consumption, which cancels out of everything the lecture prints. bbh_michigan_monthly.csv lands `redistribution: restricted`. The Zenodo deposit declares CC BY 4.0, but four of its five columns are University of Michigan Surveys of Consumers aggregates and Michigan's usage agreement forbids redistribution without written consent; the depositors are not the rights holder and the deposit does not record consent. Handled as an inherited exposure per AGENTS.md — the file has been served publicly since 2026-06-12 — and registered for licence review. #35's inventory currently records the opposite and needs the correction. Also here because CI enforces them: the three migration.yml records, the three audit_annotations.yml entries retired (a manifest and an annotation together fails the dual_recorded check added in #90), and CATALOG.md regenerated. openpyxl is pinned — both new builders need it, and ames_house_prices.py and japan_population_by_age.py have needed it since they landed without it ever being declared, because CI never runs builders. Verified: strict audit exits 0 with all five problem keys empty; both builders re-run in this branch leave `git diff -- lectures/` empty; the manifests' sha256 fields match the landed bytes; migration.yml parses to 36 datasets, 33 repointed and 3 landed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Record this PR's number in the three migration.yml records Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Note the three consumers as not-yet-repointed, and close the archive in fetch() Copilot review of #92, all four comments valid — and three of them catch this PR breaking a convention it inherited from #91. migration.yml records these three as `landed`, so the lecture still reads its own copies. Under the definition settled in #91, `consumers` answers "what must be rebuilt if these bytes change", and a consumer not yet reading this repo carries a `note` saying so. All three entries were listed without one, which made CATALOG.md's "Used by" column imply a repoint that has not happened. Each now carries a note naming where the lecture actually reads from — a local `_static` path for the two bbh files, an own-repo raw URL for the hansen bundle — and the notes come out in the repoint PR. A worse instance of the same drift, which the review pointed at obliquely: hansen_jagannathan_1991_data.json.yml carried the retired "Every lecture that reads this file FROM THIS REPO" header — the exact wording #91 removed from all eight pre-existing manifests, reintroduced here in a new one because it was drafted from a pre-#91 template. Replaced with the current wording. A grep for that phrase now returns zero manifests again, which is the check worth keeping. builders/bbh_macro_quarterly.py: fetch() read from the ZipFile returned by _open_archive() without closing it, leaking the HTTP range reader's socket to the garbage collector. Now a context manager. This matters more than it looks because the builder is designed to be re-run — the byte-identity check reruns it on every verification pass. bbh_michigan_monthly.py already used `with open_deposit()` and needed no change, which is why the review scoped its comment to the one file. Verified after the changes: both builders still reproduce their committed bytes (git diff over lectures/ empty); the fixed builder raises no ResourceWarning under -W error::ResourceWarning; 36 manifests parse with 6 noted consumers; CATALOG.md regenerated and now renders three warning markers; strict audit exits 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Open
41 tasks
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
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.
Both parts of #86: delete the eight stale entries, and make the invariant they violated something that can actually go red.
The deletions
audit_annotations.yml's own header says a migrated dataset does not get adatasets:entry, because its manifest is the source of truth. Eight datasets held both —assignat.xlsx,caron.npy,chapter_3.xlsx,dette.xlsx,fig_3.xlsx,longprices.xls,mpd2020.xlsx,nom_balances.npy— exactly the set #86 names.Each was diffed against its manifest before deletion rather than deleted on the issue's say-so. All eight are strictly subsumed. The annotations hold a one-line description and a
provenanceword; the manifests hold a title, a full description,class,builder_statusand a source note. Where an annotation carried a real observation, the manifest already carries it better: the two hand-prepared.npyfiles are annotated "no construction record" in prose and recorded machine-readably asclass: constructed+builder_status: unrecovered; the two hand-transcribed spreadsheets are annotated "hand-transcribed by authors" and their manifests quote the consuming lecture saying exactly that.One entry disagreed with its manifest rather than duplicating it.
mpd2020.xlsxwas annotatedprovenance: verbatim. Its manifest recordsclass: constructedwithintegrity.upstream.status: divergedand alocal-editdelta: three header labels on theRegional datasheet were renamed locally, every data value matches upstream, and the lecture reads that sheet withheader=(0,1,2)— so the renames are load-bearing and a pristine upstream copy would silently breaklong_run_growth. The annotation was the stale half, and had been quietly contradicting the manifest. That is the rot the rule exists to prevent, and it is the argument for deleting these rather than leaving them inert.The eight remaining
datasets:entries are all genuinely unmigrated files — the six Track C datasets, Track D'stest_pwt.csv, andgraph.txt— so the intersection withlectures/*.ymlis now empty.The enforcement
dual_recordedis a fifth key in the audit'sproblemsdict: any filename holding both a manifest and adatasets:entry, with a message naming the file and the fix.--strictalready fails on any non-empty problems value, so no gate change was needed.It is computed from the two static sources rather than from the scan, so it fires even for a dataset no lecture currently references — the case a scan-derived check would miss.
Measured in both directions, per the flip-test discipline from #69, rather than asserted:
dual_recordedmpd2020.xlsxand the fixNotes
audit.jsonis generated rather than tracked, so there is no committed artifact to regenerate;CATALOG.mdis untouched.render_audit.pysurfaces onlymigration_inconsistencies, sodual_recordedis asserted but not drawn — and since a--strictfailure skips the Pages deploy entirely, the workflow run is the surface that carries this signal rather than a dashboard nobody would see updated. Worth revisiting if the dashboard ever grows a general problems panel.This also matters for the wave C1 data PR, which will add six manifests for files that currently hold annotations: with this in place, forgetting to retire them turns the audit red instead of passing silently.
Closes #86
🤖 Generated with Claude Code