Skip to content

Commit 19defea

Browse files
vanceingallsclaude
andcommitted
feat(core,cli): ship the DE parallel router fleet-wide — remove the canary 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>
1 parent b1f7d88 commit 19defea

5 files changed

Lines changed: 31 additions & 107 deletions

File tree

packages/cli/src/commands/render.test.ts

Lines changed: 8 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,6 @@ const configState = vi.hoisted(
4242
);
4343

4444
const trackingState = vi.hoisted(() => ({
45-
// The rollout slice. Default-ON is gated on canary enrolment, so these
46-
// tests control it directly rather than depending on where the test
47-
// machine's bucketSeed happens to land.
48-
canaryEnabled: true,
4945
// maybeEnableDeParallelRouterTrial gates on the real shouldTrack(), which
5046
// (via isDevMode()) always returns false when this file itself runs as
5147
// `.ts` source under vitest — mocked here so the CLI-trial tests can
@@ -176,10 +172,6 @@ vi.mock("../telemetry/client.js", () => ({
176172
shouldTrack: vi.fn(() => trackingState.shouldTrack),
177173
}));
178174

179-
vi.mock("../telemetry/canary.js", () => ({
180-
isCanaryEnabled: vi.fn(() => trackingState.canaryEnabled),
181-
}));
182-
183175
vi.mock("../telemetry/events.js", () => ({
184176
trackRenderComplete: vi.fn(),
185177
trackRenderError: vi.fn(),
@@ -246,7 +238,6 @@ describe("renderLocal browser GPU config", () => {
246238
configState.failMirrors = 0;
247239
configState.writeConfigCalls = [];
248240
trackingState.shouldTrack = true;
249-
trackingState.canaryEnabled = true;
250241
trackingState.renderObservations = [];
251242
ffmpegEncoderState.mode = "software";
252243
ffmpegEncoderState.error = null;
@@ -749,7 +740,6 @@ describe("renderLocal — DE parallel-router circuit breaker", () => {
749740
configState.failWrites = 0;
750741
configState.writeConfigCalls = [];
751742
trackingState.shouldTrack = true;
752-
trackingState.canaryEnabled = true;
753743
// The "managed by us" flag lives at module scope in render.ts (real CLI
754744
// processes only ever run one --batch sequence, so it never needs
755745
// resetting there) — reset explicitly here so tests don't leak arm/
@@ -787,45 +777,26 @@ describe("renderLocal — DE parallel-router circuit breaker", () => {
787777
manageDeParallelRouterBreaker: true,
788778
};
789779

790-
// The rollout slice. Default-ON means every eligible render routes the
791-
// moment this ships — a ~17x exposure jump. The canary is what makes that
792-
// fraction chosen and revertible instead of emergent.
793-
it("disarms for an install the canary did not enrol", async () => {
794-
trackingState.canaryEnabled = false;
780+
// The canary that used to gate this is gone (registry entry + guard removed
781+
// together). The router is now a shipped default for every install, so the
782+
// guarantee worth pinning is the inverse of the old one: an ordinary install
783+
// must come out of the breaker with the var UNSET, so the producer's
784+
// default-ON applies. Writing "false" here would silently disarm the fleet —
785+
// that is exactly what gating at 5% did.
786+
it("leaves the var unset for an ordinary install so the producer default applies", async () => {
795787
configState.disk = {
796788
telemetryEnabled: true,
797789
deParallelRouterTrialFired: false,
798790
telemetryNoticeShown: true,
799791
};
800-
await renderLocal("/tmp/project", "/tmp/out.mp4", baseOptions);
801-
// Explicit "false", not delete: with default-ON polarity, deleting the
802-
// var means ON — the same trap the breaker fix exists for.
803-
expect(process.env.HF_DE_PARALLEL_ROUTER).toBe("false");
804-
});
805-
806-
// Setting the registry percentage to 0 must switch the router off fleet-wide
807-
// without a release. That is the revert path, so it has to be pinned.
808-
it("registry percentage is a full kill switch", async () => {
809-
configState.disk = {
810-
telemetryEnabled: true,
811-
deParallelRouterTrialFired: false,
812-
telemetryNoticeShown: true,
813-
};
814-
815-
trackingState.canaryEnabled = false;
816-
await renderLocal("/tmp/project", "/tmp/out.mp4", baseOptions);
817-
expect(process.env.HF_DE_PARALLEL_ROUTER).toBe("false");
818-
819792
delete process.env.HF_DE_PARALLEL_ROUTER;
820-
trackingState.canaryEnabled = true;
821793
await renderLocal("/tmp/project", "/tmp/out.mp4", baseOptions);
822794
expect(process.env.HF_DE_PARALLEL_ROUTER).toBeUndefined();
823795
});
824796

825797
// An explicit user choice outranks enrolment in both directions — the
826798
// documented escalation path for anyone who wants the router regardless.
827-
it("never overrides an explicit user value, enrolled or not", async () => {
828-
trackingState.canaryEnabled = false;
799+
it("never overrides an explicit user value", async () => {
829800
configState.disk = {
830801
telemetryEnabled: true,
831802
deParallelRouterTrialFired: false,

packages/cli/src/commands/render.ts

Lines changed: 17 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import { failCommand, requestCliExit } from "../utils/commandResult.js";
2-
import { isCanaryEnabled } from "../telemetry/canary.js";
32
import { defineCommand } from "citty";
43
import type { Example } from "./_examples.js";
54
import { mkdtempSync, readdirSync, readFileSync, statSync, writeFileSync, rmSync } from "node:fs";
@@ -1226,29 +1225,25 @@ function applyDeParallelRouterCircuitBreaker(quiet: boolean): boolean {
12261225
return false;
12271226
}
12281227

1229-
// The rollout slice. Default-ON means every eligible render routes the
1230-
// moment this ships — a ~17x jump in exposure, onto profiles today's trial
1231-
// population never covered (<=4 CPUs, Docker: ~12% of eligible renders
1232-
// between them). Note "trial" is not a user opt-in: it arms automatically on
1233-
// the CLI render path, so ~11% of installs already route without anyone
1234-
// choosing it. The opt-in is at the CALL SITE — the flag excludes
1235-
// programmatic renderLocal consumers, not users.
1228+
// Nothing left to gate: the router is a shipped default for every install,
1229+
// so leave the var unset and let the producer's default-ON apply. The
1230+
// breaker above is the only thing that turns it off, per install, and only
1231+
// after a real fallback. `HF_DE_PARALLEL_ROUTER=false` remains the user-
1232+
// facing kill switch.
12361233
//
1237-
// 0.7.60-0.7.64 is why that matters: every unclamped render reverted for
1238-
// five consecutive releases and nobody saw it.
1234+
// The `de-parallel-router` canary that used to sit here was removed with its
1235+
// registry entry (they had to go together — at >=100 the evaluator
1236+
// short-circuits ahead of the CI/seedless exclusions, so deleting only the
1237+
// entry would have flipped whatever still resolved false at deletion time).
12391238
//
1240-
// Ramping through the registry makes the exposed fraction a number someone
1241-
// chose. Today's ~11% is emergent — the product of eligibility rules and a
1242-
// capped trial — so it drifts with fleet composition and cannot be reverted
1243-
// without a release. Setting the percentage to 0 turns the router off for
1244-
// everyone, immediately, with no code change.
1245-
//
1246-
// Disarm uses the same explicit "false" the breaker writes, for the same
1247-
// reason: with default-ON polarity, deleting the var means ON.
1248-
if (!isCanaryEnabled("de-parallel-router")) {
1249-
applyDeParallelRouterBreaker();
1250-
return false;
1251-
}
1239+
// Two claims from the ramp's rationale were wrong, recorded so they are not
1240+
// reintroduced: "~17x jump in exposure onto <=4 CPUs / Docker" overstated
1241+
// the reach — Docker renders never use drawElement at all (0 of 4,281
1242+
// measured) and the router requires it, so no percentage ever exposed
1243+
// Docker. And "~11% of installs already route" was an OUTCOME (the share
1244+
// clearing eligibility and the old 25-render cap), not an exposure setting;
1245+
// read as a rollout knob it inverted the arithmetic, which is how gating at
1246+
// 5% came to CUT fleet exposure ~25x rather than ramp it.
12521247
return true;
12531248
}
12541249

packages/cli/src/telemetry/canary.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
*
1010
* ```ts
1111
* import { isCanaryEnabled } from "../telemetry/canary.js";
12-
* if (isCanaryEnabled("de-parallel-router")) { ...ramped path... }
12+
* if (isCanaryEnabled("your-feature")) { ...ramped path... }
1313
* ```
1414
*
1515
* That is the whole API. Percentage lives in the registry, not at the call

packages/core/src/canary.test.ts

Lines changed: 5 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,4 @@
11
import { describe, expect, it } from "vitest";
2-
import { readFileSync } from "node:fs";
3-
import { join } from "node:path";
42
import { canaryBucket, evaluateCanary, parseCanaryOverride, type CanaryInput } from "./canary.js";
53
import { CANARIES, canaryEnvVar, findCanary, overdueCanaries } from "./canaryRegistry.js";
64
import {
@@ -293,29 +291,10 @@ describe("registry", () => {
293291
}
294292
});
295293

296-
// The registry is data, so a ramp is a one-line edit with no code review
297-
// surface. This canary's own description says "ramp only alongside the
298-
// per-install circuit breaker" — without an assertion, bumping it to 5
299-
// before that wiring lands would go green.
300-
// The registry is data, so a ramp is a one-line edit with no code review
301-
// surface. The previous version enforced "ramp only alongside the circuit
302-
// breaker" by pinning the percentage to 0 — which blocks the ramp forever
303-
// and never checks the wiring it names.
304-
//
305-
// Assert the wiring instead: a non-zero percentage is allowed only while
306-
// the CLI render path really gates on this canary AND still consults the
307-
// per-install breaker. Ramping without the gate would enrol everybody at
308-
// once, which is the whole thing the ramp exists to prevent.
309-
it("only ramps de-parallel-router while the CLI render path gates on it", () => {
310-
const pct = findCanary("de-parallel-router")?.percentage ?? 0;
311-
if (pct === 0) return;
312-
const renderSrc = readFileSync(
313-
join(import.meta.dirname, "..", "..", "cli", "src", "commands", "render.ts"),
314-
"utf8",
315-
);
316-
expect(renderSrc).toContain('isCanaryEnabled("de-parallel-router")');
317-
expect(renderSrc).toContain("deParallelRouterTrialFired");
318-
});
294+
// The de-parallel-router wiring assertion that lived here was removed with
295+
// the canary itself (registry entry + render.ts guard, same commit). The
296+
// per-install circuit breaker it referenced is unchanged and is covered by
297+
// the CLI's own render tests.
319298

320299
it("has in-range percentages and a parseable sunset date", () => {
321300
for (const c of CANARIES) {
@@ -329,7 +308,7 @@ describe("registry", () => {
329308

330309
it("derives the override env var from the name", () => {
331310
expect(canaryEnvVar("de-parallel-router")).toBe("HF_CANARY_DE_PARALLEL_ROUTER");
332-
expect(findCanary("de-parallel-router")?.name).toBe("de-parallel-router");
311+
expect(findCanary("calibration-10")?.name).toBe("calibration-10");
333312
expect(findCanary("nope")).toBeUndefined();
334313
});
335314

packages/core/src/canaryRegistry.ts

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -80,27 +80,6 @@ export const CANARIES: readonly CanaryDefinition[] = [
8080
owner: "vance",
8181
sunsetAfter: "2026-09-15",
8282
},
83-
// ── Real rollouts ────────────────────────────────────────────────────────
84-
{
85-
name: "de-parallel-router",
86-
// Ramp 5 -> 25 -> 100. This gates the DEFAULT-ON behaviour (uncapped, no
87-
// telemetry precondition), not the old capped trial — so 0 means the
88-
// router is off for everyone and is a full revert without a release.
89-
//
90-
// Calibration validated the bucketer first: 9.62%/49.76% against 10%/50%
91-
// targets at n=13,547, overrides and CI both attributable, sustained
92-
// cohort flips at 0.10% — an order of magnitude under this feature's own
93-
// ~2.79% revert rate.
94-
//
95-
// At each step split revert rate by cpu_count and is_docker. Hold at 5
96-
// until PRINFRA-372 is resolved: `--workers auto` crashes every worker on
97-
// macOS arm64 while `--workers 1` is clean, and the router forces 3.
98-
percentage: 5,
99-
description:
100-
"Route auto multi-worker renders to verified parallel drawElement streaming (HF_DE_PARALLEL_ROUTER). Ramp only alongside the per-install circuit breaker.",
101-
owner: "vance",
102-
sunsetAfter: "2026-10-01",
103-
},
10483
] as const;
10584

10685
export function findCanary(name: string): CanaryDefinition | undefined {

0 commit comments

Comments
 (0)