From 76b8d0d8858fb47593454ec5aa874216d6028dfe Mon Sep 17 00:00:00 2001 From: Matt McKay Date: Mon, 10 Aug 2026 13:51:12 +1000 Subject: [PATCH 1/3] Land SCF_plus.dta in sources/, and gate sources/ on its recorded hashes (PR B2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 `## ` 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 QuantEcon/data-lectures#2. See QuantEcon/workspace-lectures#23. Co-Authored-By: Claude Opus 5 (1M context) --- .github/scripts/check_consumed_files.py | 118 +++++++++++++++++++++++ AGENTS.md | 4 +- builders/README.md | 11 ++- sources/README.md | 119 ++++++++++++++++++++++++ sources/SCF_plus.dta | 3 + 5 files changed, 251 insertions(+), 4 deletions(-) create mode 100644 sources/README.md create mode 100644 sources/SCF_plus.dta diff --git a/.github/scripts/check_consumed_files.py b/.github/scripts/check_consumed_files.py index be8859b..58f66ae 100644 --- a/.github/scripts/check_consumed_files.py +++ b/.github/scripts/check_consumed_files.py @@ -25,17 +25,33 @@ Files with no manifest yet (Phase 6 backfill pending), and manifests that record no hash, are out of scope here — the full validation suite (schema, dtypes, invariants) is PLAN Phase 5 and will subsume this check. + +Separately, for every file in sources/ (builder inputs, never served, no +manifest — sources/README.md is their audit trail): + + - the LFS rule must actually capture it + - sources/README.md must record its sha256, under a `## ` heading + - the committed bytes must hash to that value + +Same principle as the manifest gate above, keyed on the README instead: hash +whenever a hash is recorded. Without it, sources/ would carry no validation of +any kind while every file in lectures/ is validated as it migrates — and +SCF_plus.dta is the provenance root for two published datasets, so if its bytes +drifted both would become unreproducible and nothing would notice. """ from __future__ import annotations import hashlib import pathlib +import re +import subprocess import sys import yaml REPO = pathlib.Path(__file__).resolve().parents[2] LECTURES = REPO / "lectures" +SOURCES = REPO / "sources" # One builder per published dataset, in builders/ (AGENTS.md, "Builders"). # `committed` asserts a runnable four-stage builder; `committed-frozen` says the @@ -45,6 +61,15 @@ BUILDER_STATUSES = {"committed", "committed-frozen", "unrecovered", "not-applicable"} +SHA256_RE = re.compile(r"\b([0-9a-f]{64})\b") +# An LFS pointer is <200 bytes of text whose second line is `oid sha256:`. +# That oid IS the object's sha256, which is what makes this check work under +# `lfs: false` — the real bytes are never fetched and never need to be. +LFS_POINTER_RE = re.compile( + rb"\Aversion https://git-lfs\.github\.com/spec/v1\noid sha256:([0-9a-f]{64})\nsize (\d+)\n\Z" +) + + def sha256(path: pathlib.Path) -> str: h = hashlib.sha256() with open(path, "rb") as f: @@ -176,6 +201,8 @@ def main() -> int: f"vintages')" ) + checked += check_sources(errors) + for e in errors: print(f"::error::{e}") print( @@ -185,5 +212,96 @@ def main() -> int: return 1 if errors else 0 +def lfs_tracked(path: pathlib.Path) -> bool: + """Whether the LFS rule captures `path`, per .gitattributes.""" + rel = path.relative_to(REPO).as_posix() + out = subprocess.run( + ["git", "check-attr", "filter", "--", rel], + cwd=REPO, capture_output=True, text=True, + ) + return out.stdout.strip().endswith(": lfs") + + +def check_sources(errors: list[str]) -> int: + """Verify sources/ against the sha256 values in sources/README.md. + + Two assertions per file. The LFS one is not ceremony: SCF_plus.dta sits + 923,507 B (0.88%) under GitHub's hard blob limit, so a mis-scoped + .gitattributes does not error — the push succeeds as plain git and the blob + is in history permanently. That failure is silent in the one direction that + cannot be undone, so it is worth a check rather than a convention. + """ + if not SOURCES.is_dir(): + return 0 + + readme = SOURCES / "README.md" + if not readme.exists(): + errors.append( + "sources/README.md is missing — it is the audit trail for a " + "directory whose files carry no manifest (AGENTS.md, 'LFS, and " + "sources/ vs lectures/')" + ) + return 0 + + # `## ` starts a section; the first 64-hex token inside it is that + # file's recorded sha256. Parsed by section rather than by table cell so the + # README stays free to change its formatting. + sections = re.split(r"^## +", readme.read_text(), flags=re.M)[1:] + recorded: dict[str, str] = {} + for sec in sections: + name = sec.splitlines()[0].strip().strip("`") + if m := SHA256_RE.search(sec): + recorded[name] = m.group(1) + + files = sorted(p for p in SOURCES.iterdir() if p.is_file() and p.name != "README.md") + checked = 0 + + for path in files: + if not lfs_tracked(path): + errors.append( + f"sources/{path.name}: not captured by the LFS rule — " + f"`git check-attr filter` does not say `lfs`. Everything under " + f"sources/ must be LFS-tracked; a mis-scoped rule commits the " + f"real bytes as plain git and does not error below 100 MiB" + ) + + want = recorded.get(path.name) + if not want: + errors.append( + f"sources/{path.name}: no sha256 recorded in sources/README.md " + f"— add a `## {path.name}` section with its origin, retrieval, " + f"licence, sha256 and consuming builder" + ) + continue + + # Under `lfs: false` the working file IS the pointer, and the pointer's + # oid is the object's sha256 — so this verifies the real bytes without + # fetching ~100 MiB of them. With LFS smudge on locally it is the real + # file, and hashing it gives the same answer. + blob = path.read_bytes() if path.stat().st_size < 1024 else None + if blob is not None and (m := LFS_POINTER_RE.match(blob)): + actual, kind = m.group(1).decode(), "LFS pointer oid" + else: + actual, kind = sha256(path), "committed bytes" + + checked += 1 + if actual != want: + errors.append( + f"sources/{path.name}: {kind} {actual} does not match the " + f"sha256 recorded in sources/README.md ({want}). This file is " + f"a builder input, so a drift here makes its outputs " + f"unreproducible — update the README in the same PR if the " + f"change is deliberate" + ) + + for name in sorted(set(recorded) - {p.name for p in files}): + errors.append( + f"sources/README.md records `{name}`, which is not in sources/ — " + f"a stale audit-trail entry is worse than none" + ) + + return checked + + if __name__ == "__main__": sys.exit(main()) diff --git a/AGENTS.md b/AGENTS.md index fb52da9..b1ad0c9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -84,7 +84,7 @@ The failure modes are silent in both directions, which is why all three are mach LFS exists here for one purpose: **upstream inputs that builders consume and no lecture reads**, which live in `sources/` and are never served. - `lectures/` — a published dataset. Plain git, sidecar manifest required, its filename is an API. -- `sources/` — a builder input. Per-path LFS, **no** manifest, not served, recorded instead in `sources/README.md` — the audit trail: origin, retrieval date, licence, upstream identifier (DOI where one exists), `sha256`, and the builder that consumes it. +- `sources/` — a builder input. Per-path LFS, **no** manifest, not served, recorded instead in `sources/README.md` — the audit trail: origin, retrieval date, licence, upstream identifier (DOI where one exists), `sha256`, and the builder that consumes it. **CI enforces the last of those**: `check_consumed_files.py` requires every file here to be captured by the LFS rule and to hash to a `sha256` recorded under a `## ` heading in that README, and fails on a README entry with no file. It reads the pointer's `oid` rather than the object, so it costs no LFS bandwidth. Rules that still apply: @@ -92,7 +92,7 @@ Rules that still apply: - Do not LFS-track an **existing** file until you've confirmed no consumer fetches it via `raw.githubusercontent.com` — converting silently turns their download into pointer text. - A builder must read its input from `sources/`, never over the network from another QuantEcon repo. That is how a retired repo becomes load-bearing again. - **Two** workflows check this repo out, and both now say `lfs: false` — `.github/workflows/audit-dashboard.yml` (the Pages deploy) and `.github/workflows/consumed-file-check.yml` (every pull request). Leave them that way: `lfs: false` is the assertion that nothing published is an LFS object. If a `lectures/` file is ever tracked by mistake, the checker hashes the pointer and goes red, and Pages deploys the same pointer bytes a reader would get from `raw.githubusercontent.com` — whereas `lfs: true` fetches the real bytes, passes green, and publishes a file that works only from Pages. It also keeps `sources/` (a 99 MiB LFS object) off every run; LFS bandwidth is an org-wide quota. -- `git check-attr filter -- sources/` must print `filter: lfs` **before** you `git add` anything to `sources/`. `SCF_plus.dta` is 103,934,093 B against GitHub's 104,857,600 B hard limit, so a mis-scoped rule does not error — the push succeeds as plain git and the blob is in history permanently. +- `git check-attr filter -- sources/` must print `filter: lfs` **before** you `git add` anything to `sources/`. `SCF_plus.dta` is 103,934,093 B against GitHub's 104,857,600 B hard limit, so a mis-scoped rule does not error — the push succeeds as plain git and the blob is in history permanently. CI asserts this too, but only after the fact: by the time a PR goes red the blob is already in the branch's history, so run it yourself first. #### When a published file approaches the 100 MiB blob limit diff --git a/builders/README.md b/builders/README.md index 28393be..9347ece 100644 --- a/builders/README.md +++ b/builders/README.md @@ -40,9 +40,16 @@ re-fetched** — see `AGENTS.md`. | `japan_population_by_age.py` | `japan_population_by_age.csv` | committed | | `us_adult_heights.py` | `us_adult_heights.csv` | committed | | `business_cycle.py` | `business_cycle_data.csv`, `business_cycle_info.md`, `business_cycle_metadata.md` | run by hand, no validate stage yet (PLAN Phase 5); its three outputs are the repo's only unmanifested files | +| `webscrape_forbes.ipynb` | `forbes-global2000.csv`, `forbes-billionaires.csv` | **committed-frozen** — an undocumented Forbes API, a spoofed user-agent and hardcoded GDPR consent cookies. Defects recorded in the two manifests rather than fixed | +| `generating_mini.md` | `SCF_plus_mini.csv`, `SCF_plus_mini_no_weights.csv` | **committed-frozen** — its `to_csv` calls are commented out upstream and stay that way. Reads `sources/SCF_plus.dta` in substance; the URL in the file is historical, see `sources/README.md` | -**This listing is the coverage report.** The repo has 13 `constructed` datasets -and 7 builders; the difference is the Phase 9 recovery backlog, carried as +Both frozen builders keep their upstream `high_dim_data` filenames rather than +being renamed to their set stems (`forbes`, `SCF_plus_mini`), which preserves +the textual link to that repo's history. Permitted by the rule above — what CI +asserts is that the path exists. + +**This listing is the coverage report.** The repo has 17 `constructed` datasets +and 9 builders; the difference is the Phase 9 recovery backlog, carried as `builder_status: unrecovered` in each manifest rather than hidden by reclassifying the file as `verbatim`. diff --git a/sources/README.md b/sources/README.md new file mode 100644 index 0000000..59c9a0a --- /dev/null +++ b/sources/README.md @@ -0,0 +1,119 @@ +# sources + +Inputs that a builder consumes and **no lecture reads**. This directory sits +outside the published tree — it is never served, and nothing here has a sidecar +manifest. This file is the audit trail instead: origin, retrieval, licence, +upstream identifier, `sha256`, and the builder that consumes each entry. + +## What belongs here — and what does not + +The defining property is **un-refetchability**, not size. + +The normal case for a builder is to fetch from its third-party upstream at run +time, and that is what most of them do: `jse.amstat.org`, `earthquake.usgs.gov`, +`wwwn.cdc.gov`, `stat.go.jp`, `openfootball`. **None of the six `committed` +builders in this repo has a committed input.** So `sources/` is not "where +builder inputs live" as a general rule — it is the exception layer for an input +that cannot be obtained again. + +It is also emphatically **not "the big-file directory"**, even though +`.gitattributes` LFS-tracks everything under it. A 300 MB file that can be +re-fetched from a stable upstream does not belong here; a 4 KB file whose source +has vanished does. + +What it must never hold is a network read from another QuantEcon repo. That is +how a retired repo becomes load-bearing again. + +## LFS, and why this directory has it + +`sources/**` is LFS-tracked and `lectures/` is not — deliberately, and in that +direction only ([#58](https://github.com/QuantEcon/data-lectures/issues/58)). +The published tree is 100% plain git because an LFS-tracked path returns **HTTP +200 with ~130 bytes of pointer text** from `raw.githubusercontent.com`: a reader +gets a parse error rather than a 404, `pd.read_csv` raises nothing, and a +status-code check reads as green. Nothing here is served, so nothing here can +meet that trap. + +`README.md` is excluded from the LFS rule so this audit trail stays readable +text on GitHub. + +**Before `git add`ing anything to this directory**, confirm the rule actually +captures it: + + git check-attr filter -- sources/ # must print: filter: lfs + +That is a real gate, not a formality — see the size note under `SCF_plus.dta`. + +--- + +## `SCF_plus.dta` + +| | | +| --- | --- | +| **Origin** | Inherited from `QuantEcon/high_dim_data` (`SCF_plus/SCF_plus.dta`), where it was LFS-tracked. Folded in 2026-08-10 when that repo was retired | +| **Upstream** | SCF+ — Kuhn, Schularick and Steins (2020), *Income and Wealth Inequality in America, 1949-2016*, Journal of Political Economy 128(9), 3469-3519 | +| **Upstream identifier** | DOI [10.1086/708815](https://doi.org/10.1086/708815) — the **article**. No data deposit was locatable; see "Provenance gap" below | +| **Retrieved** | `null` — no retrieval date was recorded upstream, and the per-file commit dates in `high_dim_data` record when QuantEcon acquired it, not when it was obtained from the source. Do not promote one to the other | +| **Licence** | `null` — no licence statement was locatable at any deposit. Registered on [#35](https://github.com/QuantEcon/data-lectures/issues/35) | +| **`sha256`** | `c208ccd49b3bd11205a88bce08864ea445898b182098002fd6b5e38664aa3f01` | +| **Size** | 103,934,093 B | +| **Consumed by** | `builders/generating_mini.md` (`builder_status: committed-frozen`) | +| **Produces** | `lectures/SCF_plus_mini.csv`, `lectures/SCF_plus_mini_no_weights.csv` | + +### The size note — this file must stay LFS-tracked permanently + +103,934,093 B against GitHub's hard blob limit of 104,857,600 B leaves +**923,507 B of headroom — 0.88%**. + +That margin is what makes the `check-attr` gate above load-bearing rather than +ceremonial. A mis-scoped LFS rule does **not** error on a file this size: the +push simply succeeds as plain git, and a 99 MiB blob is in the repository's +history permanently, with no way to remove it short of a history rewrite. The +failure is silent in exactly the direction that cannot be undone. + +An upstream vintage 1% larger could not be pushed as plain git at all. + +### Provenance gap + +The SCF+ deposit record could not be located, and this was searched to +exhaustion on 2026-08-10 rather than assumed: + +- Crossref registers `10.1086/708815` with `license: null` and **no data + relation**; +- DataCite returns zero results; +- Harvard Dataverse returns zero results; +- openICPSR and the JPE supplementary-material path both return **403**. + +So `retrieved` and `license` above are honest nulls with this note, not +placeholders awaiting a lookup someone else should repeat. If a deposit is +located later, both fields and the two manifests' `integrity.upstream` blocks +can be filled in together. + +The variable dictionary inherited as `SCF_plus/README.md` upstream is **not** +provenance: it carries no URL, no date, no licence and no DOI. Its content was +migrated into `lectures/SCF_plus_mini.csv.yml`'s `schema.columns[].description` +rather than landing here as a second, unstructured record beside the structured +one. + +### The builder reads a URL, not this file — and that is deliberate + +`builders/generating_mini.md` still contains: + + pd.read_stata('https://github.com/QuantEcon/high_dim_data/blob/main/SCF_plus/SCF_plus.dta?raw=true') + +That URL is **historical, not a live dependency.** The builder is +`committed-frozen`: it is kept as the record of what produced the two published +extracts and deliberately will not run, so it is committed verbatim and not +edited — editing it is what would destroy its value as provenance +([#14](https://github.com/QuantEcon/data-lectures/issues/14), +[#61](https://github.com/QuantEcon/data-lectures/pull/61)). + +The rule requiring a builder to read its input from `sources/` binds builders +that **run**. This is the substitution recorded as prose, which is where a +frozen builder's corrections belong: **the input is now committed at +`sources/SCF_plus.dta`, byte-identical to what that URL served.** + +`high_dim_data` is archived rather than deleted, so the URL above continues to +resolve. Do not treat that as a reason to leave the dependency live in any +builder that does run — and do not delete branches or rewrite history on that +repo after archiving, which is what actually breaks an external reader. diff --git a/sources/SCF_plus.dta b/sources/SCF_plus.dta new file mode 100644 index 0000000..45b7b52 --- /dev/null +++ b/sources/SCF_plus.dta @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:c208ccd49b3bd11205a88bce08864ea445898b182098002fd6b5e38664aa3f01 +size 103934093 From 049ede45aa5c51c3db896a2ab79a6db4c9f026a0 Mon Sep 17 00:00:00 2001 From: Matt McKay Date: Mon, 10 Aug 2026 13:54:18 +1000 Subject: [PATCH 2/3] consumed-file-check: the lfs:false comment now says why sources/ wants it too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .github/workflows/consumed-file-check.yml | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/.github/workflows/consumed-file-check.yml b/.github/workflows/consumed-file-check.yml index 5b68704..26994aa 100644 --- a/.github/workflows/consumed-file-check.yml +++ b/.github/workflows/consumed-file-check.yml @@ -21,8 +21,16 @@ jobs: # goes red — which is exactly what a reader would get from # raw.githubusercontent.com. Fetching the real bytes would hash them # correctly and pass green while every consumer downloads ~130 bytes - # of pointer text. LFS belongs to `sources/**` only (.gitattributes), - # and nothing here reads `sources/`. + # of pointer text. LFS belongs to `sources/**` only (.gitattributes). + # + # This job DOES read `sources/` now, and `lfs: false` is right there + # too, for the opposite reason: those files are meant to be LFS, so + # the checkout hands us pointers — and a pointer's `oid` IS the + # object's sha256, which is the value sources/README.md records. The + # check verifies ~100 MiB of bytes without fetching one of them, and + # the direction still holds: a `sources/` file committed as plain git + # by mistake arrives as real bytes, hashes to something the README + # does not record, and goes red. lfs: false - uses: actions/setup-python@v5 with: From d6c81e1b0f18619af88ba34711d239b99ad9d5a4 Mon Sep 17 00:00:00 2001 From: Matt McKay Date: Mon, 10 Aug 2026 14:00:39 +1000 Subject: [PATCH 3/3] Harden the sources/ gate against Copilot's five findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .github/scripts/check_consumed_files.py | 66 ++++++++++++++++++++----- builders/README.md | 2 +- 2 files changed, 55 insertions(+), 13 deletions(-) diff --git a/.github/scripts/check_consumed_files.py b/.github/scripts/check_consumed_files.py index 58f66ae..074df72 100644 --- a/.github/scripts/check_consumed_files.py +++ b/.github/scripts/check_consumed_files.py @@ -62,11 +62,25 @@ SHA256_RE = re.compile(r"\b([0-9a-f]{64})\b") +# A `sources/` entry is a `## ` section. Requiring the heading to look +# like a filename is what keeps a prose section from being read as one: the +# README has several, and a sha256 quoted in an example inside any of them would +# otherwise register as a recorded file and fail the no-such-file check below. +SOURCE_HEADING_RE = re.compile(r"^[\w.\-]+\.\w+$") # An LFS pointer is <200 bytes of text whose second line is `oid sha256:`. # That oid IS the object's sha256, which is what makes this check work under # `lfs: false` — the real bytes are never fetched and never need to be. +# +# Line endings are tolerated rather than pinned. `.gitattributes` sets `-text` on +# sources/** so git does no conversion, and CI is ubuntu — but if a pointer ever +# did pick up a CR or a stray trailing newline, a stricter pattern would fall +# through to hashing the pointer text and report "committed bytes do not match" +# for an object that is perfectly correct. Still far too tight for any real +# data file to match by accident. LFS_POINTER_RE = re.compile( - rb"\Aversion https://git-lfs\.github\.com/spec/v1\noid sha256:([0-9a-f]{64})\nsize (\d+)\n\Z" + rb"\Aversion https://git-lfs\.github\.com/spec/v1\r?\n" + rb"oid sha256:([0-9a-f]{64})\r?\n" + rb"size (\d+)\s*\Z" ) @@ -85,7 +99,9 @@ def main() -> int: for manifest_path in manifests: try: - manifest = yaml.safe_load(manifest_path.read_text()) + # Explicit encoding: manifests carry em-dashes and non-ASCII source + # names, and read_text() otherwise decodes with the platform locale. + manifest = yaml.safe_load(manifest_path.read_text(encoding="utf-8")) except yaml.YAMLError as exc: errors.append(f"{manifest_path.name}: invalid YAML — {exc}") continue @@ -212,14 +228,30 @@ def main() -> int: return 1 if errors else 0 -def lfs_tracked(path: pathlib.Path) -> bool: - """Whether the LFS rule captures `path`, per .gitattributes.""" +def lfs_tracked(path: pathlib.Path) -> tuple[bool, str | None]: + """Whether the LFS rule captures `path`, per .gitattributes. + + Returns (tracked, error). A git failure must NOT come back as `False`: an + empty stdout would then read as "not captured by the LFS rule", which is + the one catastrophe this check exists to report. A broken environment + reporting the disaster it is meant to detect is worse than no check, so it + gets its own message. + """ rel = path.relative_to(REPO).as_posix() - out = subprocess.run( - ["git", "check-attr", "filter", "--", rel], - cwd=REPO, capture_output=True, text=True, - ) - return out.stdout.strip().endswith(": lfs") + try: + out = subprocess.run( + ["git", "check-attr", "filter", "--", rel], + cwd=REPO, capture_output=True, text=True, + ) + except OSError as exc: # git absent entirely + return False, f"could not run `git check-attr` ({exc})" + if out.returncode != 0: + detail = out.stderr.strip().splitlines() + return False, ( + f"`git check-attr` exited {out.returncode}" + + (f" — {detail[0]}" if detail else "") + ) + return out.stdout.strip().endswith(": lfs"), None def check_sources(errors: list[str]) -> int: @@ -245,11 +277,14 @@ def check_sources(errors: list[str]) -> int: # `## ` starts a section; the first 64-hex token inside it is that # file's recorded sha256. Parsed by section rather than by table cell so the - # README stays free to change its formatting. - sections = re.split(r"^## +", readme.read_text(), flags=re.M)[1:] + # README stays free to change its formatting. Headings that are not + # filename-shaped are prose and are skipped — see SOURCE_HEADING_RE. + sections = re.split(r"^## +", readme.read_text(encoding="utf-8"), flags=re.M)[1:] recorded: dict[str, str] = {} for sec in sections: name = sec.splitlines()[0].strip().strip("`") + if not SOURCE_HEADING_RE.match(name): + continue if m := SHA256_RE.search(sec): recorded[name] = m.group(1) @@ -257,7 +292,14 @@ def check_sources(errors: list[str]) -> int: checked = 0 for path in files: - if not lfs_tracked(path): + tracked, git_error = lfs_tracked(path) + if git_error: + errors.append( + f"sources/{path.name}: {git_error}. This is a tooling failure, " + f"not a finding about the file — the LFS assertion could not be " + f"evaluated either way" + ) + elif not tracked: errors.append( f"sources/{path.name}: not captured by the LFS rule — " f"`git check-attr filter` does not say `lfs`. Everything under " diff --git a/builders/README.md b/builders/README.md index 9347ece..8aa9b12 100644 --- a/builders/README.md +++ b/builders/README.md @@ -41,7 +41,7 @@ re-fetched** — see `AGENTS.md`. | `us_adult_heights.py` | `us_adult_heights.csv` | committed | | `business_cycle.py` | `business_cycle_data.csv`, `business_cycle_info.md`, `business_cycle_metadata.md` | run by hand, no validate stage yet (PLAN Phase 5); its three outputs are the repo's only unmanifested files | | `webscrape_forbes.ipynb` | `forbes-global2000.csv`, `forbes-billionaires.csv` | **committed-frozen** — an undocumented Forbes API, a spoofed user-agent and hardcoded GDPR consent cookies. Defects recorded in the two manifests rather than fixed | -| `generating_mini.md` | `SCF_plus_mini.csv`, `SCF_plus_mini_no_weights.csv` | **committed-frozen** — its `to_csv` calls are commented out upstream and stay that way. Reads `sources/SCF_plus.dta` in substance; the URL in the file is historical, see `sources/README.md` | +| `generating_mini.md` | `SCF_plus_mini.csv`, `SCF_plus_mini_no_weights.csv` | **committed-frozen** — its `to_csv` calls are commented out upstream and stay that way. As written it still fetches the `high_dim_data` URL; that URL is historical, and the input is now committed at `sources/SCF_plus.dta`. See `sources/README.md` | Both frozen builders keep their upstream `high_dim_data` filenames rather than being renamed to their set stems (`forbes`, `SCF_plus_mini`), which preserves