mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-03 04:38:33 +00:00
fix(cli): keep the DE parallel-router trial on until a real failure, not first engagement
Only consuming telemetry from one data point per install badly undersampled the "routed" (successful) outcome — the far more common case. Changed maybeConsumeDeParallelRouterTrial to only turn the trial off when the router's OWN safety net actually fired (deParallelRouter === "reverted"), not on a clean "routed" success. This runs the experiment on every eligible render for an install indefinitely until it hits one real failure, then stops for that install going forward — trading a slightly higher per-install ceiling on experimental-path exposure for dramatically more successful- routing telemetry volume across the fleet. Also fixed a related edge case while updating this: a render that merely "routed" (router fired, self-verify never even tripped) but then crashed for an unrelated reason (e.g. cancellation) no longer counts as a router failure — only "reverted" (the router's fallback path actually engaged) does. Cancelling a render isn't evidence the router is unsafe. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
37b6a4e7e5
commit
19f90b0b92
@@ -505,7 +505,7 @@ describe("renderLocal — DE parallel-router CLI trial", () => {
|
||||
expect(process.env.HF_DE_PARALLEL_ROUTER).toBeUndefined();
|
||||
});
|
||||
|
||||
it("persists deParallelRouterTrialFired when the router actually engages on a successful render", async () => {
|
||||
it("does NOT persist the trial as fired on a clean 'routed' success — keeps trying on future renders", async () => {
|
||||
configState.config = { telemetryEnabled: true, deParallelRouterTrialFired: false };
|
||||
producerState.executeImpl = async (job) => {
|
||||
job.perfSummary = {
|
||||
@@ -514,6 +514,18 @@ describe("renderLocal — DE parallel-router CLI trial", () => {
|
||||
};
|
||||
};
|
||||
await renderLocal("/tmp/project", "/tmp/out.mp4", baseOptions);
|
||||
expect(configState.writeConfigCalls).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("persists the trial as fired when the router's own safety net actually reverted", async () => {
|
||||
configState.config = { telemetryEnabled: true, deParallelRouterTrialFired: false };
|
||||
producerState.executeImpl = async (job) => {
|
||||
job.perfSummary = {
|
||||
resolution: { width: 100, height: 100 },
|
||||
drawElement: { parallelRouter: "reverted" },
|
||||
};
|
||||
};
|
||||
await renderLocal("/tmp/project", "/tmp/out.mp4", baseOptions);
|
||||
expect(configState.writeConfigCalls).toContainEqual(
|
||||
expect.objectContaining({ deParallelRouterTrialFired: true }),
|
||||
);
|
||||
@@ -528,11 +540,23 @@ describe("renderLocal — DE parallel-router CLI trial", () => {
|
||||
expect(configState.writeConfigCalls).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("persists the trial as fired from the failure path when the router engaged before a hard crash", async () => {
|
||||
it("does NOT persist the trial as fired when a render merely 'routed' crashes for an unrelated reason (e.g. cancellation) — not a router failure", async () => {
|
||||
configState.config = { telemetryEnabled: true, deParallelRouterTrialFired: false };
|
||||
producerState.executeImpl = async (job) => {
|
||||
job.errorDetails = { observability: { capture: { deParallelRouter: "routed" } } };
|
||||
throw new Error("worker crashed");
|
||||
throw new Error("render cancelled");
|
||||
};
|
||||
await renderLocal("/tmp/project", "/tmp/out.mp4", { ...baseOptions, throwOnError: true }).catch(
|
||||
() => {},
|
||||
);
|
||||
expect(configState.writeConfigCalls).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("persists the trial as fired from the failure path when the router's safety net reverted but the retry still failed", async () => {
|
||||
configState.config = { telemetryEnabled: true, deParallelRouterTrialFired: false };
|
||||
producerState.executeImpl = async (job) => {
|
||||
job.errorDetails = { observability: { capture: { deParallelRouter: "reverted" } } };
|
||||
throw new Error("worker crashed even after fallback");
|
||||
};
|
||||
await renderLocal("/tmp/project", "/tmp/out.mp4", { ...baseOptions, throwOnError: true }).catch(
|
||||
() => {},
|
||||
|
||||
@@ -1612,14 +1612,17 @@ function createNoopProducerLogger(): ProducerLogger {
|
||||
|
||||
/**
|
||||
* Enable the DE parallel-router experiment (`HF_DE_PARALLEL_ROUTER`, default
|
||||
* off) for this render, one time per install, so we get real-traffic router
|
||||
* telemetry (revert rate, verify-db distribution) without requiring anyone
|
||||
* to manually set the env var — see `HyperframesConfig.deParallelRouterTrialFired`.
|
||||
* off) for this render, on EVERY eligible render for this install, so we get
|
||||
* real-traffic router telemetry (revert rate, verify-db distribution)
|
||||
* without requiring anyone to manually set the env var — see
|
||||
* `HyperframesConfig.deParallelRouterTrialFired`. Deliberately runs
|
||||
* indefinitely (not just once) to maximize "routed" success-telemetry
|
||||
* volume; see `maybeConsumeDeParallelRouterTrial` for what turns it off.
|
||||
* Returns whether this call armed it (so the caller knows to check for
|
||||
* consumption afterward) — false if it's already fired once, or the user
|
||||
* already set the env var themselves (never override an explicit choice),
|
||||
* or telemetry is disabled (no point risking the experimental path if we
|
||||
* can't even record the resulting signal).
|
||||
* consumption afterward) — false if it's already failed once for this
|
||||
* install, or the user already set the env var themselves (never override
|
||||
* an explicit choice), or telemetry is disabled (no point risking the
|
||||
* experimental path if we can't even record the resulting signal).
|
||||
*/
|
||||
function maybeEnableDeParallelRouterTrial(quiet: boolean): boolean {
|
||||
if (process.env.HF_DE_PARALLEL_ROUTER !== undefined) return false;
|
||||
@@ -1629,8 +1632,9 @@ function maybeEnableDeParallelRouterTrial(quiet: boolean): boolean {
|
||||
if (!quiet) {
|
||||
console.log(
|
||||
c.dim(
|
||||
" Trying the experimental parallel drawElement capture path once for this install " +
|
||||
"(opt out: HF_DE_PARALLEL_ROUTER=false)",
|
||||
" Trying the experimental parallel drawElement capture path for this install " +
|
||||
"(disabled automatically if it ever needs to fall back; opt out anytime: " +
|
||||
"HF_DE_PARALLEL_ROUTER=false)",
|
||||
),
|
||||
);
|
||||
}
|
||||
@@ -1638,20 +1642,28 @@ function maybeEnableDeParallelRouterTrial(quiet: boolean): boolean {
|
||||
}
|
||||
|
||||
/**
|
||||
* After a trial-armed render, persist that the experiment actually engaged
|
||||
* (routed or reverted — either produces telemetry) so it's never enabled
|
||||
* again for this install. Checks both the success path (`perfSummary`) and
|
||||
* the failure path (`errorDetails.observability.capture`, mutated in place
|
||||
* before a hard failure throws) — a crash while routed still counts as a
|
||||
* fired trial. No-ops if the router never actually became eligible for this
|
||||
* render (e.g. too few frames): the trial stays available for a future run.
|
||||
* After a trial-armed render, persist that the router's OWN bet actually
|
||||
* failed — its self-verify/generic-failure safety net fired
|
||||
* (`deParallelRouter === "reverted"`) — so it's never enabled again for this
|
||||
* install. A clean "routed" (the render succeeded with no fallback) does
|
||||
* NOT consume the trial — the whole point is to keep trying on every
|
||||
* eligible render until we see one real failure signal, maximizing
|
||||
* successful-routing telemetry volume rather than stopping at the first
|
||||
* data point. Checks both the success path (`perfSummary`) and the failure
|
||||
* path (`errorDetails.observability.capture`, mutated in place before a
|
||||
* hard failure throws) — a render that still failed even after the
|
||||
* fallback retry counts too. A render that crashed for an unrelated reason
|
||||
* while merely "routed" (never reached "reverted" — e.g. cancellation)
|
||||
* does NOT count as a router failure and does not turn the trial off.
|
||||
* No-ops if the router never became eligible for this render (e.g. too few
|
||||
* frames): the trial stays available for a future run either way.
|
||||
*/
|
||||
function maybeConsumeDeParallelRouterTrial(trialArmed: boolean, job: RenderJob): void {
|
||||
if (!trialArmed) return;
|
||||
const engaged =
|
||||
job.perfSummary?.drawElement?.parallelRouter ??
|
||||
job.errorDetails?.observability?.capture.deParallelRouter;
|
||||
if (engaged === undefined) return;
|
||||
const failed =
|
||||
job.perfSummary?.drawElement?.parallelRouter === "reverted" ||
|
||||
job.errorDetails?.observability?.capture.deParallelRouter === "reverted";
|
||||
if (!failed) return;
|
||||
const config = readConfig();
|
||||
config.deParallelRouterTrialFired = true;
|
||||
writeConfig(config);
|
||||
|
||||
@@ -65,12 +65,14 @@ export interface HyperframesConfig {
|
||||
skillsMissingCount?: number;
|
||||
/**
|
||||
* True once the DE parallel-router experiment ("HF_DE_PARALLEL_ROUTER")
|
||||
* has actually engaged (routed or reverted — either produces telemetry)
|
||||
* on a render from this install. The CLI enables the experiment for free
|
||||
* on renders from a fresh install until this fires once, then never
|
||||
* touches it again — a one-shot trial to get real-traffic router
|
||||
* telemetry without requiring anyone to manually opt in via env var.
|
||||
* See `renderLocal`'s `maybeEnableDeParallelRouterTrial`.
|
||||
* has actually FAILED (its self-verify/generic-failure safety net fired —
|
||||
* "reverted", not merely "routed") on a render from this install. The CLI
|
||||
* enables the experiment for free on EVERY eligible render from a fresh
|
||||
* install — not just once — to maximize real-traffic router telemetry
|
||||
* (mostly successful "routed" outcomes) without requiring anyone to
|
||||
* manually opt in via env var; only a real failure turns it off, and only
|
||||
* for this install going forward. See `renderLocal`'s
|
||||
* `maybeEnableDeParallelRouterTrial`/`maybeConsumeDeParallelRouterTrial`.
|
||||
*/
|
||||
deParallelRouterTrialFired?: boolean;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user