feat(core,cli): ship the DE parallel router fleet-wide — remove the canary gate - #3120
Conversation
…anary gate Deletes the `de-parallel-router` canary entry and the `isCanaryEnabled` guard in render.ts together, leaving the producer's default-ON in place. Net effect for users: the parallel drawElement router is on for everyone again. ## Why, and why not a ramp Gating at 5% was itself the regression. Measured 2026-08-08, the day after v0.7.101 shipped the canary: fleet router exposure fell from 3.13-4.25% of non-CI renders to **0.13%**, roughly 25x, because out-of-cohort installs are explicitly disarmed and #2840 deleted the everyone-armed trial in the same change. 2,537 installs lost a feature they already had. Severity is speed only, never output, and nothing is persisted to disk. PR #2840's body claimed "the canary does not make exposure smaller; it makes it chosen and revertible." That was true of the end state and false of the first step. This lands the end state. Entry and guard go together deliberately: at >=100 the evaluator short-circuits ahead of the CI/seedless exclusions, so removing only the entry would have flipped whatever still resolved false at deletion time, unstaged. ## Both stated blockers are void - **≤4-CPU / Docker coverage gap.** Docker renders never use drawElement — 0 of 4,281 across every CPU tier, software GL gates it out — and the router requires it. No percentage could ever expose Docker, so no ramp closes that gap. ≤4 CPUs yields ~42 drawElement candidates in three days. - **PRINFRA-372.** Its signature has hits on 0.4.12, 0.4.37, 0.6.52, 0.6.93, 0.6.109 and 0.6.110 — versions predating drawElement (v0.7.38) and therefore this router. It is real, still live on 0.7.101, and belongs to the screenshot/beginframe path. 11 reproduction runs across four configurations on the enriched profile (darwin/arm64 25.5.0) came back clean. ## Safety unchanged The per-install circuit breaker and the per-render self-verify are untouched; `HF_DE_PARALLEL_ROUTER=false` remains the user-facing kill switch. Post-canary data at 14 days: >8 CPUs 3.02% revert (177/5,857), 5-8 CPUs 2.40% (6/250) — consistent with the 2.75-3.16% baseline. Revert path is now a code revert rather than a registry edit. That is the trade this shape accepts in exchange for one release instead of two. ## Corrects two claims that shipped wrong `~17x jump in exposure onto <=4 CPUs / Docker` overstated the reach, and `~11% of installs already route` was an OUTCOME (the share clearing eligibility and the old 25-render cap), not an exposure setting — read as a rollout knob it inverts the arithmetic, which is how gating at 5% came to cut exposure rather than ramp it. Both are recorded in render.ts so they are not reintroduced. ## Tests Removed the core wiring assertion and the two CLI canary-gating tests, which pinned a gate that no longer exists. Added the inverse guarantee in its place: an ordinary install must come out of the breaker with the var UNSET so the producer default applies — writing "false" there is precisely what disarmed the fleet at 5%. core 1701 passing, cli 2491 passing, studio canary 29 passing. The 2 failures in play.test.ts reproduce on clean origin/main and are unrelated (#3114 area). oxlint and oxfmt clean. Note: telemetry for this rollout stops with the entry — `$feature/canary-de-parallel-router` and `canary_reason_de_parallel_router` are emitted from the registry, so the `Ramp —` tiles and the exposure-floor alert on PostHog dashboard 1918875 go blank once this ships. Watch drawElement engagement on 1807532 instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
efa5ac7 to
19defea
Compare
miga-heygen
left a comment
There was a problem hiding this comment.
Well-argued, well-scoped. The core claim — gating at 5% was itself the regression, cutting fleet exposure ~25x rather than ramping it — holds up against the code.
The removal is mechanically correct. Entry and guard go together (at >=100% the evaluator short-circuits ahead of CI/seedless exclusions, so deleting only the entry flips whatever still resolved false). The control flow post-change is:
- User-managed env var → return user's choice
- In-process breaker latch →
applyDeParallelRouterBreaker(), return false - On-disk breaker state → trip latch, apply breaker, return false
- return true — var stays unset, producer default-ON applies
The canary gate that used to sit between 3 and 4 is cleanly removed. The per-install breaker, per-render self-verify, and HF_DE_PARALLEL_ROUTER=false kill switch are untouched.
The test inversion is right. The old tests pinned "canary not enrolled → var is false". The new test pins the inverse: ordinary install → var is undefined. This is exactly the guarantee that matters now — writing "false" here is what the 5% gate was doing to the fleet.
Both stated blockers are void. Docker never uses drawElement (0/4,281 measured), so no percentage could have exposed it. PRINFRA-372 predates drawElement entirely. Good that these are recorded in the source so they're not reintroduced.
Stale docstring examples (non-blocking): The PR updated the main API docstring example (canary.ts line 12) to "your-feature", but four more in the same file + canaryRegistry.ts still use "de-parallel-router" as the illustrative example:
canary.ts:58—CanaryInput.featureJSDoccanary.ts:173—canaryFeatureKeyJSDoccanary.ts:185—canaryReasonKeyJSDoccanaryRegistry.ts:89—canaryEnvVarJSDoc
Same story for test fixtures in telemetryIdentity.test.ts and several canary.test.ts assertions — they use the removed name as example data. None of these affect correctness (the functions work on arbitrary strings), but grepping for the name after this ships returns ~15 hits that look like the gate is still alive. Worth a find-and-replace pass to a living canary name (e.g. calibration-10) or a generic placeholder.
The render.ts tombstone comment is justified — it prevents reintroduction of the two wrong claims. 17 lines is on the long side for inline comments but the content earns its keep.
LGTM — ship it.
Review by Miga
jrusso1020
left a comment
There was a problem hiding this comment.
Approving at 19defeab. Every check run on this head is success or skipped — none failing, none pending — and autoMergeRequest is null, so the merge stays your call.
miga-heygen covered the mechanics: entry + guard had to go together, the safety surfaces are untouched, the test inversion pins the right guarantee, and the residual "de-parallel-router" strings are cosmetic. Not repeating those. Four deltas.
1. I reproduced the exposure claim independently, and the version-controlled cut is stronger than the one in the body.
Splitting render_complete by cli_version for 2026-08-08 — non-CI only, share of renders with de_parallel_router = "routed":
| version | non-CI renders | routed | share |
|---|---|---|---|
| 0.7.101 | 22,997 | 30 | 0.130% |
| 0.7.100 | 3,821 | 118 | 3.088% |
| 0.7.98 | 2,454 | 78 | 3.178% |
| 0.7.94 | 3,803 | 106 | 2.787% |
Same day, same fleet, one release apart: 23.8x against 0.7.100. Your 0.13% is exact, and the arithmetic closes — 3.088% × 5% predicts 0.154%, observed 0.130%, with the gap being the exclusions stacking on top of the percentage (point 3).
Worth adding to the rollout note: the fleet aggregate for 08-08 reads 0.947%, not 0.13%, because most installs had not upgraded yet. Anyone watching a fleet-level tile saw a soft dip, not a 25x cut, and it would have recovered on its own as adoption moved — which is a good reason this was easy to miss for a day. The per-version cut is the one that shows it.
2. The canary was also disarming every telemetry opt-out — which render.ts explicitly says must not happen.
decideCanary resolves telemetry_opt_out before evaluateCanary ever runs (packages/cli/src/telemetry/canary.ts:86-88), so an opted-out install returned false from isCanaryEnabled and fell into the old guard's applyDeParallelRouterBreaker() → an explicit HF_DE_PARALLEL_ROUTER=false.
Meanwhile render.ts:1132-1137 states the opposite principle in so many words: gating the router on telemetry "would mean users who opted out of analytics silently get a slower renderer — punishing a privacy choice with a performance penalty," concluding that telemetry state "governs REPORTING, never behavior."
The 5% gate reintroduced exactly that, in the same file that rejects it. That is a stronger argument for this PR than the exposure number, and it is not in the body.
3. Consequence for the "should have ramped to 100 first" objection.
The pct >= 100 short-circuit (packages/core/src/canary.ts:119-127) is documented as the mechanism that exercises CI and seedless installs before deletion, so nothing takes the new path unstaged. It cannot do that for telemetry opt-outs — the opt-out check sits upstream of evaluateCanary entirely, so no percentage, including 100, ever reaches them.
So ramp-to-100-and-hold would have de-risked two of the three populations that flip at merge, not all three. Going straight to deletion forfeits less than that comment implies. Worth recording next to it, since the comment currently reads as an unconditional guarantee.
4. What the 14-day revert data does not cover.
By construction the enrolled cohort excludes CI (excluded), seedless installs (no_unit_id), and telemetry opt-outs. Those three are precisely the populations whose behaviour changes at merge, so the 3.02% / 2.40% figures cannot speak to them — and two of the three never report at all.
Non-blocking, and I want to be clear why: severity is speed-only, the per-render self-verify and the per-install breaker are both untouched, and this restores the pre-#2840 state rather than creating exposure that never existed. The ask is only that the revert data not be read as covering the delta this PR actually ships.
One correction to your telemetry note, in your favour. "Telemetry for this rollout stops with the entry" undersells what survives. de_parallel_router is a first-class property on render_complete (packages/cli/src/telemetry/events.ts:140 and :320), emitted independently of the registry. Last 7 days, non-CI: 6,830 routed / 214 reverted — a 3.04% revert rate, consistent with the 2.75–3.16% baseline you quote.
So the safety metric is intact and a post-merge regression stays visible. What dies is the cohort split, which is meaningless at 100% anyway. The real blind spot is the opt-out population: newly exposed by this change, and reporting nothing by definition.
Review by Rames Jusso
Deletes the
de-parallel-routercanary entry and theisCanaryEnabledguardin render.ts together, leaving the producer's default-ON in place. Net effect
for users: the parallel drawElement router is on for everyone again.
Why, and why not a ramp
Gating at 5% was itself the regression. Measured 2026-08-08, the day after
v0.7.101 shipped the canary: fleet router exposure fell from 3.13-4.25% of
non-CI renders to 0.13%, roughly 25x, because out-of-cohort installs are
explicitly disarmed and #2840 deleted the everyone-armed trial in the same
change. 2,537 installs lost a feature they already had. Severity is speed
only, never output, and nothing is persisted to disk.
PR #2840's body claimed "the canary does not make exposure smaller; it makes
it chosen and revertible." That was true of the end state and false of the
first step. This lands the end state.
Entry and guard go together deliberately: at >=100 the evaluator
short-circuits ahead of the CI/seedless exclusions, so removing only the entry
would have flipped whatever still resolved false at deletion time, unstaged.
Both stated blockers are void
of 4,281 across every CPU tier, software GL gates it out — and the router
requires it. No percentage could ever expose Docker, so no ramp closes that
gap. ≤4 CPUs yields ~42 drawElement candidates in three days.
0.6.109 and 0.6.110 — versions predating drawElement (v0.7.38) and therefore
this router. It is real, still live on 0.7.101, and belongs to the
screenshot/beginframe path. 11 reproduction runs across four configurations
on the enriched profile (darwin/arm64 25.5.0) came back clean.
Safety unchanged
The per-install circuit breaker and the per-render self-verify are untouched;
HF_DE_PARALLEL_ROUTER=falseremains the user-facing kill switch. Post-canarydata at 14 days: >8 CPUs 3.02% revert (177/5,857), 5-8 CPUs 2.40% (6/250) —
consistent with the 2.75-3.16% baseline.
Revert path is now a code revert rather than a registry edit. That is the
trade this shape accepts in exchange for one release instead of two.
Corrects two claims that shipped wrong
~17x jump in exposure onto <=4 CPUs / Dockeroverstated the reach, and~11% of installs already routewas an OUTCOME (the share clearingeligibility and the old 25-render cap), not an exposure setting — read as a
rollout knob it inverts the arithmetic, which is how gating at 5% came to cut
exposure rather than ramp it. Both are recorded in render.ts so they are not
reintroduced.
Tests
Removed the core wiring assertion and the two CLI canary-gating tests, which
pinned a gate that no longer exists. Added the inverse guarantee in its place:
an ordinary install must come out of the breaker with the var UNSET so the
producer default applies — writing "false" there is precisely what disarmed
the fleet at 5%.
core 1701 passing, cli 2491 passing, studio canary 29 passing. The 2 failures
in play.test.ts reproduce on clean origin/main and are unrelated (#3114 area).
oxlint and oxfmt clean.
Note: telemetry for this rollout stops with the entry —
$feature/canary-de-parallel-routerand
canary_reason_de_parallel_routerare emitted from the registry, so theRamp —tiles and the exposure-floor alert on PostHog dashboard 1918875 goblank once this ships. Watch drawElement engagement on 1807532 instead.