Guardrail: hash a file whenever its manifest records a hash - #56
Merged
Conversation
`consumers: []` short-circuited the check before it reached the sha256 comparison, and this is the repo's only byte-integrity gate. Manifests land ahead of their repoints by convention, so a dataset arrives here with an empty consumer list and is flipped to `repointed` by a later PR in another repo — which made the one PR that introduces new bytes the one PR that never verifies them. `mpd2020.xlsx.yml` landed exactly that way in #38: hash recorded, consumers empty, never hashed until #41 filled them in. The hash check now keys on `integrity.sha256` being recorded rather than on `consumers` being non-empty. The consumer-specific errors are unchanged, so a live lecture whose file goes missing still says so in those terms. No behaviour change on `main`: all 18 manifests carry consumers and a hash today, so the run is identical bar the summary wording. Verified against a scratch tree for the four cases that matter — unconsumed + correct hash passes; unconsumed + drifted bytes fails; unconsumed + hash recorded but file absent fails; unconsumed with no hash stays out of scope. Lands before the `high_dim_data` fold, which introduces six datasets at `consumers: []` — QuantEcon/workspace-lectures#23 step 3, PR B. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Updates the required “consumed-files” guardrail so byte-integrity verification runs whenever a manifest records integrity.sha256, even if consumers: [], aligning CI with the repo convention that manifests land before repoints.
Changes:
- Keyed hashing/byte-match verification on presence of
integrity.sha256rather than non-emptyconsumers. - Added a dedicated missing-file error path for “hash recorded but file absent”.
- Updated guardrail docstring and end-of-run summary wording to reflect hash-check behavior.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…ashes Two fixes from Copilot's review, both confirmed by reproduction. A present-but-non-mapping `integrity` (e.g. `integrity: "sha256: …"`, one missing indent level) read as "no hash recorded", so a manifest with `consumers: []` short-circuited before the byte check and the run was silently green — 0 errors, 0 files hashed, exit 0. That is a hole this PR opened: with a consumer present the same manifest still errored, so it was only the new unconsumed path, which is exactly the shape every new dataset lands in. It is now a hard error either way, and the comment claiming a non-dict integrity "lands in the not recorded error below" goes with it, because after the keying change it did not. `checked` was also incremented before the existence and hash-recorded checks, so the summary could report a file as hash-checked when sha256() was never called. Accurate under the old "consumed file(s) checked" label; wrong under this PR's rename. The counter moves to where the hash is computed. Verified: the four cases in the PR body still behave, plus the two new ones — a string integrity with no consumers now exits 1, and a consumed manifest with no recorded hash reports 0 hash-checked rather than 1. The real tree is unchanged: 18 manifests, 18 hash-checked, exit 0. 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
Copilot review on #60, confirmed by reproduction — the builder checks had four holes, two of which crashed rather than reported. A non-string `builder_status` (a YAML list, say) raised `TypeError: unhashable type` on the set-membership test, and a non-string `builder` raised on `REPO / builder`. Both surfaced as a traceback with no `::error::` line — the same class of defect the review caught in #56, in a file that already handles it that way for `integrity` and for the manifest itself. The path check was satisfiable by things that are not builders in this repo. `REPO / builder` silently discards REPO when `builder` is absolute, so `/etc/hosts` passed; a relative path can climb out with `../`, so a real file outside the repo passed; and `.exists()` is true for directories, so naming `builders` passed. That last one the review did not name. Resolving and asserting `is_relative_to(REPO)` plus `is_file()` closes all three. Framing note: the read is correctness rather than security — the check only calls `.exists()`, never reads — but an assertion satisfiable by a file that is not a builder here establishes nothing, and the likely trigger is a typo'd relative path that happens to resolve on the runner, not an attack. Verified: real tree 18/18 exit 0; the six cases from the PR body unchanged; five new ones — list status, list builder, escaping relative path, absolute path, directory — all now fail cleanly with no traceback. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
* Give builders their own directory, one per published dataset scripts/ was two unrelated things sharing a name: seven builders that produce published datasets, and the dashboard/catalog toolchain that has nothing to do with data production. Its own README already apologised for the split with a "Not builders" section — while listing 1 of the 7. builders/<stem>.<ext> now builds lectures/<stem>.<ext2>. That is not a new convention: six of the seven already matched their output stem exactly, each with an explicit OUT_FILE = '<stem>.csv'. The move makes a latent pattern enforceable. Where one builder produces a SET of files, it is named for the set and several manifests point at the same path — business_cycle.py writes three, and both incoming notebook builders write two. So the stem rule is the default, not an invariant, and what CI asserts is weaker and truer: * builder_status must be a known value * a `committed*` status must name a builder * a named builder must exist on disk None of that was checked anywhere before — build_audit only records the value and build_catalog only formats it, so a manifest could assert a builder that was never committed, or one that had been moved. Which is exactly what this commit does to six of them. Adds `committed-frozen` to the enum: the builder is here and deliberately will not run, for a dataset built from a source that must not be refreshed. `committed` asserts a runnable four-stage builder and is a false claim for a frozen vintage; `unrecovered` says the builder is absent and is a false claim for one arriving in the same PR. This is the answer to #14's notebook question, and it lets the two high_dim_data builders land as what they are. Also records where a builder reads its input from, since sources/ is about to exist and is easy to misread: the normal case is the third-party upstream at run time — six of seven do that — and sources/ is only for an input that cannot be re-fetched. It is not a general input tree and not "the big-file directory"; the defining property is un-refetchability. Safe to do now, and cheaper now than later: no workflow runs any builder, nothing outside the repo references the paths, and PLAN's warning that the restructure window is spent applies to lectures/, where filenames are the public API — not to scripts/, which is never served. Doing it before the high_dim_data fold means its two builders land in the final shape. Verified: real tree 18/18 hash-checked exit 0 (which also proves the six moved paths resolve), strict audit exit 0, CATALOG.md regenerates byte-identical. Six negative cases exercised — missing builder path, a committed status with no builder, an unknown status, committed-frozen, and the verbatim/not-applicable shape that must keep passing. Part of #14. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Guardrail: type-check the builder fields, and require an in-repo file Copilot review on #60, confirmed by reproduction — the builder checks had four holes, two of which crashed rather than reported. A non-string `builder_status` (a YAML list, say) raised `TypeError: unhashable type` on the set-membership test, and a non-string `builder` raised on `REPO / builder`. Both surfaced as a traceback with no `::error::` line — the same class of defect the review caught in #56, in a file that already handles it that way for `integrity` and for the manifest itself. The path check was satisfiable by things that are not builders in this repo. `REPO / builder` silently discards REPO when `builder` is absolute, so `/etc/hosts` passed; a relative path can climb out with `../`, so a real file outside the repo passed; and `.exists()` is true for directories, so naming `builders` passed. That last one the review did not name. Resolving and asserting `is_relative_to(REPO)` plus `is_file()` closes all three. Framing note: the read is correctness rather than security — the check only calls `.exists()`, never reads — but an assertion satisfiable by a file that is not a builder here establishes nothing, and the likely trigger is a typo'd relative path that happens to resolve on the runner, not an attack. Verified: real tree 18/18 exit 0; the six cases from the PR body unchanged; five new ones — list status, list builder, escaping relative path, absolute path, directory — all now fail cleanly with no traceback. 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
* Give builders their own directory, one per published dataset scripts/ was two unrelated things sharing a name: seven builders that produce published datasets, and the dashboard/catalog toolchain that has nothing to do with data production. Its own README already apologised for the split with a "Not builders" section — while listing 1 of the 7. builders/<stem>.<ext> now builds lectures/<stem>.<ext2>. That is not a new convention: six of the seven already matched their output stem exactly, each with an explicit OUT_FILE = '<stem>.csv'. The move makes a latent pattern enforceable. Where one builder produces a SET of files, it is named for the set and several manifests point at the same path — business_cycle.py writes three, and both incoming notebook builders write two. So the stem rule is the default, not an invariant, and what CI asserts is weaker and truer: * builder_status must be a known value * a `committed*` status must name a builder * a named builder must exist on disk None of that was checked anywhere before — build_audit only records the value and build_catalog only formats it, so a manifest could assert a builder that was never committed, or one that had been moved. Which is exactly what this commit does to six of them. Adds `committed-frozen` to the enum: the builder is here and deliberately will not run, for a dataset built from a source that must not be refreshed. `committed` asserts a runnable four-stage builder and is a false claim for a frozen vintage; `unrecovered` says the builder is absent and is a false claim for one arriving in the same PR. This is the answer to #14's notebook question, and it lets the two high_dim_data builders land as what they are. Also records where a builder reads its input from, since sources/ is about to exist and is easy to misread: the normal case is the third-party upstream at run time — six of seven do that — and sources/ is only for an input that cannot be re-fetched. It is not a general input tree and not "the big-file directory"; the defining property is un-refetchability. Safe to do now, and cheaper now than later: no workflow runs any builder, nothing outside the repo references the paths, and PLAN's warning that the restructure window is spent applies to lectures/, where filenames are the public API — not to scripts/, which is never served. Doing it before the high_dim_data fold means its two builders land in the final shape. Verified: real tree 18/18 hash-checked exit 0 (which also proves the six moved paths resolve), strict audit exit 0, CATALOG.md regenerates byte-identical. Six negative cases exercised — missing builder path, a committed status with no builder, an unknown status, committed-frozen, and the verbatim/not-applicable shape that must keep passing. Part of #14. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Guardrail: type-check the builder fields, and require an in-repo file Copilot review on #60, confirmed by reproduction — the builder checks had four holes, two of which crashed rather than reported. A non-string `builder_status` (a YAML list, say) raised `TypeError: unhashable type` on the set-membership test, and a non-string `builder` raised on `REPO / builder`. Both surfaced as a traceback with no `::error::` line — the same class of defect the review caught in #56, in a file that already handles it that way for `integrity` and for the manifest itself. The path check was satisfiable by things that are not builders in this repo. `REPO / builder` silently discards REPO when `builder` is absolute, so `/etc/hosts` passed; a relative path can climb out with `../`, so a real file outside the repo passed; and `.exists()` is true for directories, so naming `builders` passed. That last one the review did not name. Resolving and asserting `is_relative_to(REPO)` plus `is_file()` closes all three. Framing note: the read is correctness rather than security — the check only calls `.exists()`, never reads — but an assertion satisfiable by a file that is not a builder here establishes nothing, and the likely trigger is a typo'd relative path that happens to resolve on the runner, not an attack. Verified: real tree 18/18 exit 0; the six cases from the PR body unchanged; five new ones — list status, list builder, escaping relative path, absolute path, directory — all now fail cleanly with no traceback. 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
* Give builders their own directory, one per published dataset scripts/ was two unrelated things sharing a name: seven builders that produce published datasets, and the dashboard/catalog toolchain that has nothing to do with data production. Its own README already apologised for the split with a "Not builders" section — while listing 1 of the 7. builders/<stem>.<ext> now builds lectures/<stem>.<ext2>. That is not a new convention: six of the seven already matched their output stem exactly, each with an explicit OUT_FILE = '<stem>.csv'. The move makes a latent pattern enforceable. Where one builder produces a SET of files, it is named for the set and several manifests point at the same path — business_cycle.py writes three, and both incoming notebook builders write two. So the stem rule is the default, not an invariant, and what CI asserts is weaker and truer: * builder_status must be a known value * a `committed*` status must name a builder * a named builder must exist on disk None of that was checked anywhere before — build_audit only records the value and build_catalog only formats it, so a manifest could assert a builder that was never committed, or one that had been moved. Which is exactly what this commit does to six of them. Adds `committed-frozen` to the enum: the builder is here and deliberately will not run, for a dataset built from a source that must not be refreshed. `committed` asserts a runnable four-stage builder and is a false claim for a frozen vintage; `unrecovered` says the builder is absent and is a false claim for one arriving in the same PR. This is the answer to #14's notebook question, and it lets the two high_dim_data builders land as what they are. Also records where a builder reads its input from, since sources/ is about to exist and is easy to misread: the normal case is the third-party upstream at run time — six of seven do that — and sources/ is only for an input that cannot be re-fetched. It is not a general input tree and not "the big-file directory"; the defining property is un-refetchability. Safe to do now, and cheaper now than later: no workflow runs any builder, nothing outside the repo references the paths, and PLAN's warning that the restructure window is spent applies to lectures/, where filenames are the public API — not to scripts/, which is never served. Doing it before the high_dim_data fold means its two builders land in the final shape. Verified: real tree 18/18 hash-checked exit 0 (which also proves the six moved paths resolve), strict audit exit 0, CATALOG.md regenerates byte-identical. Six negative cases exercised — missing builder path, a committed status with no builder, an unknown status, committed-frozen, and the verbatim/not-applicable shape that must keep passing. Part of #14. * Guardrail: type-check the builder fields, and require an in-repo file Copilot review on #60, confirmed by reproduction — the builder checks had four holes, two of which crashed rather than reported. A non-string `builder_status` (a YAML list, say) raised `TypeError: unhashable type` on the set-membership test, and a non-string `builder` raised on `REPO / builder`. Both surfaced as a traceback with no `::error::` line — the same class of defect the review caught in #56, in a file that already handles it that way for `integrity` and for the manifest itself. The path check was satisfiable by things that are not builders in this repo. `REPO / builder` silently discards REPO when `builder` is absolute, so `/etc/hosts` passed; a relative path can climb out with `../`, so a real file outside the repo passed; and `.exists()` is true for directories, so naming `builders` passed. That last one the review did not name. Resolving and asserting `is_relative_to(REPO)` plus `is_file()` closes all three. Framing note: the read is correctness rather than security — the check only calls `.exists()`, never reads — but an assertion satisfiable by a file that is not a builder here establishes nothing, and the likely trigger is a typo'd relative path that happens to resolve on the runner, not an attack. Verified: real tree 18/18 exit 0; the six cases from the PR body unchanged; five new ones — list status, list builder, escaping relative path, absolute path, directory — all now fail cleanly with no traceback. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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>
mmcky
added a commit
that referenced
this pull request
Aug 10, 2026
…s a URL resolver (#65) * Add PLAN-QELD-PACKAGE.md: the consumer-side data package, re-scoped Distils a design session into an operative plan. The package is a URL resolver, not a fetch-and-cache library: `qeld.url('mpd2020.xlsx')` returns a context-aware URL and the lecture reads it with pandas, in the open. What changed from the first design pass: - The version pins the key/URL contract, not bytes. CalVer retired for semver; `>=1,<2` in install cells, `==` in env files. The `qbn==1.6` precedent does not transfer: quantecon_book_networks ships its data inside the wheel, so its pin is immortal because nothing is fetched. qeld inverts that. - No fetch(), load(), cache, or runtime integrity. After the format convention is applied, `dataBHS.mat` is the only file in the endgame that cannot be read from a URL, which is what justifies dropping fetch(). - Context detection may change transport, never semantics: the CORS-correct URL form, and the emscripten shim that retires pyodide_http.patch_all(). - Integrity moves to CI. The committed-bytes leg already landed in #56; the remaining gap is a live serving-URL check, which belongs post-merge and on a schedule because a PR branch's bytes are never on the serving host. The call-site rule was chosen by measurement, not taste. Every static data read in six repos (115 sites, 40 lectures) was swept and clustered by idiom; the worklist lands as scripts/qeld_callsites.yml. Substituting in place of the URL expression the lecture already uses is the only form that wins on every idiom — inlining destroys reused variables across 47 sites, and always-two-step turns one line into two across 18 inline literals. The plan also records the honest case: 13 sites are structural, 70 are cosmetic shortenings of literals adopted in the recent repoint PRs. The package earns its place on those 13, the wasm shim, the host cutover, and the relative-path portability bugs — not on the read-site tally. Section 9 lists corrections to the original report so they are not re-proposed. Section 8 carries the ambiguities, each tagged with the phase that needs it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Move the qeld working artifacts to .dev/qeld/, and add the migration catalog `callsites.yml` says *where* to convert; it does not say *how*. The migration catalog carries the worked before/after per idiom and the head-to-head of the three candidate syntaxes, so it is the spec a sweep is checked against. Both are evidence rather than tooling, so they move out of scripts/ into .dev/qeld/ with a README stating what they are, that the plan wins on any disagreement, and that both are 2026-08-10 snapshots which must be re-verified before use — the lecture repos moved during the session that produced them. Nothing here is published: the Pages job assembles _site from site/, lectures/ and audit.json only. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Fix the repo field in the call-site worklist, and a manifest count in the plan The worklist's `repo` was normalised with a regex that truncated at the first `(` or `/`, so `QuantEcon/lecture-python-advanced.myst (...)` became `QuantEcon` and a path-leading string became the empty string -- 28 of 115 entries carried no repo at all. Regenerated from the raw sweep output with a normaliser that matches against the known repo set and fails loudly on anything unrecognised, so every entry is now `owner/repo`. That also explains the four apparent duplicate entries: heavy_tails.md exists in lecture-python-intro, lecture-wasm and lecture-intro.zh-cn, and the truncation collapsed the intro and wasm reads onto the same key. They are distinct call sites -- and specifically the intro/wasm pairs repoint rule 2 requires be converted together -- so they stay. The header now says so, since (lecture, filename) is not a unique key in this file. Counts are unchanged: 115 entries, per-repo totals matching the sweep, and the same idiom, action and win distributions. Separately, the plan said `sheets` is used in 8 manifests; it is 5. The 8 came from grepping for the string, which also matches prose in comments. `read_as` at 6 was right. 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.
Part of QuantEcon/workspace-lectures#23 step 3 — lands before PR B, which introduces six datasets at
consumers: [].check_consumed_files.pyshort-circuits on an emptyconsumerslist before it reaches the sha256 comparison, and it is this repo's only byte-integrity gate — the single required status check onmain.That interacts badly with a convention we rely on: manifests land ahead of their repoints. A dataset arrives here with
consumers: [], and a later PR in another repo is what makes the consumer list true. So the one PR that introduces new bytes has always been the one PR that never verifies them.mpd2020.xlsx.ymllanded exactly that way in #38 — hash recorded, consumers empty, not hash-checked until #41 filled the list in.The fix keys the hash check on
integrity.sha256being recorded rather than onconsumersbeing non-empty. Consumer-specific errors are untouched, so a live lecture whose file goes missing still fails in those terms.The alternative — populate
consumersin PR B — was rejected: it asserts a read that does not exist until PR set C merges, in a provenance record, in a repo whose policy elsewhere is honest nulls (retrieved: null,builder_status: unrecovered).manifest-schema.ymlexplicitly blessesconsumers: []as "nothing consumes it yet".Verification
No behaviour change on
main. All 18 manifests carry both consumers and a hash today, so the run is identical bar the summary wording (18 consumed file(s) checked→18 file(s) hash-checked), exit 0.Four cases exercised in a scratch tree:
consumers: [], hash matches the bytes — the PR B shapeconsumers: [], bytes drifted from the hashconsumers: [], hash recorded, file absentconsumers: [], no hash recordedNote for PR A
This composes with the
lfs: falsechange in the LFS-scoping PR. Withlfs: true, alectures/file accidentally LFS-tracked is checked out as real bytes, hashes correctly and passes green — whileraw.githubusercontent.comserves pointer text to every reader. Withlfs: falsethe checker hashes the pointer, mismatches, and goes red. Together they turn a published pointer from a silent false green into a failure.🤖 Generated with Claude Code