Skip to content

Clean up cache actions: honest restore key + caching docs (#34) - #41

Merged
mmcky merged 2 commits into
mainfrom
fix/cache-action-cleanup
Jun 16, 2026
Merged

mmcky merged 2 commits into
mainfrom
fix/cache-action-cleanup

Conversation

@mmcky

@mmcky mmcky commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Addresses all four items in #34 — correctness + documentation cleanup in the cache actions. No behavioural change to caching; one dead code path removed and the caching design documented.

Changes

H4 — honest restore key (code)

restore-jupyter-cache read-only mode set the primary key to …-00000000, which can never match a saved key (the saver always suffixes with ${github.run_id}). It silently fell through to restore-keys prefix matching. Now it uses the content/env prefix as the key directly:

  • exec: jupyter-cache-{hash}-
  • build: build-{env}-{update}-

Same restore result (prefix match), but the logged "Requested Key" is now truthful and the dead exact-match path is gone.

H5 — README no longer claims strictly read-only (docs)

Documented the optional save-cache input (added in 0.5.2) in the Design Philosophy section and the inputs table — PR-scoped saving that GitHub Actions confines to the PR branch.

L21 — path constraint documented (docs)

path must match where build-jupyter-cache saves (_build). Documented the constraint in the input description and the README table rather than leaving a knob that silently restores to the wrong location. (No caller passes path: today; removing it outright would be a cleaner but breaking follow-up.)

H6 — build-cache key design documented (docs)

Per the decision recorded in #34, documented that the build-cache key is intentionally environment-only: _build is a warm-start baseline, and freshness is handled by jupyter-cache (per-notebook, content-addressed), Sphinx incremental rebuilds, and the weekly cold rebuild. A content hash would miss on nearly every PR and force a cold rebuild for no correctness gain. Added to both cache action READMEs and the caching section of docs/ARCHITECTURE.md.

Verification

  • restore-jupyter-cache/action.yml validated as YAML; no -00000000 remains.
  • Restore behaviour is unchanged (prefix matching was already the only working path).

Closes #34

🤖 Generated with Claude Code

- H4: restore-jupyter-cache read-only mode used a fake `-00000000` primary
  key that can never match a saved key (those carry a -{run_id} suffix), so
  it silently relied on restore-keys prefix matching. Use the content/env
  prefix as the key directly — same restore behaviour, honest "Requested Key".
- H5: README claimed the action is strictly read-only; document the optional
  `save-cache` input (PR-scoped saving) in the Design Philosophy and inputs
  table.
- L21: document the `path` constraint (must match build-jupyter-cache's
  `_build`) in the input description and README, rather than leaving a knob
  that silently restores to the wrong place.
- H6: document that the build-cache key is intentionally environment-only
  (warm-start baseline; freshness handled by jupyter-cache + Sphinx
  incremental + the weekly cold rebuild) in both cache READMEs and
  docs/ARCHITECTURE.md.

Closes #34

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 16, 2026 02:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refines the cache actions’ restore-key behavior and updates documentation to better reflect the intended caching design (prefix-based restores; build cache keyed by environment only).

Changes:

  • Removed the non-functional “exact match” restore key in read-only mode by using the appropriate key prefixes directly.
  • Updated restore-jupyter-cache documentation to describe save-cache, path constraints, and the environment-only build-cache key rationale.
  • Added architecture/docs rationale explaining why _build cache is intentionally environment-keyed (not lecture-content-keyed).

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
restore-jupyter-cache/README.md Updates design philosophy, inputs table, and documents build-key rationale.
restore-jupyter-cache/action.yml Makes read-only restore key “honest” by using the prefix key directly; clarifies path description.
docs/ARCHITECTURE.md Documents the environment-only build-cache key rationale at the architecture level.
CHANGELOG.md Records the restore-key correction and documentation updates under Unreleased.
build-jupyter-cache/README.md Adds a short explanation for why the build-cache key omits lecture-content hashing.

Comment thread restore-jupyter-cache/action.yml
Comment thread restore-jupyter-cache/README.md
Comment thread restore-jupyter-cache/README.md
- path input (action.yml + README): reworded to describe the real failure
  mode. Cache *keys* match regardless of restore path; overriding `path`
  restores the cached _build into a different directory, so build-lectures
  (which reads _build) ignores the restored state. Previous wording wrongly
  implied the cache "won't match".
- QUICK-REFERENCE.md: completes H5 — the summary table no longer calls the
  action "read-only", and the Fast PR Builds section documents save-cache.
  (failure-artifact-name gap stays with the #40 docs sweep.)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@mmcky
mmcky merged commit 16e86d7 into main Jun 16, 2026
@mmcky
mmcky deleted the fix/cache-action-cleanup branch June 16, 2026 02:39
mmcky added a commit that referenced this pull request Jun 16, 2026
Roll up the work landed since v0.7.0 (#41#81): conda-cache fix, sibling
@v0 pins, release/checkout-SHA guards, build-lectures eval removal,
container Miniconda pin + node-LTS cap, preview de-dup + script-injection
fix, SHA-pinned third-party actions, Dependabot config, and the docs sweep.

- CHANGELOG: cut [Unreleased] → [0.8.0]; refresh the Version History line.
- README / copilot-instructions: bump the v0.7.0 mentions to v0.8.0.

The floating v0 tag will be moved to this release after merge.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cache actions: key correctness, stale read-only README, and H6 build-key triage

2 participants