Land SCF_plus.dta in sources/, and gate sources/ on its recorded hashes (PR B2) - #63
Merged
Conversation
…es (PR B2) The second half of PR B, and the programme's only LFS operation. Adds the SCF+ source extract that produces both published minis, its audit trail, and the CI check that makes that audit trail load-bearing. sources/SCF_plus.dta is 103,934,093 B, verified byte-identical to high_dim_data's LFS object (the pointer's oid IS the sha256). Committed under LFS: the index holds a 134-byte pointer, and lectures/ is untouched and still plain git. That file sits 923,507 B — 0.88% — under GitHub's hard blob limit, which is why AGENTS.md makes `git check-attr filter` a precondition rather than a convention. Below 100 MiB a mis-scoped rule does not error: the push succeeds as plain git and the blob is in history permanently. The sources/ hash gate, which the work plan left as a decision: check_consumed_files.py now asserts, for every file in sources/, that the LFS rule captures it and that it hashes to a sha256 recorded under a `## <filename>` heading in sources/README.md, and it fails on a README entry with no corresponding file. Same principle as #56 — hash whenever a hash is recorded — keyed on the README, because sources/ files carry no manifest by design. It reads the pointer's oid rather than the object, so it works under the `lfs: false` checkout both workflows use and costs no LFS bandwidth. Exercised against all six branches before landing: clean with real bytes, clean with pointer text as CI sees it, drifted bytes, a mis-scoped .gitattributes, a missing README section, and a stale README entry. Without it sources/ would carry no validation of any kind while every file in lectures/ is validated as it migrates — and this file is the provenance root for two published datasets, so a drift would make both unreproducible silently. Also folds in two docs that PR B1 left stale: builders/README.md's coverage report was still 13 constructed / 7 builders and did not list the two frozen builders, and AGENTS.md did not record that either sources/ rule is now enforced. generating_mini.md is not edited, including its high_dim_data input URL. The substitution is recorded as prose in sources/README.md, which is where a frozen builder's corrections belong. Part of #2. See QuantEcon/workspace-lectures#23. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…s it too The comment ended "and nothing here reads `sources/`", which this PR makes false — the job now checks every file there. `lfs: false` is still right, for the opposite reason from the lectures/ case. Those files are meant to be LFS, so the checkout hands us pointers, and a pointer's oid IS the object's sha256 — the value sources/README.md records. So ~100 MiB is verified without fetching a byte of it, and the red-on-mistake direction holds either way: a sources/ file committed as plain git arrives as real bytes and hashes to something unrecorded. Both comments in this file had their reasoning inverted once before (#57), which is why this one is worth spelling out rather than deleting.
There was a problem hiding this comment.
Pull request overview
This PR folds the SCF+ upstream extract into sources/ (as an LFS-tracked builder input) and makes the sources/README.md audit trail CI-enforced by extending the existing check_consumed_files.py guardrail.
Changes:
- Add
sources/SCF_plus.dtaas a Git LFS pointer and introducesources/README.mdas the provenance/audit record forsources/. - Extend
.github/scripts/check_consumed_files.pyto validatesources/files are LFS-tracked and have matching sha256 entries recorded under## <filename>headings insources/README.md. - Update docs (
builders/README.md,AGENTS.md) to reflect frozen builders and the new enforcement behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
sources/SCF_plus.dta |
Adds the SCF+ extract as an LFS pointer under sources/ (not published/served). |
sources/README.md |
Adds the sources/ audit trail and SCF+ provenance entry (origin, licence/retrieval gaps, sha256, consumer builder). |
builders/README.md |
Updates builder coverage/status listing, including frozen builders. |
AGENTS.md |
Documents that CI now enforces sources/ audit-trail hashing and notes the “check-attr before add” trap. |
.github/scripts/check_consumed_files.py |
Implements the sources/ hash/LFS-tracking gate keyed on sources/README.md. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
All five were valid and all five are on code this PR introduced. The one that changes behaviour: the README parser treated ANY `##` section containing a 64-hex token as a file entry, and sources/README.md already has two prose sections. A sha256 quoted as an example in either would have registered as a recorded file and then failed the no-such-file check. Headings must now look like filenames (SOURCE_HEADING_RE), which keeps the stale-entry check working in both directions — keying on "matches a real file" would have removed it. lfs_tracked() ignored git's exit status, so any git failure returned False and reported "not captured by the LFS rule" for every file — the precise catastrophe the assertion exists to detect. A broken environment announcing that disaster is worse than no check, so a non-zero exit and a missing git binary now get their own message saying the assertion could not be evaluated either way. read_text() decoded with the platform locale on a README full of em-dashes. Fixed, and fixed at :88 too, which Copilot did not flag and which has the same bug against manifests that are also full of them. LFS_POINTER_RE pinned exact LF line endings and a trailing newline. `-text` on sources/** plus ubuntu CI makes CRLF near-unreachable, but the fallback was to hash the pointer text and report "committed bytes do not match" for a correct object. Now tolerant of CRLF and trailing whitespace, still far too tight for a real data file to match. builders/README.md said generating_mini.md "Reads sources/SCF_plus.dta in substance", which hedged correctly but read as a behaviour claim in a Status column. Reworded to Copilot's suggestion. Re-ran the branch sweep, now eleven cases: real bytes, pointer, pointer without a trailing newline, CRLF pointer, extra newlines, a prose section quoting a sha256, a stale filename entry, drifted bytes, a mis-scoped .gitattributes, a missing README section, and git absent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 10, 2026
mmcky
added a commit
that referenced
this pull request
Aug 10, 2026
* PLAN: record the P3 data half, and retract a box that is now wrong Housekeeping after #62 and #63. The retraction matters more than the ticks. `Repoint generating_mini.md's input URL` was written while #14 was open and said to do it "either way". #14 then settled the question the other way: the builder lands as provenance, committed-frozen, not edited at all. The box is struck rather than deleted, with the reasoning, because its worry was legitimate — an archived repo keeps serving that URL — and the answer is that the rule binds builders that RUN. Left as a bare unticked box it reads as outstanding work that would destroy the artifact's value as provenance. Corrected while here: the fold's consuming reads are 28 across FOUR repos, not 21 across three. `test-actions-lecture-intro` was found after that line was written and has been mistaken for out-of-scope once already, so the old figure is named rather than silently replaced. Ticked with their outcomes: sources/ exists and its README is now load-bearing rather than documentary (the hash gate), and the 0.88% blob-limit margin is recorded where it explains why check-attr is a gate. Two additions that are not just status. The headline now explains why CATALOG.md says 24 datasets while audit.json says 18 migrated — one counts what lives here, the other what is read from here, and a wave that lands ahead of its repoints shows the gap by design. And P3 records three things it proved that were not on its test list, including that a constructed dataset's builder must land in the same PR as its data. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * PLAN: describe the sources/README format CI actually enforces Copilot review on #64. "one row per committed file" predates the format and is now wrong in a way that costs someone a red build. check_sources() splits the README on `## ` headings and reads the first 64-hex token in each filename-shaped section, so a second sources/ file documented as a row in a shared table has no recorded hash and fails the required check. AGENTS.md already described the enforced shape correctly; PLAN.md was the last place carrying the old one, which is exactly the drift this document's own header warns about. The retracted sentence is named rather than silently replaced, on the same principle as the generating_mini.md box in the previous commit: a reader who remembers the old wording should learn it was wrong, not wonder whether they misread it. Also named the two paths behind "both sources/ paths return 404 on Pages", and re-measured them: sources/SCF_plus.dta and sources/README.md are 404, lectures/cities_us.csv is 200. Every other measurement in this document names its target; that one did not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
mmcky
added a commit
that referenced
this pull request
Aug 12, 2026
* Record graph.txt's disposition now that wasm embeds it QuantEcon/lecture-wasm#63 stopped lecture-wasm fetching lecture-python-intro's committed graph.txt over the network and gave it a `%%file` cell like every sibling repo. That was the only URL read of that blob anywhere in the organisation, so three records here are now wrong. `scripts/audit_annotations.yml`: - the graph.txt note said lecture-wasm "fetches intro's committed copy by URL (requests.get), making that copy load-bearing". No longer true. Every consumer now embeds the data in its own lecture, and graph.txt has dropped out of the scanned dataset set entirely — it is `embedded` in four repos and a shadowed orphan in the rest. - the `lecture-wasm:lectures/graph.txt` mirror-orphan entry names a file that #63 deleted. Dead key, removed. - intro's copy became a BARE orphan the moment that read went away — `kind: orphan`, no note — which reads as an unexplained stray file. It is annotated `shadowed`, matching lecture-dp's identical case: short_path regenerates it via `%%file` before reading it, so the committed bytes are never consumed (the build prints "Overwriting graph.txt" where lecture-jax, which commits no copy, prints "Writing graph.txt"). `PLAN.md`: Track A is complete. The last two CSVs landed as wave A4 (#74, flipped in #75), and graph.txt is recorded as a deliberate NON-migration rather than a remaining item — synthetic teaching data, null in every provenance field, and the exercise teaches its format by quoting the first line, so the data has to stay visible on the page. Hosting it here would have put a toy in a registry that exists to carry provenance. The note also carries the deletion caveat, because the obvious next step is wrong at scale: intro's copy is deletable as Track X, but the same blob sits in 8 repos and is regenerated at 17 `%%file` sites including archived `.rst` ancestors that `gh search code` cannot see. That deletion needs a per-repo reader sweep, not an org-wide one. Figures re-read from audit.json rather than carried: 40 static files (was 41 — graph.txt is no longer a scanned dataset), 38 committed files (was 43 — five deletions across wasm and intro), 24 orphans, 26 migrated. Strict audit exit 0, zero warnings, zero problems in all four categories; catalog freshness gate green. * Copilot: distinguish 'shadowed' from 'embedded', and fix a contradiction in rule 2 Two review comments, both correct, plus a third instance of the first one that the review did not flag. **'shadowed everywhere' was wrong** (audit_annotations.yml). Shadowed means a committed copy exists and is overwritten by the %%file cell before being read. That is true of two of the four embedding repos, not all of them: lecture-python-intro %%file + committed copy -> shadowed lecture-dp %%file + committed copy -> shadowed lecture-jax %%file, no copy -> just embedded lecture-wasm %%file, no copy -> just embedded The note implied a committed copy exists everywhere, which is backwards for the two repos that model the shape we actually want. Reworded to split the cases. **PLAN.md rule 2 contradicted the rest of the document.** It still read "(graph.txt is Track A but single-consumer: only lecture-wasm reads intro's committed copy.)" — false twice over after this PR, since wasm no longer reads it and it is not a Track A item. Rule 2 is where someone scoping a repoint set looks, so the parenthetical is rewritten to say graph.txt is out of scope and why, rather than deleted. **The same imprecision was in PLAN.md's own disposition note**, which the review did not comment on: "embedded in four repos and a shadowed orphan in the rest" puts intro and dp in both sets. Now states which repos embed, which of those also commit a copy, and that the remaining committed copies (lecture-intro.zh-cn, the canary, lecture-python.zh-cn, lecture-dp.monorepo, ipynb_pdf_constructor) are read by nothing. Verified against the repos rather than restated: all four embed via %%file; only intro and dp commit a copy. Strict audit exit 0 with zero warnings, catalog gate green, audit_annotations.yml parses with the dead wasm key gone and the intro key present.
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.
PR B2 of the
high_dim_datafold — the SCF+ source extract, its audit trail, and the CI gate that makes that audit trail load-bearing. Follows #62. Work plan: QuantEcon/workspace-lectures#23.This is the programme's only LFS operation and its only ~100 MiB push. Nothing here is served, nothing downstream reads it, and
lectures/is untouched.sources/SCF_plus.dtaVerified byte-identical to
high_dim_data's LFS object atc208ccd4…3f01— the pointer'soidis the object's sha256, so the comparison is exact.Storage verified at every layer rather than assumed:
git check-attr filterbeforegit addfilter: lfssources/README.mdlectures/SCF_plus_mini.csvin the indexWhy the check-attr step is a gate and not a formality
The file is 923,507 B — 0.88% — under GitHub's 104,857,600 B hard blob limit. Below that limit a mis-scoped LFS rule does not error: the push simply succeeds as plain git, and a 99 MiB blob is in history permanently with no way out short of a history rewrite. The failure is silent in the one direction that cannot be undone.
The
sources/hash gate — the decision the plan left opencheck_consumed_files.pynow asserts, for every file insources/:sources/README.mdrecords itssha256under a## <filename>heading;And it fails on a README entry with no corresponding file, because a stale audit trail is worse than none.
Same principle as #56 — hash whenever a hash is recorded — keyed on the README rather than a manifest, since
sources/files carry no manifest by design. It reads the pointer's oid rather than the object, so it works under thelfs: falsecheckout both workflows use and costs zero LFS bandwidth.Without it,
sources/would carry no validation of any kind while every file inlectures/is validated as it migrates — and this file is the provenance root for two published datasets, so a drift would make both unreproducible silently.Exercised against all six branches before landing, not just the happy path: clean with real bytes; clean with pointer text as CI sees it; drifted bytes; a mis-scoped
.gitattributes; a missing README section; a stale README entry. Exit codes 0/0/1/1/1/1 respectively.Provenance, recorded honestly
sources/README.mdcarries origin, retrieval, licence, upstream identifier,sha256and consuming builder, per AGENTS.md. Two of those are nulls with a reason: the SCF+ deposit could not be located — Crossref registers10.1086/708815with a null licence and no data relation, DataCite and Harvard Dataverse return zero, openICPSR and the JPE supplement are 403. Gate 4 Decision 2, settled by exhaustion. Registered on #35.generating_mini.mdis not edited, including itshigh_dim_datainput URL. It iscommitted-frozen, so editing it destroys the property that makes it worth keeping; the rule requiring a builder to read fromsources/binds builders that run. The substitution is recorded as prose insources/README.md, which is where a frozen builder's corrections belong.Two docs B1 left stale
builders/README.md's coverage report still said 13 constructed / 7 builders — it is now 17 and 9 — and did not list the two frozen builders at all. That listing is the coverage report, so a stale one understates the Phase 9 backlog.AGENTS.mddid not record that eithersources/rule is now CI-enforced, and itscheck-attrline now says explicitly that CI catches this only after the blob is already in the branch's history — so run it yourself first.Still open, deliberately not in this PR
The
webscrape_forbes.ipynbrepublication question — it spoofs a browser user-agent and hardcodes GDPR consent cookies against an undocumented internal API — belongs on #35 as a row separate from the Forbes data licence. Filing next.🤖 Generated with Claude Code