LFS: scope it to sources/**, and stop fetching it in CI - #57
Merged
Conversation
Infra only — no data moves here. This is the groundwork the high_dim_data fold needs, landed on its own so the fold PR is a data diff. `.gitattributes` did not exist, so `sources/` had no rule waiting for it and `lectures/` had no rule keeping it plain. LFS is now scoped to `sources/**`, with `sources/README.md` excluded so the audit trail stays readable text. Never a blanket rule like high_dim_data's `*.csv` + `*.dta` — that is what puts all of its consumers on the media host. Both workflow checkouts go `lfs: true` -> `lfs: false`, and the comments explaining them were inverted. `lfs: false` is the assertion, not a saving: a manifest records the sha256 of the real bytes, so an accidentally LFS-tracked file under `lectures/` gets hashed as its pointer, mismatches, and goes red — the same bytes a reader would get from raw.githubusercontent.com. With `lfs: true` the checker fetches the real bytes, hashes them correctly, passes green, and Pages publishes a file that resolves only from Pages. It also keeps a 99 MiB object out of every PR run once `sources/` is populated. Verified: `git check-attr filter` prints `lfs` for sources/SCF_plus.dta and `unspecified` for sources/README.md and for lectures/ paths; both workflows still parse and now report lfs=False. AGENTS.md's LFS bullet described the pre-change state and prescribed this change, so it is rewritten here rather than left to go stale, plus a new line for the trap this opens: SCF_plus.dta is 923,507 B under GitHub's hard blob limit, so a mis-scoped rule does not error — the push just silently succeeds as plain git. Check `git check-attr` before adding to sources/. PR A of QuantEcon/workspace-lectures#23 step 3. Part of #1. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR tightens Git LFS handling so that only sources/** can be LFS-tracked, and CI explicitly does not fetch LFS objects—ensuring that any accidental LFS pointer in the published lectures/ tree is surfaced as a hard failure rather than a silent success.
Changes:
- Add a new
.gitattributesthat scopes LFS tosources/**while keepingsources/README.mdas normal text. - Switch both GitHub Actions checkouts (
audit-dashboardandconsumed-file-check) fromlfs: truetolfs: falsewith updated rationale comments. - Update
AGENTS.mdto document thelfs: falsepolicy and the pre-git addgit check-attrtrap check.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
AGENTS.md |
Updates repository guidance to reflect the lfs: false CI policy and adds a git check-attr guardrail. |
.github/workflows/consumed-file-check.yml |
Sets checkout to lfs: false so pointer bytes are hashed (and fail) if a published file is accidentally LFS-tracked. |
.github/workflows/audit-dashboard.yml |
Sets checkout to lfs: false so Pages deploys exactly what readers would get (including pointer bytes if misconfigured), making mistakes visible. |
.gitattributes |
Introduces per-path LFS scoping to sources/** while excluding sources/README.md from LFS. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
"Per-path LFS for large binaries only; storage choice invisible to consumers because URLs decouple from hosting" is the position repoint rule 6 overturned on 2026-08-06, still sitting in the plan's own summary section — contradicted by rule 6 itself, by the settled-decision note further down, and by AGENTS.md. It is also the sentence a future session would cite to put an LFS object back into lectures/, which is the one thing this PR exists to prevent. Measured: `https://github.com/<org>/<repo>/raw/<ref>/<path>` — the form AGENTS.md calls storage-agnostic — is a 302 whose Location is raw.githubusercontent.com for a plain-git path and media.githubusercontent.com for an LFS path, and whose access-control-allow-origin header is present but EMPTY. So it is storage-agnostic only because the server resolves storage on the author's behalf, and it is precisely the form a browser rejects. For a wasm consumer there is no URL form invariant under a storage flip. Reasoning and the >100 MiB ladder recorded in #58. 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
) * Docs: the storage position, the URL rule, and the refs that drifted #57 deleted the "storage choice invisible to consumers" line from PLAN, but the same position survived in two more places, and one of them was telling authors to do the opposite of what CI enforces. README.md said the interim URL form "works for both plain-git and LFS-tracked files" and told readers to AVOID raw.githubusercontent.com — which is the only form lecture-wasm can fetch, and the form the strict audit demands there. Replaced with the runtime-keyed rule: CPython gets the github.com/*/raw/ form, browser consumers get raw.githubusercontent, and the media host is never valid for this repo. AGENTS.md's URL section had the same "works regardless of storage" claim and never mentioned repoint rule 5 or CORS at all — so an agent working from the conventions doc alone would write the CORS-broken form into a wasm repoint, which is exactly the incident data-lectures#46 records. Also adds the ladder for a published file approaching 100 MiB, which the repo had no answer for: subset, plain git, gzip in place, split, publish from outside git. Reasoning and measurements in #58. PLAN corrections, all verified against main today: * rule coverage — CI now checks rules 5, 6 and resolvability, not "only a corner" * the browser row of the URL table still offered the media host for LFS files; #55 made that a hard failure * the .gitattributes and lfs:false boxes tick, with the line refs moved (audit-dashboard.yml:51, consumed-file-check.yml:26) and the LFS quota restated as measured-and-not-binding, keeping the mechanism * the consumed-file check is rekeyed off consumers (#56) * generating_mini.md: do NOT uncomment its to_csv writes until #14 settles runnable-builder vs provenance — PLAN was instructing one branch of an open decision. The input-URL repoint stands either way * data.quantecon.org is NXDOMAIN at its own authoritative nameserver and the Pages cname is null, so Track Y has no external blocker left — but classify_url knows six GitHub hosts and neither canonical one, so `final` is a status the audit must report as broken until the classifier learns it * build_audit.py:45-46 -> :46-47 and :45-57 -> :46-57; wasm mirror-orphans "a dozen" -> five, matching Track X graph.txt is a 100-node digraph, not the 15-node toy audit_annotations described — that string renders into the public dashboard. Strict audit exit 0, consumed-file check 18/18. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * AGENTS: spell out access-control-allow-origin in the URL table The table used the full header name in one row and abbreviated it to ACAO in the next, and the abbreviation appears nowhere else in the repo's docs. 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 10, 2026
…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.
mmcky
added a commit
that referenced
this pull request
Aug 10, 2026
…es (PR B2) (#63) * Land SCF_plus.dta in sources/, and gate sources/ on its recorded hashes (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> * consumed-file-check: the lfs:false comment now says why sources/ wants 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. * Harden the sources/ gate against Copilot's five findings 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> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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 A of QuantEcon/workspace-lectures#23 step 3 — infra only, no data moves. Part of #1.
What changes
.gitattributes(new). The repo had none, sosources/had no rule waiting for it andlectures/had nothing keeping it plain. LFS is now scoped tosources/**, withsources/README.mdexcluded so the audit trail stays readable text. Explicitly not a blanket rule likehigh_dim_data's*.csv+*.dta— that rule is the reason all 21 of its consuming reads sit on the media host today.Both workflow checkouts:
lfs: true→lfs: false. The comments justifyinglfs: truehad the reasoning backwards, so they are rewritten too, not just flipped.Why
lfs: falseis a correctness fix, not a savingA manifest records the sha256 of the real bytes. So if a
lectures/file is ever LFS-tracked by mistake:lfs: trueraw.githubusercontent.com, HTTP 200lfs: falseSame for the dashboard job, which publishes
lectures/to Pages: withlfs: trueit would deploy a file that reads correctly from Pages and as pointer text everywhere else.lfs: falsemakes the two hosts agree, including when they are both wrong.This composes with #56 (hash whenever a hash is recorded): together they turn a published pointer from a silent false green into a failure.
The bandwidth argument still holds as a secondary reason —
sources/SCF_plus.dtais 99.1 MiB andconsumed-file-checkruns on every PR — but it is no longer the case for the change.On the org LFS quota
#23 lists "confirm the org's LFS bandwidth plan before the first
git lfs push" as a blocker. It is measurable read-only, and it does not gate this PR:gh api /organizations/QuantEcon/settings/billing/usagereturns per-repogit_lfsrows, only four repos in the org use LFS, every month of 2026 has a net charge of $0 against the included allowance, andhigh_dim_data's LFS bandwidth is 0 GB for June, July and August-to-date.Correcting myself on the rest of it, though: the outage mechanism behind that blocker is real and documented, and an earlier draft of this PR under-stated it. Anonymous public LFS downloads are billed to the repository owner, with no open-source exemption; forks and pulls count against the parent; and under a $0 budget GitHub blocks LFS usage for the remainder of the calendar month rather than billing it. On the Team allowance of 250 GiB/month, roughly 3,536 reads of one SCF-sized file would exhaust it, after which the next reader gets a hard failure — a reader-facing outage triggered by popularity. What is absent is evidence that we are near that ceiling, not evidence that the ceiling exists. (Pre-paid data packs no longer exist; LFS is metered.) Recorded in #58.
Verification
git check-attr filter diff merge text:sources/SCF_plus.dta→filter: lfs,diff: lfs,merge: lfs,text: unsetsources/README.md→ allunspecified,text: setlectures/employ.csv,lectures/SCF_plus_mini.csv→ allunspecified(plain git, as intended)Both workflows parse and report
lfs=False.One trap this opens, recorded in AGENTS.md
SCF_plus.dtais 103,934,093 B against GitHub's 104,857,600 B hard blob limit — 923,507 B, 0.88%, of headroom. A mis-scoped rule therefore does not error: the push succeeds as plain git and the blob is in history permanently.git check-attr filter -- sources/<file>must printfilter: lfsbefore thegit add.🤖 Generated with Claude Code