Skip to content

fix(cache): register concat switch and explain codegen inputs - #9748

Closed
proggeramlug wants to merge 2 commits into
mainfrom
fix/9158-codegen-cache-guidance
Closed

fix(cache): register concat switch and explain codegen inputs#9748
proggeramlug wants to merge 2 commits into
mainfrom
fix/9158-codegen-cache-guidance

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

New codegen switches repeatedly miss build-cache registration because the failing test names the constants but does not identify their source file. The failure now names crates/perry/src/commands/compile/build_cache.rs, both declaration anchors, why registration matters, and the exact test command to rerun. Contributor guidance and the OnceLock reader example carry the same requirement.

Running the gate found another omission on current main: PERRY_CONCAT_SITE_CACHE. It emits/removes per-site concatenation tables, so it now participates in the build-cache key. The existing missing-input and stale-exclusion checks remain intact. No version bump.

Validation:

  • cargo test --profile perry-dev -p perry --bin perry codegen_env_vars_are_build_cache_inputs passes.
  • All four tests in commands::compile::build_cache::tests pass.
  • Adding a temporary unregistered reader makes the compiled Perry test binary fail with its name, the registry path, both declaration anchors, and the rerun command; removing the probe restores the pass.
  • Changed-file Rust formatting, file-size, test-registration, and whitespace checks pass.

Fixes #9158.

Summary by CodeRabbit

  • Bug Fixes

    • Build caching now correctly accounts for the concatenation-table configuration, preventing stale generated output when this setting changes.
    • Improved diagnostics identify the required cache-registration updates and verification steps when new code-generation settings are added.
  • Documentation

    • Added contributor guidance explaining how environment-based code-generation settings must be registered for reliable cache invalidation.
    • Clarified which settings may be excluded and documented the associated validation checks.

@coderabbitai

coderabbitai Bot commented Sep 4, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: 515d58e4-1d42-4397-ab79-a66121cb0461

📥 Commits

Reviewing files that changed from the base of the PR and between 12efed1 and f05ee6c.

📒 Files selected for processing (4)
  • CLAUDE.md
  • changelog.d/9748-codegen-cache-guidance.md
  • crates/perry-codegen/src/codegen/helpers.rs
  • crates/perry/src/commands/compile/build_cache.rs

Included review availability: Your plan provides up to 8 included reviews per hour; 5 remain after this review.


📝 Walkthrough

Walkthrough

The build cache now includes PERRY_CONCAT_SITE_CACHE. Diagnostics and contributor documentation identify how new codegen environment variables must be registered or excluded with justification.

Changes

Codegen cache registration

Layer / File(s) Summary
Cache registry and diagnostics
crates/perry/src/commands/compile/build_cache.rs
Registers PERRY_CONCAT_SITE_CACHE as a build-cache input. The validation failure message identifies the registry locations and verification command.
Registration guidance
CLAUDE.md, crates/perry-codegen/src/codegen/helpers.rs, changelog.d/9748-codegen-cache-guidance.md
Documents registration requirements for new PERRY_* readers and justified exclusions from cache inputs.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to f05ee

Changing PERRY_CONCAT_SITE_CACHE now invalidates build-cache entries, preventing cached generated output from being reused across incompatible settings. The accompanying validation guidance and documentation are aligned, with no remaining merge-blocking risk identified.

Suggested reviewers: jdalton, thehypnoo

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main changes: registering the concatenation cache switch and documenting codegen cache inputs.
Description check ✅ Passed The description provides the summary, concrete changes, linked issue, validation commands, test results, and version-bump status. It does not reproduce the template headings or checklist, but the requ…
Linked Issues check ✅ Passed The PR satisfies the objectives in [#9158]. It improves the failure guidance, adds contributor and reader-pattern documentation, registers PERRY_CONCAT_SITE_CACHE, and preserves the validation checks.
Out of Scope Changes check ✅ Passed The changes remain within scope for [#9158]. The CLAUDE.md guidance, changelog entry, diagnostic update, and cache registration all relate to preventing missing codegen build-cache inputs.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (2 skipped: 2 …
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/9158-codegen-cache-guidance

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@proggeramlug

Copy link
Copy Markdown
Contributor Author

Landed on main via merge train #9798 (rebase-merged, so your commits keep their authorship). Thanks!

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.

codegen kill switches keep missing BUILD_CACHE_ENV_VARS — 7 PRs in one day

1 participant