Seven PRs in the last day introduced a codegen kill switch and were red on codegen_env_vars_are_build_cache_inputs because the knob wasn't added to BUILD_CACHE_ENV_VARS:
| PR |
knob |
| #9060 |
PERRY_PACKED_LOOP_NUMERIC_ACCUMULATOR |
| #9071 |
PERRY_CALLEE_BINDING_RESOLUTION |
| #9105 |
PERRY_CALL_DEVIRT |
| #9122 |
PERRY_OBJECT_LITERAL_SHAPE_METHODS |
| #9124 |
PERRY_METHOD_INLINE_PROBE |
| #9149 |
PERRY_LOOP_PROPERTY_HOIST |
| #9154 |
PERRY_PACKED_LOOP_ABRUPT |
Every one was the same two-line fix, and in each case the author had done the harder part correctly — the knob itself, the kill-switch semantics, the tests. Seven identical misses by seven competent changes is a process gap, not seven oversights.
It also nearly escaped once: I merged #9122 with the registration prepared but not actually in the commit, which left main red until #9144. So the failure mode isn't only "PR is red", it's "PR is red in a way that's easy to wave through when you're fixing it yourself".
Why it's easy to miss
The knob lives in perry-codegen; the registry lives in crates/perry/src/commands/compile/build_cache.rs. Nothing in the codegen file points at the registry, and the connection (a knob that changes emitted code must key the build cache, or a cached object from one setting silently serves the other) is real but non-obvious at the point of writing.
Cheap fixes, roughly in order of effort
- Make the failure self-servicing. The assertion already names the missing vars; have it also print the exact file and the anchor line to add them to, so the fix needs no archaeology.
- A line in
CLAUDE.md next to the existing codegen guidance: "a new PERRY_* knob read in codegen must be added to BUILD_CACHE_ENV_VARS (or BUILD_CACHE_ENV_EXCLUSIONS with a reason)."
- A doc comment on the reader helper. Most of these knobs are read through a small
fn <name>_enabled() with a OnceLock; a note in whatever that pattern's canonical example is would reach people at the moment they copy it.
Not proposing to weaken the gate — it works, and it caught all seven. The point is that it's currently the only thing carrying this knowledge, and it only speaks after the fact.
Seven PRs in the last day introduced a codegen kill switch and were red on
codegen_env_vars_are_build_cache_inputsbecause the knob wasn't added toBUILD_CACHE_ENV_VARS:PERRY_PACKED_LOOP_NUMERIC_ACCUMULATORPERRY_CALLEE_BINDING_RESOLUTIONPERRY_CALL_DEVIRTPERRY_OBJECT_LITERAL_SHAPE_METHODSPERRY_METHOD_INLINE_PROBEPERRY_LOOP_PROPERTY_HOISTPERRY_PACKED_LOOP_ABRUPTEvery one was the same two-line fix, and in each case the author had done the harder part correctly — the knob itself, the kill-switch semantics, the tests. Seven identical misses by seven competent changes is a process gap, not seven oversights.
It also nearly escaped once: I merged #9122 with the registration prepared but not actually in the commit, which left
mainred until #9144. So the failure mode isn't only "PR is red", it's "PR is red in a way that's easy to wave through when you're fixing it yourself".Why it's easy to miss
The knob lives in
perry-codegen; the registry lives incrates/perry/src/commands/compile/build_cache.rs. Nothing in the codegen file points at the registry, and the connection (a knob that changes emitted code must key the build cache, or a cached object from one setting silently serves the other) is real but non-obvious at the point of writing.Cheap fixes, roughly in order of effort
CLAUDE.mdnext to the existing codegen guidance: "a newPERRY_*knob read in codegen must be added toBUILD_CACHE_ENV_VARS(orBUILD_CACHE_ENV_EXCLUSIONSwith a reason)."fn <name>_enabled()with aOnceLock; a note in whatever that pattern's canonical example is would reach people at the moment they copy it.Not proposing to weaken the gate — it works, and it caught all seven. The point is that it's currently the only thing carrying this knowledge, and it only speaks after the fact.