Add the consumed-file check — the go-live CI guardrail#16
Merged
Conversation
The narrowest possible test that a PR cannot break a live lecture (PLAN Phase 5, go-live guardrails; prerequisite for the first repoint). For every manifest sidecar in lectures/: - the manifest's `filename` must match the sidecar's own name (the pairing Phase 2 promised CI would assert) - if `consumers` is non-empty — a lecture reads the file in production — the data file must exist, integrity.sha256 must be recorded, and the committed bytes must hash to it. The mismatch message points at the corrections-vs-vintages rule so a deliberate in-place fix knows to update the hash in the same PR. Files with no manifest yet (Phase 6 backfill) or empty consumers are out of scope; the full Phase 5 validation suite subsumes this job when it lands. Runs on every PR and on pushes to main. Checkout uses lfs: true proactively: nothing is LFS-tracked today (Phase 3), but if a consumed file ever is, the job must hash bytes, not pointer text. Verified locally: passes on the clean tree (1 consumed file checked), and fails correctly on each of the four failure modes — tampered bytes, deleted data file, sidecar/filename mismatch, and a consumed file with no recorded hash. Ticks the PLAN go-live checklist item in the same PR, per convention. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Introduces the minimal “consumed-file” CI guardrail (PLAN Phase 5) to prevent PRs from breaking datasets that are already referenced by live lecture consumers, and marks the checklist item as completed in PLAN.md.
Changes:
- Add a new GitHub Actions workflow that runs on PRs and on pushes to
mainto validate consumed datasets against their manifest sidecars. - Add a Python checker script that verifies sidecar/filename alignment and validates
integrity.sha256+ on-disk bytes for manifests with non-emptyconsumers. - Update
PLAN.mdto mark the guardrail as implemented and reference the workflow file.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| PLAN.md | Marks the minimal consumed-file CI guardrail as completed and links to the workflow. |
| .github/workflows/consumed-file-check.yml | Adds the workflow wiring (checkout with lfs: true, setup Python, run checker). |
| .github/scripts/check_consumed_files.py | Implements the consumed-file guardrail checks (sidecar pairing + sha256 verification). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Both crashes would still have failed CI (non-zero exit via the traceback), so the guardrail was never bypassable — but a guardrail should diagnose, not stack-trace, and should keep scanning past one bad file: - Empty or invalid YAML: caught per file (yaml.YAMLError, or a non-mapping parse result) and reported as ::error:: with the filename; the remaining manifests still get checked. - Non-dict `integrity` (e.g. a stray string): treated as missing, so a consumed file lands in the existing "sha256 not recorded" error instead of raising AttributeError. Re-verified all five original scenarios plus three new ones: empty sidecar, unparseable YAML, and integrity-as-string — each fails cleanly with the right message and exit 1, no tracebacks. Co-Authored-By: Claude Opus 4.8 (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.
The go-live CI guardrail from PLAN Phase 5 — the second of the two prerequisites (with branch protection, added 2026-07-17) that the amended ordering note requires before the first repoint (QuantEcon/lecture-python-intro#792) makes
raw/maina production URL. This is the repo's first CI and its first PR under the new protect-main ruleset.What it checks
For every manifest sidecar
lectures/<datafile>.yml:filenamematches the sidecar's own nameintegrity.sha256recorded"Consumed" means the manifest's
consumerslist is non-empty. Files with no manifest yet (Phase 6 backfill pending) or emptyconsumersare out of scope — the full Phase 5 validation suite (schema, dtypes, invariant tests) subsumes this job when it lands.The hash-mismatch message tells the author what a legitimate change looks like: a deliberate in-place correction updates
integrity.sha256in the same PR and plans rebuilds for the consumers (AGENTS.md, "Corrections vs vintages").Verified
Ran locally against all five scenarios: passes on the clean tree (1 manifest, 1 consumed file checked), and fails with the right error on each failure mode — tampered bytes, deleted data file, sidecar/filename mismatch, and a consumed file with its hash nulled. The workflow run on this PR is the live proof.
Checkout uses
lfs: trueproactively: nothing is LFS-tracked today (Phase 3), but if a consumed file ever is, this job must hash the bytes, not the pointer.Ticks the PLAN go-live checklist item in the same PR, per convention.
After merge
Once this job has run on
main, it can be added to the protect-main ruleset as a required status check — happy to do that as a follow-up so a red check physically blocks merging.Part of #8
See QuantEcon/meta#338