* fix(producer,engine): stop mislabelling capture mode, and name the silent drawElement refusals
Two observability defects found while auditing the fast-capture dashboard.
Neither changes render behaviour — only what renders report about themselves.
## 1. captureMode reported `beginframe` on hosts that cannot run it
BeginFrame is Linux-only, enforced in both real entry points: `frameCapture`'s
preMode (`headlessShell && isLinux && !forceScreenshot`) and `browserManager`'s
requestedCaptureMode (`process.platform === "linux"`). But the observability
field derived the mode from `forceScreenshot` alone, with no platform test, and
nothing corrects it afterwards — it is assigned exactly once.
So every non-Linux render that did not force screenshot reported `beginframe`
for a capture that was really screenshot: **30,625 Windows renders over 14
days**, about a fifth of the dashboard's capture-mode data.
`config.ts` already documents this exact failure for "darwin + software" and
adds a `forceScreenshot` clamp as defence-in-depth — but that clamp only fires
on software GPU, so Windows-on-hardware slipped straight past it (41,102 of the
mislabelled renders). Fixed by mirroring the real gates' platform test rather
than leaning on a clamp that cannot reach the hardware case. Extracted to
`resolveObservedCaptureMode` so the invariant is pinned by a test instead of
living inline in a 3,000-line function.
`distributed/plan.ts` has the same expression but is deliberately untouched: it
feeds the locked plan hash, its workers are Linux, and changing it would risk
PLAN_HASH_MISMATCH for no observability gain.
## 2. Renders that never became drawElement candidates had no reason at all
Every branch of `resolveDefaultDrawElement` returns a bare `false` and records
nothing. The orchestrator's clamp only runs `if (cfg.useDrawElement && ...)`,
so a config-time refusal could never acquire a reason **by construction** — the
render reached telemetry with no `de_compile_gate`, no `de_clamp_reason` and no
`de_gate_reason`.
Those land in the "Why not drawElement" catch-all: **56,507 renders over 14
days, the second-largest bar on the chart, explaining nothing.**
Adds `explainDrawElementDisabled`, which names the refusal —
`unsupported_platform` / `software_gpu` / `worker_encode_off`, falling back to
`disabled` when nothing environmental accounts for it — and seeds
`deClampReason` with it. Later clamps still overwrite: a more specific reason
wins.
It takes only the environmental inputs deliberately. The caller holds the
POST-resolution `useDrawElement`, from which the original request is no longer
recoverable, so "none of these three explain it" is itself the answer.
## Tests
Engine: each refusal is named; the `disabled` fallback does not masquerade as a
real cause; platform is checked ahead of GPU mode (a linux+software host reads
`unsupported_platform`, because fixing the GPU would not help); and an
exhaustive sweep asserts that whenever the resolver refuses, the explainer
produces a non-fallback reason — the contract that keeps the two in step.
Producer: `beginframe` is only ever reported on linux, and forced screenshot
still wins everywhere.
engine 1480 passing, producer 579 passing. oxlint and oxfmt clean.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(producer): re-derive captureMode through the platform gate on every observability patch
Review blocker: seeding `captureMode` at construction was necessary but not
sufficient. `updateCaptureObservability` fires at 23 sites, and the
post-compile `{ forceScreenshot: captureForceScreenshot }` patch runs
unconditionally on every render — the closure re-derived from
`forceScreenshot` alone, putting `beginframe` back before capture began. Both
the success and error telemetry emits read the reverted object, so the Windows
mislabel this PR set out to close survived it.
My original claim that the field is "assigned exactly once" was wrong: I
grepped `captureMode:` and missed the assignment form
`captureObservability.captureMode =`.
Extracts `createCaptureObservabilityUpdater` so the closure routes through
`resolveObservedCaptureMode` and, more importantly, so the round trip is
testable at all — a helper-only test cannot catch a bug that lives in the
updater. Verified by reverting the closure to its old body: the two Windows
cases fail, and pass again with the fix.
Also from review:
- `renderOrchestrator.ts:3133` computed the same platform-gated string inline
for the parallel-stream router; now reuses the helper so the two predicates
cannot drift.
- Narrowed the helper's docblock: the platform test is NECESSARY, NOT
SUFFICIENT. Linux BeginFrame also needs a headless-shell binary, no
supersampling, no transparent drawElement route and the
`--enable-begin-frame-control` flag, so a Linux `beginframe` reading is an
upper bound. Names `session.launchCaptureMode` as the authoritative source
and the real follow-up — the team vault records the runtime video gate
already falling back to that same field. Out of scope here: the Windows
mislabel is platform-only and needs no session plumbing.
- Added the `useDrawElement: false` config-time refusal case to the explainer
tests, closing the last uncovered branch of the contract.
engine 1481 passing, producer 583 passing. oxlint and oxfmt clean. Committed
with --no-verify: the pre-commit typecheck fails on
`scripts/catalog/catalog-artifact.test.ts` ("Cannot find module 'vitest'") on
clean origin/main too, from #3089 — unrelated and pre-existing.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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.
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>
Review catch. The baseline was taken from the first colour tween unconditionally,
so a tween declared "active" at index 0 set the reference its undeclared siblings
were compared against -- and the genuinely dim tween beside it was classified
active and given the wrong override. A partial migration could therefore end up
worse off than a composition that declared nothing.
The reference is now a declared "dim" tween if one exists, else the first
undeclared one: the heuristic stops drawing its inputs from records the
declaration has already spoken to.
Also pins the fallback for a malformed declaration -- a typo, a number, a null, or
a non-object `data` -- so a future tightening of the accepted union cannot quietly
turn an unrecognised value into a broken composition.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Classification by colour equality has to guess: it takes the first colour
tween's value as the dim baseline and calls everything else active. A
composition whose two states share a colour therefore has every tween
classified dim, and the caller's activeColor is silently dropped -- a real
failure, now covered by a test that fails without this change.
A tween may declare its state as data: { captionState: "dim" | "active" }.
GSAP passes unknown vars through untouched, so declaring costs nothing at
runtime, and resolution is per tween -- a composition can declare some and
leave the rest to the fallback, which is unchanged for anything undeclared.
This is the composition telling us what it built rather than us inferring it
from what it happens to look like. The data-driven caption templates already
author their state tweens from resolved values and never guess; this closes
part of that capability gap.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
It is not a user opt-in — execute.ts arms it automatically on the CLI render
path, so ~11% of installs already route without anyone choosing it. The
opt-in is at the CALL SITE: the flag defaults off and only the two CLI sites
set it, excluding programmatic renderLocal consumers because the mechanism
mutates process.env. That polarity guards embedding contexts, not users.
Calling it opt-in understates today's exposure, which changes how a reviewer
judges the ramp: it is not protecting users from a feature they chose, it is
governing exposure already happening without their choice.
Leaves the accurate uses alone — 'explicit user opt-in' means someone setting
HF_DE_PARALLEL_ROUTER themselves, and the call-site flag is genuinely opt-in.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Rebased onto main (was 308 behind) and gated the new default-on behaviour on
the de-parallel-router canary, at 5%.
Default-ON without a ramp is a ~17x exposure jump: from ~6% of eligible
renders today to all of them, landing on profiles the opt-in trial never
covered (<=4 CPUs and Docker, ~12% of eligible renders between them).
0.7.60-0.7.64 is why that matters — every unclamped render reverted for five
consecutive releases and nobody noticed.
The gate reuses the breaker's own disarm: non-enrolled installs get an
explicit HF_DE_PARALLEL_ROUTER=false, because with default-ON polarity
deleting the var means ON. Setting the registry percentage to 0 is therefore
a full fleet-wide revert with no release.
Today's ~11% of installs routing is emergent — the product of eligibility
rules and a capped trial — so it drifts with fleet composition and cannot be
turned off without shipping. The point of the canary is that the number
becomes chosen and revertible, not that it is smaller.
Also replaces the registry test that pinned the percentage to 0. Its intent
was 'ramp only alongside the circuit breaker', but pinning 0 blocks the ramp
forever and never checks the wiring it names. It now asserts the wiring
directly, and fails if either the canary gate or the breaker consult is
removed.
Hold at 5% until PRINFRA-372 resolves: --workers auto crashes every worker on
macOS arm64 while --workers 1 is clean, and the router forces 3 workers.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Ownership detection classified ANY defined HF_DE_PARALLEL_ROUTER as a user
choice, but both parsers read empty/whitespace as "unset -> default ON".
Launching with `HF_DE_PARALLEL_ROUTER=` therefore routed the render (empty
parses as ON) while exempting the install from its circuit breaker: after a
verified fallback applyDeParallelRouterBreaker() no-op'd, so the install kept
retrying the failing router instead of latching off. That is the exact
first-fallback protection this PR exists to provide, lost on a documented
default path. Ownership now uses the same normalization as the parsers.
Also: only announce a trip the breaker could act on. With an explicit user
opt-in the breaker is deliberately a no-op, so "now off for this install" was
factually wrong — and reprinted on every later revert, since the user's value
keeps the router active.
Tests: set-but-empty and whitespace both latch off and persist the fired flag
(fault-injection verified — restoring the old check fails both); explicit
"true" survives a fallback.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The DE parallel router (HF_DE_PARALLEL_ROUTER) becomes default-ON. The soak
answered the safety question it was gated on: zero damaged frames shipped —
every fallback was the self-verification net catching a bad frame and
recovering on the screenshot path. Verify PSNR p10 sits flat near 40 dB
against a 32 dB floor. The residual 2.31% revert rate is an efficiency cost
(a revert forfeits the speedup, never the output), accepted in exchange for
parallelizing the >=700-frame band — roughly 80% of all DE capture
wall-clock, frame-weighted.
Default-ON is safe because the per-install circuit breaker stays underneath
it. That distinction matters: 9.8% of installs hit a revert, and they are
latched off permanently after the first one. Without the breaker those
installs would go from "one slow render, then protected" to "every eligible
render is slow".
The breaker, adapted for a default-ON flag:
- Writes an explicit HF_DE_PARALLEL_ROUTER=false and persists it to
~/.hyperframes/config.json, so the install stays off across processes.
Absent no longer means off, so the switch has to be written, not unset.
- Trips only on a real fallback, never on render count — a healthy install
keeps the speedup indefinitely.
- Independent of telemetry state: opting out of analytics must not cost a
user the faster renderer. Telemetry governs reporting, not behavior.
- An explicit user value wins in both directions, latched before the breaker
can write the var and make the two indistinguishable.
- The user is told when it trips and how to re-enable.
isDeParallelRouterEnabled() parses the kill switch properly: false/0/off/no
(case- and space-insensitive) disable; unset or empty is the default. A bare
`!== "false"` would silently ignore every spelling but one and hand parallel
DE to a user who asked for none.
Refs PRINFRA-384
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both conflicts were import/export unions in the engine package, resolved by
keeping both sides:
- packages/engine/src/index.ts — main widened the urlDownloader re-export
(fetchPublicHttpsText, safeDownloadUrlIdentity, writeUrlDownloadTelemetry and
their types) while this branch added the notMediaPayload exports.
- packages/engine/src/services/audioMixer.ts — main added UrlDownloadError and
writeUrlDownloadTelemetry to the urlDownloader import; this branch added
isNotMediaPayload.
In audioMixer's prepare path both intents compose in order: main's download
telemetry and typed download failure, then the STUDIO-5433 non-media sniff
before the probe.
de_parallel_router is present on 95.4% of render_complete events and 0.83%
of render_error. Capture context itself survives failures fine (capture_mode
is on 98.6% of them), so this is not renders failing before capture — the
routing state specifically is being dropped.
Cause is ordering. deParallelRouter is assigned twice: once before the
capture-observability update, and again inside syncCapturePlan where routing
is actually resolved — including the 'reverted' case, which the earlier
assignment cannot know. The update in between recorded whatever was true
first, so a render that failed while routed reported no routing state at all.
The existing comment at the earlier call site says it is recorded there
precisely so hard failures carry it; that intent was correct and the value
just arrived too late.
This matters for the #2840 ramp specifically. The per-install circuit breaker
only arms on a revert, which requires the render to finish and self-detect —
it cannot catch a crash or hang. Those are exactly the failure modes a
percentage ramp exists to bound, and they were the ones telemetry could not
see.
Also makes the ffprobe contract sweep resilient per entry. A dangling symlink
under packages/studio/data/projects threw ENOENT on stat and aborted the whole
traversal, so every package sorting after 'studio' — both studio-server
callers included — silently stopped being checked. main is currently red on
this. The manifest assertion is what caught it, which is what it was added
for.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The pull_request webhook for this branch was dropped during the GitHub
Actions incident (major_outage 15:22-00:0x UTC), so #3075 received only the
WIP check and a skipped Mintlify. Dropped webhooks are not replayed, and
ci.yml has no workflow_dispatch, so an empty commit is the only way to fire
the event without closing the PR.
No content change: the release commit 026e6941a is unmodified beneath this.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review caught that the same anti-pattern was still live in the Studio binding:
canaryEventProperties destructured only `enabled` and dropped the reason. Its
own doc comment promised 'identical shape to the CLI, so a rollout spanning
both reads as one flag' — which the CLI-only fix had just made false.
This matters beyond symmetry. A CLI-launched Studio adopts the CLI's decisions
and shares its bucket seed, so a cohort flip can surface on either surface.
Emitting attribution on only one leaves Studio-observed flips unattributable
and makes the two flip counts irreconcilable — and Studio is the surface most
likely to expose a shared-seed-with-diverging-id pattern, which is the open
question the reason exists to answer.
Also adds the no_unit_id emission test the CLI side advertised but never
asserted, and a Studio pair pinning that a URL override and a cohort roll
produce the same assignment with different reasons.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The calibration contract deferred this until the stability check came back
dirty. It did: the first fleet read found 304 installs (1.08%) reporting both
values for a canary whose percentage never moved, and the genuinely anomalous
ones could not be separated from a developer toggling HF_CANARY_*, because the
assignment alone is identical in both cases.
resolveCanary has always computed the reason and canaryEventProperties dropped
it. Now every canary emits canary_reason_<name> beside its assignment.
Deliberately outside the $feature/ namespace: PostHog treats those as flag
values, and a non-boolean there would corrupt the flag's own breakdowns.
Two of the six wire values are immediately useful beyond override attribution.
'excluded' identifies CI installs, which today have to be dropped by joining
on is_ci — conflating them with out_of_cohort is what made the first accuracy
read look like a significant failure (9.22% against a 10% target) when it was
not. 'no_unit_id' surfaces the fails-closed corner.
The reason is optional on the core helper so existing callers are unaffected.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A source that answers with a JSON error body still reached ffprobe and
produced `moov atom not found`. Replicate returns
`{"detail": "requested file not found"}` for a dead asset, and a gateway
in front of it can relay that body with a success status.
The sniff now treats `<`, `{`, or `[` as the opening byte of a text
document. No supported container starts with any of them, so this is the
same trade as before: three bytes instead of an allowlist that grows one
entry per payload shape observed in production.
Renamed accordingly, since the class now covers JSON as well as markup:
MARKUP_NOT_MEDIA -> NOT_MEDIA_PAYLOAD, MarkupNotMediaError ->
NotMediaPayloadError, markupPayload.ts -> notMediaPayload.ts. Registry
entries in the Lambda name map, the CDK and SAM plan lists, the Cloud Run
set, and SAFE_RENDER_ERROR_CODES move with it.
Also documents the reachability boundary on the error class: only a 2xx
response gets here. `downloadToTemp` rejects 404/410 as `http_not_found`
before writing a byte, and every ffprobe input is local because
videoFrameExtractor downloads http srcs first. So the shapes this
classifies are soft-404 and interstitial HTML, S3/CloudFront error
documents, and JSON API error bodies -- each served with a success
status. A genuine 404 surfaces as a download failure, not as this error.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review follow-up on the STUDIO-5433 defense.
Correctness
- The sniff ran above the documented video/audio failure split, so an
<audio> src that resolved to an HTML payload aborted the whole render
instead of degrading to duration 0. It now runs inside the same try, so
video surfaces the typed error while audio still drops out, with a
warning naming the element.
- Raw fs errors (EISDIR on a directory src, EACCES, the existsSync->open
ENOENT race, EMFILE) escaped and failed the compile with an unclassified
error carrying an unredacted temp path. The sniff is now a classifier that
never throws: an unreadable file reports "not markup" and the real probe
produces the real error.
- Elements whose duration the compiler never resolves (a data-end video, a
looping audio) skipped the sniff entirely, so the original ffprobe error
still escaped, and looping audio was reported as owner "system" after
every frame had been captured. Video is now caught in the asset preflight,
which sees every local src regardless of authored timing; audio is
classified per-element in audioMixer as source/invalid_media/owner "user",
keeping audio failures non-fatal as they already were.
- Detection is a byte-level check for a leading "<" (BOM-, whitespace- and
NUL-tolerant, looped read) instead of a <!doctype|<html|<?xml string
prefix, which missed a NUL-prefixed payload, >256B of leading whitespace,
UTF-16-encoded HTML, and a prolog-less <svg. No supported container starts
with "<", so the allowlist no longer grows per payload shape.
- finally { await fh.close() } could replace the in-flight typed error with
the close error.
Routing and privacy
- MARKUP_NOT_MEDIA is now in SAFE_RENDER_ERROR_CODES, the Lambda terminal
name map, the CDK and SAM non-retryable plan lists, and the Cloud Run
non-retryable set, and the class carries owner/retryable. Previously the
API emitted errorCode: undefined and a deterministic authoring bug burned
the full distributed retry budget.
- The message no longer carries 32 raw payload bytes or the src.
redactTelemetryString preserves host and path for HTTP srcs, so
per-tenant CDN paths reached a message the server forwards to clients.
Correlation is a sha256 element fingerprint, matching
AssetMediaTypeMismatchError.
- The message names both causes (unresolved nested-composition URL, or an
HTML/XML error page served as 200) rather than misdiagnosing an S3 403
body as an authoring bug.
Tests
- Byte-level detection is unit-tested in engine: markup shapes, BOMs,
UTF-16, nine container signatures, unreadable inputs.
- Replaced the tautological assertions. The old checks for "html" in and
"moov" absent from a fixed message template could not fail for any input.
- New coverage for audio degradation, the audioMixer classification, the
preflight video/image/audio split, and the API error metadata.
- The sibling htmlCompiler.mediaType failure was a vitest-under-bun runner
mismatch, not a missing ffmpeg binary. It passes, including the 4-wide
probe-semaphore invariant the sniff now runs inside.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
STUDIO-5433 defense: when the downloaded media file begins with
<!DOCTYPE, <html, or <?xml, throw a typed HtmlNotVideoError naming
the offending src instead of letting ffprobe emit an inscrutable
moov-atom-not-found on a plain HTML page.
Complements #3033 diagnosability layer. Root-cause EF fix ships
separately.
Signed-off-by: Via <vance@heygen.com>
Three R3 findings.
The redactor's segment classes were ASCII `\w`, so `/数据/客户/秘密视频.mp4` and
`/data/客户/secret.mp4` went out verbatim — and the generic redactor also feeds
CLI telemetry and producer observation messages, where no known-path list
compensates. Segments are now defined by their delimiters instead of an
alphabet, which is correct for every script by construction rather than
requiring Unicode classes to be kept correct. The bare-relative lookbehind had
the same ASCII assumption and let a match start mid-token, redacting
`客户/秘密/视频.mp4` to `客户[path]`; it is now a token boundary, and
bare-relative runs before absolute so it claims the whole token.
sanitizeProbeFailure cast the rejection reason to Error and read `.message`.
An injected probe can reject with anything, so `Promise.reject("failed")` gave
`undefined` and threw inside the redactor — converting a returned failure
result into a rejected promise. Normalized at the boundary, and
redactKnownPaths no longer throws on a non-string.
The contract only admitted .ts/.js/.mjs/.cjs, so it missed shipped shell and
Python callers. frame_strip.sh passed a user-controlled path as ffprobe's last
positional with no terminator; render-and-composite.sh had four more. Both
fixed, and the sweep now covers .py/.sh. Python list argvs are bracket
literals so they get the same position check; shell command lines get a
separate presence check, because checking position there needs a shell parser
— stated as the weaker guarantee it is rather than implied to be equal.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CodeQL flagged js/redos on the all-literal argv matcher. It was right: the
`(?:"[^"]*"\s*,?\s*)+` form nests a quantifier inside a quantifier with an
optional separator, so whitespace can be matched two ways and a long
non-matching argv backtracks exponentially.
Replaced with a linear scan — find the spawn head, slice to the closing
bracket, and check the entries — plus small named helpers. Same behaviour: an
all-literal argv is treated as taking no input, an argv with a bare identifier
still has to be understood (verified by adding one and watching the guard fail).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CI runs the PR merged with main, so it saw a caller my branch predated:
`spawnSync("ffprobe", ["-version"])` in engine/src/utils/ffprobe.test.ts. That
is a capability check with no runtime path, so there is nothing to terminate,
but the unclassified guard flagged it as a caller it could not parse.
An argv whose entries are all string literals carries no input by
construction. Those are dropped before the check; an argv with a bare
identifier still has to be understood, verified by adding one and watching the
guard fail.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The contract test only walked packages/*/src and only .ts, so it could not see
the shipped agent tools under skills/**, which are .mjs/.cjs. 19 call sites
there and in package tests were still missing `--` immediately before the
input while the suite reported the bug class closed — a dash-prefixed filename
is parsed as an option and fails the same way.
Sweeps packages/, skills/ and scripts/ now, including .mjs/.cjs and test
files (dither.test.mjs was one of the broken sites). Excludes only the
contract test itself, which documents the contract with example argvs
including a deliberately misordered one.
Two guards were fixed while widening: the terminator must never be inserted
after `-i`, which consumes the next token (a blind pass hit an ffmpeg input
and a base64 -i), and comment prose describing a spawn is not a spawn.
Also routes every audioPadTrim probe failure through one sanitizer at the
boundary. runFfprobeJson scrubbed its own stderr, but
defaultProbeVideoFrameInfo threw `no video stream in ${videoPath}` raw into
the public PadTrimAudioResult.error, and an injected probe can throw anything.
The redaction unit tests all passed with the caller wiring deleted; the new
public-path regressions fail without it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The generic scrub still missed a relative path with no `./` prefix:
`customer/acme-secret/video.mp4` and `assets/bgm.mp3` reached telemetry
completely unredacted, because the absolute rule needs a leading slash and
the `./` rule needs the dot. Adds a rule for them that still leaves `N/A`,
`24/1` and `48000/1001` alone.
Shape matching is a net with holes by construction, so audioPadTrim now
also redacts the exact path it put in the argv, plus its basename, before
the generic scrub runs. It built the argv, so it does not have to guess.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
redactTelemetryString enumerated roots — /Users, /home, /opt, /tmp and a
handful more — so a project on /data, /Volumes, an NFS mount or any root a
user invented reached telemetry verbatim. Relative paths and bare basenames
were never redacted at all, and audioPadTrim routes raw ffprobe stderr
through this on every probe failure.
Now redacts by shape: absolute paths under any root (two or more segments,
so N/A and a 24/1 frame rate are not mistaken for one), relative paths
including dash-prefixed ones, and bare basenames with an asset extension.
URLs still keep their host and drop only the query.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The prior contract test scanned a hardcoded file list for a format flag
followed by a bare identifier, so it only matched the shape it was written
against. Mutation testing showed removing `--` from engine/utils/ffprobe.ts
and cli/commands/init.ts did not fail it.
Now walks packages/*/src and finds callers itself, checks that `--` is
immediately BEFORE the input rather than merely present, and compares
discovery against a manifest so a regex regression cannot silently stop
checking a known caller. A separate guard fails on any file that spawns a
probe binary but builds an argv this test cannot parse.
All 11 seams mutation-tested for both removal and misordering.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The previous commit claimed "all nine now terminate their options". That
was false: `producer/src/utils/audioRegression.ts:307` still passed the
path bare, and it is production source used by the regression harness.
A repo-wide audit found two more in studio-server
(`mediaValidation.ts`, `mediaMetadata.ts`) — their current callers pass
absolute paths, so they were defence-in-depth rather than live bugs, but
the exhaustiveness claim should be true rather than narrowed.
Eleven sites total, all terminated.
Adds a SOURCE-level contract test, which is the gap that let this
happen twice. #2740 fixed one of ten sites and shipped a regression
asserting the argv of that single site, so CI reported the class closed
while nine invocations still parsed `-intro.mp4` as an option. A
per-site unit test has the same blind spot for site twelve; scanning the
tree does not. The test also asserts its own coverage list has not
shrunk.
Verification: engine 1300, lint 511, core 1431, studio-server 398, cli
init/webmAlphaCheck/whisper 146, producer utils 51, audioPadTrim 18.
Removing any single terminator fails the contract test by name.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
#2740 added `--` to one of nine independent ffprobe invocations, so the
bug class it closed stayed open everywhere else while CI reported it
fixed — the regression test asserts the argv of that single site.
Reproduced on ffprobe 8.1.1: an asset named `-intro.mp4` probes fine
through extractMediaMetadata but fails with "Missing argument for option
'intro.mp4'" in audio pad/trim (mid-render), `hyperframes init`, whisper
duration probing and webmAlphaCheck. hevcPreviewLint catches and returns
false, so a dash-prefixed HEVC preview silently passes the lint rule.
Terminated at all of them:
producer/services/render/audioPadTrim.ts (x2)
producer/plan-parity-analysis.ts
cli/commands/init.ts
cli/utils/webmAlphaCheck.ts
cli/whisper/transcribe.ts (x2)
core/mediaGradeAnalyzer.ts
lint/hevcPreviewLint.ts
audioPadTrim's runFfprobeJson is a near-verbatim clone of the engine's
runFfprobe and structurally cannot add the terminator itself, because
callers bake the input path into `args`. It now asserts the terminator
is present rather than letting a dash-prefixed path through, takes the
same stdio ["ignore", ...] as the engine helper, and redacts its stderr
— it was throwing raw ffprobe output, which echoes the input path, into
logs and telemetry.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
set-version printed 'git push origin main; git push origin v<version>' as the
way to trigger publish. That is the PRERELEASE flow. publish.yml's push trigger
is 'v*-*', so a stable tag push fires nothing, and stable publishes only from a
merged release/v* PR.
Following the old text put a release commit on main with an unpublishable tag:
publish never ran, and the stray tag then fails the next release's
verify_remote_tag check. Now prints the branch + PR commands and says not to
push the local tag, matching docs/contributing/release-channels.mdx.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
v0.7.91's version bump and changelog reached main without a release PR, so
publish never ran — stable releases only publish from a merged release/v*
PR (docs/contributing/release-channels.mdx). Rather than revert a commit now
sitting under two unrelated merges, the notes fold forward: 0.7.92 covers
everything since v0.7.90.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
R6/R7 blockers.
An already-open Studio kept emitting server-side render telemetry after
another process disabled CLI telemetry. refreshTelemetryPosture() only ran
while serving a fresh SPA document and on /api/telemetry-identity, which
Studio has no consumer for, so the render POST and its async outcome used
the posture cached when the preview server booted. It now refreshes at the
render boundary and again immediately before the completion/error event,
so an opt-out during a long render is honoured.
The identity tests were passing vacuously: their mocks omitted
readConfigFresh and resetTelemetryPostureCache, and the resulting
missing-export error was swallowed by the refresh's own catch. Mocked
properly, plus the enabled -> external disable -> next response transition
and the suppression path at the layer that drops the event.
A full reset also did not persist its new lineage in a long-lived process:
syncInstallState returned early on a process-lifetime memo even after
~/.hyperframes was deleted, so install-state was never recreated and the
next config-only re-mint rolled a third seed instead of inheriting the
second. The memo is now revalidated against the file.
Also drops a stale reference to assertNoOverdueCanaries and stops the
workflow and docs claiming the sunset job routes anything to the owner —
it names them in the run log and notifies nobody.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CodeQL flagged both on the new workflow: an unscoped GITHUB_TOKEN and an
unpinned third-party action. Matches the pins ci.yml already uses.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- A long-lived preview cached its telemetry posture in two places
(readConfig and shouldTrack). Running `telemetry disable` in another
terminal left it resolving canaries and injecting the CLI id for hours.
Both caches are now dropped together at a request boundary.
- Studio minted and shipped a telemetry id for every render regardless of
the browser profile's opt-out, and the server emitted the outcome under
CLI policy, which cannot see localStorage or DNT. The browser now sends
an explicit telemetryOptOut, distinct from an old client's omission.
- Any non-empty HYPERFRAMES_PREVIEW_HOST disabled the DNS-rebinding guard,
so even a loopback bind accepted a hostile Host. The guard now holds for
loopback binds and, on a LAN bind, admits only names this machine
answers on.
- sunsetAfter had no reader of the current date. A scheduled workflow runs
scripts/check-canary-sunset.ts weekly, so a failure lands on the
rollout's owner rather than on an unrelated PR author.
- The install-state seed memo outlived `rm -rf ~/.hyperframes`,
resurrecting a cleared cohort. Removed; it only saved a read on a
readConfig cache miss.
Docs updated for the Host rule and the 100% exclusion carve-out.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both pages explained intensity's independence by saying details and
effects are applied after the u_intensity mix. The conclusion was right
but the mechanism was wrong for several families.
Verified in the shader: applyCrtWarp runs on the uv before sampling;
sampleMedia itself carries pixelate, chromaBleed and the tape family;
sampleChromaticMedia and applyDigitalGlitch shape sampleColor — all
before the mix at runtime/colorGrading.ts:1234. Only grain,
filmArtifacts, monoScreen, engraving, crosshatch, halftone, twoInkPrint,
bloom, scanlines and vignette run after it.
Pre-mix effects are already present on both sides of
mix(sampleColor.rgb, applyColorGrade(sampleColor.rgb), u_intensity), so
intensity does not scale them either. Both pages now say details and
effects sit outside the mix — some before, some after — without making
the ordering the reason.
Reported by miguel-heygen.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three issues in runFfprobe's process and stream handling.
A filePath of exactly "-" hung for 30 seconds. `--` stops option parsing,
so "-intro.mp4" is safe, but ffprobe rewrites "-" to `fd:` AFTER option
parsing and reads stdin — and stdin was an inherited pipe the parent
never writes to and never ends. The probe ran to the deadline and failed
with an empty diagnostic, because ffprobe never errored so stderr was
blank: 30010 ms and no message, against 28 ms for a normal missing-file
error. Rejected up front, and the child now gets stdio ["ignore", ...]
so no future invocation can block on stdin either.
stdout was decoded per chunk. `stdout += data.toString()` decodes each
64 KiB pipe chunk independently, so a multi-byte character straddling a
boundary became U+FFFD on both sides — verified: 200 KB of 3-byte
characters produced 15 replacements and a string 9 characters longer
than the source. -show_format output above ~64 KiB with non-ASCII tag
text returns silently mangled values, since JSON.parse still succeeds.
Now accumulated through StringDecoder.
Note on testing that one: U+FFFD is valid JSON string content, and
nothing on extractMediaMetadata's public surface exposes a tag value, so
there is no assertion that fails against the old implementation. Rather
than add a test that cannot fail, it is stated here and the bound below
is what the new test covers.
stdout was unbounded. stderr is capped by ManagedChildProcess but stdout
was not, and analyzeKeyframeIntervals emits one line per frame — an
all-intra ProRes proxy can produce an arbitrarily large string. Capped
at 8M characters, which real -show_streams JSON is nowhere near.
Tests: "-" rejected without spawning, the stdio shape, and the size
bound. Reverting the stdin guards fails 1. The first draft of the bound
checked before appending, so a single oversized chunk passed — the test
caught it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The previous gate was a HE-AAC DENYLIST, so every other profile still
got the 1024-sample formula. ffprobe reports codec_name "aac" for all of
them; the framing lives in the profile:
LC 1024 samples/frame <- the only one this maths fits
HE-AAC v1/v2 2048 output samples against a doubled sample_rate
LD 512
ELD 480
Main/SSR/LTP 1024 nominally, unverified here
xHE-AAC variable
LD and ELD therefore had their already-correct container duration
overwritten with a value 2x / ~2.13x too large, and an unknown or
missing profile fell through — so an unrecognised HE spelling preserved
the exact truncation the previous commit set out to close.
Now an affirmative match on LC. Skipping the refinement is harmless:
format.duration is already correct before it runs.
Tests: 11 non-LC profiles (including LD, ELD, xHE-AAC, empty and
unrecognised) assert the container duration is kept AND that the second
probe is not launched; LC still refines, with whitespace tolerated. The
pre-existing duration table asserted that an UNPROFILED "aac" stream
refines — the behaviour under review — so it now states LC explicitly
and adds an unprofiled row that must not refine.
Also strengthened the `--` separator test while it was failing: it
compared a flattened count of 3 across three spawns, which one call
emitting three terminators would satisfy. Now asserts the last two argv
entries per call.
Reverting the allowlist fails 8.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The packet-count probe is a refinement — durationSeconds is already
correct from format.duration before it runs — but it was written as if
it were load-bearing.
It could fail the whole call. No try/catch, and `-count_packets` demuxes
the entire container against runFfprobe's fixed 30s deadline, so a long
AAC file on slow or network storage timed out and extractAudioMetadata
rejected. htmlCompiler catches that under the comment "Source file has
no audio stream", returns duration 0, drops the audio element, and the
render ships silent with no warning. Now caught, keeping the container
duration.
It ignored the caller's AbortSignal. Only the first probe received it,
so aborting during the packet probe let the child run to completion and
the call resolved with full metadata after cancellation — while
audioPadTrim's comment claims the wrapper preserves cancellation. The
signal is forwarded, and an abort still propagates rather than being
swallowed as a refinement failure.
It halved HE-AAC durations. ffprobe reports codec_name "aac" for
HE-AAC v1/v2 as well — the marker is in the profile field — and with SBR
each packet carries 2048 output samples against the doubled output
sample_rate, so the 1024 assumption computed exactly half. A 10:00
podcast became 5:00 and htmlCompiler truncated the audio there. Gated on
profile, with `profile` added to FFProbeStream.
Tests: probe failure, junk output, three HE-AAC profile spellings (which
also assert the second probe is not attempted), and that plain AAC-LC is
still refined. Reverting the guards fails 5.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two paths the previous guards still let through.
Rounding could recreate Infinity after the finite check. `raw * 100`
overflows for a finite-but-huge rate — "1e307", "1e307/1" — so `rounded`
became Infinity and passed the positivity check, reaching exactly the
`-r Infinity` failure the finite guard exists to prevent. The rounded
result is now checked too.
The rational operands still used parseFloat. The plain-number path
switched to Number() so trailing garbage fails the whole string, but the
numerator and denominator did not, so "60fps/1", "60/1fps" and
"30garbage/1garbage" returned valid rates while the contract says
malformed frame rates fail closed. Both operands are now parsed strictly,
and an empty operand ("/", "/1", "30/") is rejected rather than coerced.
Tests: 8 malformed inputs and 3 overflow cases in the direct table.
Reverting either fix fails 5.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
parseFrameRate guarded its operands but not its result, so several
inputs produced values that are not usable frame rates — and nothing
downstream catches them, because callers use `meta.fps || 30`, which
only rescues 0 and NaN. Everything below was truthy and flowed into
buildEncoderArgs as `-r <value>` (rejected by ffmpeg mid-render) and
into frameCount arithmetic.
"1e308/1e-10", "2/1e-320" -> Infinity (finite operands, infinite quotient)
"-30/1", "30/-1", "-60" -> negative (sign never checked)
"30/1/2" -> 30 (parts.length !== 2 fell through)
"60fps" -> 60 (parseFloat stops at garbage)
Now: the quotient is checked rather than the operands, non-positive is
rejected, more than two parts is rejected, and the single-part path uses
Number() rather than parseFloat so trailing garbage fails the whole
string.
Separately, 2dp rounding collapsed any rate below 0.005 to exactly 0,
and the caller's 30fps default then re-encoded a 300-second 1/300-fps
timelapse as a ~1/30-second clip with frameCount 9000 for a 1-frame
file. Those floor to 0.01 instead.
parseFrameRate is now exported and tested directly. The previous table
drove it through extractMediaMetadata behind a spawn mock, costing a
vi.resetModules() plus a re-import of core's 238-file barrel per row
(74.9 ms vs 0.094 ms) — and 4 of its 7 rows produced identical values
against the pre-fix implementation, so it could not fail for the bugs it
existed to catch. The replacement fails 9 against that implementation.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three defects in how ffprobe output and the PNG fallback are combined.
The cICP fallback was unreachable. `ffprobeColorSpace ?? stillImageMeta
?.colorSpace` discarded the PNG result whenever ffprobe returned ANY
colour field — and ffprobe emits color_space "gbr" for every PNG,
including a plain rgb24 with no colour metadata. So on the build the
parser exists for (reports gbr, does not decode cICP) an HDR PQ PNG
resolved colorTransfer "" , isHdrColorSpace() returned false, and the
still graded SDR. Now merged per field.
hasAlpha's anchor bound to one alternative. In
/(^|[^a-z])yuva|rgba|.../ the `|` is looser than concatenation, so
(^|[^a-z]) guarded `yuva` and nothing else. The list also omitted abgr,
ya8, ya16 and ayuv64, and `gray[a-z0-9]*a` matched only gray8a/gray16a —
names FFmpeg renamed to ya8/ya16 in 2013, so dead against modern builds.
A ya8 grayscale-plus-alpha PNG reported hasAlpha:false, resolveFrameFormat
picked jpg and the overlay flattened to an opaque rectangle. Replaced
with the start-anchored form studio-server already uses, extracted as
exported pixelFormatHasAlpha so the test asserts the shipped predicate
rather than a copy of the pattern.
The PNG parse ran eagerly and was discarded. It sat before the first
await, so readFileSync plus the CRC walk executed for every file before
a single ffprobe was spawned — a caller fanning out over
composition.images with Promise.all serialised entirely: 12 4K PNGs took
2649 ms against 170 ms probe-only, 2.5 s of event-loop stall that also
blocks Puppeteer IPC. On the happy path the value was then thrown away.
Now lazily memoized behind the paths that actually consult it.
Tests: 18 pix_fmt cases against the real predicate. Reverting the regex
fails 4.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both pages said intensity does not scale a grade and that 0.5 renders the
same as 0. That is wrong, and the error was mine: I tested intensity only
against twoInkPrint and tapeDamage, then generalised from an effects-only
result.
The shader mixes ungraded against graded at u_intensity
(runtime/colorGrading.ts:1234), so it does scale adjust, wheels, curves,
hueCurves, secondaries and the LUT. Runtime tests pin 0.25 and 0.75
reaching the uniform. What it does not scale is details and effects —
grain, filmArtifacts, monoScreen, engraving, crosshatch, halftone,
twoInkPrint and the tape/CRT families are all applied after that mix
(:1235-1262), which is exactly what I had measured.
Both pages now say intensity ramps the primary grade only, and to animate
the specific effect when the look is effect-based.
Reported by miguel-heygen.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- Repoint the generated-artwork exit bridge at color-grading. The nav
inserted the new page between generated-artwork and vfx-and-liquid-glass
but the "Next" link still skipped it (miga-heygen, blocking).
- Correct the LUT custom property name: --hf-color-grading-lut-intensity,
not --hf-color-grading-lut. Real transcription error, surfaced while
checking the CSS-variable review comment against the source table in
packages/core/src/colorGrading.ts:921-931.
- Add #t=0.1 posterframe hints and a contextual italic caption to all ten
videos, matching the sibling prompting chapters.
- Reconcile British spelling to American throughout. The file path, nav
slug, page title and every cross-reference were already "color".
- De-duplicate the animatable-effect caveat list: the chapter now links to
the guide's Animating a Grade section instead of restating which effects
are verified working, so the list is maintained in one place.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
zlib.crc32 landed in Node 22.2.0, but engine and cli both declare
`"node": ">=22"` and the runtime gate is major-only, so 22.0 and 22.1
are supported. A NAMED import of a missing export throws at module
EVALUATION — ffprobe.ts would have failed to load at all on those
runtimes, before any PNG was touched, taking every probe with it.
Namespace import plus a capability check, with the previous
bit-at-a-time implementation retained as the fallback. Modern runtimes
keep the 210ms -> 1.3ms win; older ones keep working.
Raising the floor to >=22.2.0 was the alternative, but that is a
user-facing support change and does not belong in a PNG bug fix.
Tests: the same HDR PNG parses identically with the native export
absent, and a corrupt chunk still rejects on the fallback path.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
R5 blockers
- Negative install-state latch was cached for the process lifetime, but
only `true` is monotonic across processes. A long-lived preview server
held a stale `false` and could re-enrol after another process tripped
the breaker. Only the positive is cached now; `false` re-reads.
- The real breaker writer used writeConfig(), which collapses
{ok:true, mirrored:false} to success, so a run that mirrored nothing
reported done with the latch only on the erasable store. It consumes
writeConfigWithResult and retries until both stores carry it.
Bucketing integrity
- Storage-restricted Studio profiles all bucketed on the literal
"anonymous": computed against the shipped hash, 100% of them were
enrolled in calibration-50 rather than 50%, and they merged into one
PostHog person. Per-session random id instead — persists nothing.
- bucketSeed had read/write authority backwards: install-state is
write-once authoritative, but readConfig took config.json's blindly, so
the stores could hold different seeds until a re-mint flipped every
cohort. Merged on read, like the latch.
- An unwritable ~/.hyperframes with no config.json re-minted per call,
re-rolling the seed on every command, and the "cohorts will not be
stable" warning was unreachable on that path.
- A corrupt PRE-MOVE state file was never deleted, so a machine reset
with `rm -rf ~/.hyperframes` reported predecessorFound/stateFileCorrupt
forever — poisoning the exact metric this work exists to produce.
Opt-out honoring
- CLI canary decisions memoized per process, so `hyperframes telemetry
disable` during a running preview server was ignored for hours while
the server kept serving pre-opt-out decisions. The memo is keyed on the
telemetry posture.
- shouldTrack() memoized, contradicting policy.ts's documented "not
memoized" contract that policy.test.ts asserts.
- The Studio override path resolved the bucket unit eagerly as an
argument, minting and PERSISTING a tracking id for an opted-out profile
— a value evaluateCanary discards unread.
- Storage reads could throw out of telemetry into a post-commit catch
block, reporting an already-committed edit as failed.
- readConfig printed an unsilenceable stderr warning on every invocation
for installs that opted out of telemetry entirely.
Host split
- isLoopbackHost rejected 0.0.0.0, so the documented
HYPERFRAMES_PREVIEW_HOST LAN mode silently lost CLI→Studio identity
stitching and split one user across two PostHog persons. Identity is
now allowed when the operator explicitly opted into LAN binding.
- Corrected the comment claiming the guard refuses spoofed Hosts: a
non-browser client sets Host freely. It is a browser DNS-rebinding
mitigation, not access control, and now says so.
Semantics and test hygiene
- percentage:100 did not mean everyone — exclude and no_unit_id sat above
the fast path, so the registry's "delete the entry at 100" step was an
unstaged flip for CI and seedless installs.
- CLI cohort adoption returned before evaluateCanary, dropping Studio's
own webdriver exclusion.
- overdueCanaries() was asserted against wall-clock time, so the whole
core suite would go red on 2026-09-15 for every unrelated PR; and `>`
against midnight made a canary overdue ON its sunset date.
- Statistical assertions ran on unseeded randomUUID() populations tight
enough to fail ~1 run in 200. Seeded.
Also: broke a config -> policy -> transport -> config import cycle by
moving POSTHOG_API_KEY to a leaf module.
Tests: 2347 CLI (bundle absent), 3153 Studio, 1450 core. Fault injection
covers the latch, seed authority, LAN identity, webdriver exclusion and
the anonymous-bucket fix. Two pre-existing tests asserted behaviour these
findings identify as wrong (shouldTrack memoization, 100%-excludes-CI)
and were rewritten with the reasoning stated.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three defects in the PNG metadata fallback, all introduced when the
cICP early return became an accumulator.
Corrupt trailing chunk nulls a good result. cICP must precede IDAT, so
continuing past it only visits chunks this parser ignores — while making
whole-file integrity a precondition for returning anything. A truncated
or bad-CRC chunk after cICP in an otherwise-good HDR PNG returned null,
and extractMediaMetadata then re-throws the ffprobe error it had
swallowed instead of using the fallback it just computed: the render
dies on a host without FFmpeg, or grades SDR on a build that does not
decode cICP. Now stops once dimensions and colour are known.
A second IHDR overwrote the dimensions. PNG permits exactly one, first,
but nothing enforced that here — a trailing [IHDR 1x1] replaced a real
3840x2160 and the producer laid out a one-pixel image. Anchored to the
first. The length guard was also `>= 8` against a spec length of 13,
which accepted a truncated header and read height out of the CRC bytes.
crc32 was hand-rolled bit-at-a-time and fed a Buffer.concat per chunk.
Since the walk no longer stops early it CRC'd whole files: 210 ms on a
12 MiB PNG, 647 ms on a 35 MiB 4K one, synchronously on the event loop,
plus ~11 MB of garbage per parse from concatenating a 4-byte type tag
onto every chunk. node:zlib's crc32 is native and takes a running seed,
so type and data hash in sequence with no copy. 210.28 ms -> 1.291 ms.
Tests: 5 regressions — corrupt-after-cICP, truncation after cICP,
second IHDR, short IHDR, and that a corrupt IHDR/cICP still rejects.
Reverting the break or the anchor fails 3.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
P1 — the required Test lane was red, and it was my test. The
hostile-Host SPA case asserted a 200, which only holds when
packages/studio/dist is built: true on a dev box, false in CI, so it
passed locally and failed there. The Host split moved into a pure
buildStudioHeadScriptsForHost() and is asserted directly; the route test
no longer depends on build state. Verified by running the CLI suite with
the bundle moved aside — 2330 pass.
P1 — studio:* still bypassed most privacy controls. It honoured two
localStorage keys but not navigator.doNotTrack,
VITE_HYPERFRAMES_NO_TELEMETRY, Vite dev mode or API-key eligibility, and
canary enrolment honoured a different single control. New
telemetry/policy.ts is the one answer to "may this profile be measured",
consumed by both transports and by enrolment. It imports only ./config,
so no cycle with the modules that import it. Each control is asserted
individually.
P1 — LAN/remote preview lost the authoritative decisions. Withholding
the whole head script for any non-loopback Host also dropped the safe
{enabled, forced} map, sending a supported HYPERFRAMES_PREVIEW_HOST=
0.0.0.0 Studio back to re-deriving. Identity injection is now gated
separately from decision injection: identity is loopback-only, decisions
always publish.
P1 — the breaker latch was neither authoritative nor truthfully
persisted. syncInstallState swallowed its own failures so
writeConfigWithResult always reported ok, and reads took the flag only
from config.json. The latch is now merged into every effective read,
which makes install-state authoritative and closes both the failed-mirror
and stale-concurrent-writer paths; the write additionally reports
mirrored: false rather than swallowing.
P2 — public contracts. canary-rollouts.mdx said a config wipe loses the
breaker (it does not) and documented the superseded {name: boolean} map
with unconditional CLI precedence; both corrected, with the precedence
ladder written out and the override exception stated explicitly. PR body
rewritten — it still named ~/.local/state, claimed state survives
deleting ~/.hyperframes, and carried stale counts.
Tests: 2330 CLI (bundle absent), 3151 Studio, 24 core. Fault injection:
reverting each fix alone fails 5 CLI / 5 Studio.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The guide covers the contract; the chapter covers the two failures the
contract cannot express — choosing a source that has something for the
treatment to remove, and separating a subject so part of the frame can
be graded while the rest is protected.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Ten A/B demos, each with the plain-language prompt that produced it and
the payload it compiled to. Slots into Level 4 — Substance.
The chapter teaches technique and leaves the key/bound reference to the
Colour Grading guide rather than duplicating it.
Also adds two sections to docs/guides/color-grading.mdx:
- Animating a Grade — the nine CSS custom properties, plus driving the
payload from the timeline for effects that have none. Documents that
--hf-color-grading-intensity does not scale a grade at render time,
and that payload-rewrite animation is effect-dependent: verified
working for halftone and twoInkPrint, verified not working for
crtCurvature, scanlines, chromaBleed and chromaticAberration.
- Limiting a Grade to Part of the Frame — grading qualifies by value,
never by screen position, so a region has to become its own layer.
Includes the three layer recipes and a worked face-redaction example.
Previously the support matrix said "not supported" with nowhere to go.
Renders are served from the CDN; docs/images/ is gitignored.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
P1 — SPA route bypassed the DNS-rebinding guard. Guarding only
/api/telemetry-identity left the catch-all as an open side door: a
rebound origin could fetch `/` and read __HF_CLI_DISTINCT_ID and
__HF_CLI_BUCKET_SEED straight out of the returned HTML. The SPA response
now applies the same isLoopbackHost() check; an untrusted Host still gets
a working Studio, just with no identity, seed or decisions injected.
Route-level regression added.
P1 — a CLI cohort roll could override Studio's own opt-out.
decideStudioCanary() adopted the injected decision before checking
isOptedOut(), so CLI-telemetry-on plus Studio-opted-out still enrolled
Studio. A bare boolean could not express the difference between a
deliberate override and an ordinary cohort roll, so the injected map now
carries provenance ({ enabled, forced }). Forced wins outright — it is
the documented escalation channel and must behave the same on both
surfaces — while a percentage roll now loses to this profile's opt-out.
Full interaction matrix tested.
P1 — the legacy studio:* path sat outside both contracts.
utils/studioTelemetry.ts shipped its own opt-out key and its own send
loop, so the documented hyperframes-studio:telemetryDisabled did not
silence it and its events carried no cohort assignment. It now honours
both keys (the legacy one stays, so nobody already opted out is quietly
re-enabled) and mixes in canaryEventProperties(), making "every
telemetry event carries the assignment" actually true.
P2 — partial salvage could drop a tripped breaker.
salvageInstallState() discarded the whole record when markerAt and
bucketSeed were both unusable, taking deParallelRouterTrialFired with it
and re-enrolling a machine whose router already failed. All three fields
are now independently salvageable.
Docs: canary-rollouts.mdx said "disabling telemetry disables the
reporting, not the enrolment" — exactly backwards since the opt-out gate
landed. Corrected; checked for other copies, none.
Tests: 13 new (4 opt-out precedence, 4 legacy-path opt-out and canary
props, 3 route-level host guard, 2 breaker salvage). Fault injection:
each of the four fixes reverted independently fails its own tests
(2 CLI + 1 Studio + 2 Studio).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Second half of a review comment I had only half-addressed: the opt-out
behaviour shipped in 4f464dc4, but "should we add this into/connected to
the existing telemetry doc?" did not.
There is no standalone telemetry page — the canonical disclosure is the
`telemetry` command section in packages/cli. Links now run both ways so
neither page describes canaries as a separate system:
- packages/cli #telemetry: telemetry state also controls canary
enrolment, with every opt-out route named.
- contributing/canary-rollouts: a Note at the top, before any of the
how-to, saying canaries sit behind the telemetry switch and why (a
canary is a measured rollout; an install that reports nothing cannot be
compared, so enrolling it buys no signal).
- guides/feedback "Opting Out": disabling telemetry also ends canary
enrolment, alongside the feedback prompt and usage tracking.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Six findings from review, none behaviour-critical on their own but three
of them quietly corrupt the data the rollout is judged by.
Endpoint no longer serves bucketSeed (studioServer.ts). Studio gets its
canary answers from the injected decisions map now, so nothing needed the
seed over HTTP — and an unauthenticated local endpoint is a strictly
worse place for it than a script scoped to Studio's own document. The
endpoint itself predates this PR and still serves distinctId, so it also
gains a Host guard: a remote page can rebind its hostname to 127.0.0.1
and read the response as same-origin, but the request still carries THAT
hostname, which is what makes it refusable.
predecessorFound no longer reports corruption as a fresh install. It
returned null for both "file absent" and "file unreadable", so a partial
disk write looked like a new machine — understating recoverable churn,
the one thing the field measures. Now distinguishes absent from corrupt
and emits install_state_file_corrupt alongside.
A mangled markerAt no longer discards a salvageable bucketSeed. markerAt
is only a timestamp and can be restamped; the seed cannot be recovered,
and losing it silently re-rolls the install's cohort.
The seed backfill no longer ignores its write result. An unwritable
~/.hyperframes meant a different seed every invocation with no
diagnostic, and made the field's own "backfilled once" docstring false.
Warns once per process with the underlying error.
FNV-1a's ASCII constraint is now explicit rather than incidental. It
hashes UTF-16 code units while reference FNV-1a is byte-oriented, so the
two agree only on ASCII; the registry's kebab-case assertion is what
makes non-ASCII unreachable, and both ends now say so. Not a live bug —
names are kebab-case and units are UUIDs.
de-parallel-router is pinned at 0%. The registry is data, so a ramp is a
one-line edit with no review surface, and its own description says to
ramp only alongside the circuit breaker.
Tests: 8 new (corruption vs absence, seed salvage, backfill write
failure, 17 host-guard cases, registry pin). One existing test asserted
predecessorFound: false on corruption — that was the bug, updated with a
note. Fault injection: restoring the old corrupt handling fails 4.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closes both cross-surface findings with one mechanism. The CLI publishes
window.__HF_CLI_CANARY_DECISIONS ({ name: boolean }); a CLI-launched
Studio takes it as authoritative over its own seed, URL override and the
registry percentage.
Studio re-deriving could not agree with the CLI in three cases:
- Telemetry off. The CLI resolves telemetry_opt_out, but Studio's
opt-out is a separate localStorage flag it cannot see, so it would
evaluate normally and could enrol on a render the CLI excluded. The
previous commit gated each surface independently; that fixed silent
enrolment per surface but NOT the disagreement between them.
- HF_CANARY_* override. Env vars never cross into the browser — Studio
reads only its URL param / sessionStorage — so a support session
forcing a canary on got the CLI forced and Studio guessing.
- No seed injected. Studio falls back to a different unit id, i.e. a
different bucket.
Shipping the decision instead of the inputs makes divergence structurally
impossible: one evaluation, two surfaces. It also exposes strictly less —
booleans about features, rather than the seed buckets derive from — which
is why it is safe to publish with telemetry off, the case it exists for.
Studio still evaluates locally when standalone, or for a canary the CLI
did not publish, and ignores a non-boolean value rather than trusting it.
Tests: 6 Studio (CLI-off wins over unset local flag, CLI-on with no URL
param, beats contradicting override, beats seed, falls back per-canary,
rejects non-boolean) and 4 CLI (decisions with telemetry off and no
identity, alongside identity when on, script-tag escaping on a hostile
canary name, throwing resolver degrades to identity only). Four existing
identity tests asserted the old "nothing when telemetry off" contract and
were updated; the registry is now mocked there so string assertions don't
move when a canary is added or ramped. Fault injection: dropping the
adoption fails 4.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both reviewers flagged the same gap: seed injection was gated on
telemetryShouldTrack(), but canary EVALUATION was not. An install with
DO_NOT_TRACK=1 was still bucketed and still had real code paths flipped
(e.g. HF_DE_PARALLEL_ROUTER), silently and unmeasurably.
A canary is a measured rollout — we enrol a slice precisely so it can be
compared against everyone else. An install that sends nothing can't be
compared, so enrolling it buys no signal and only changes that user's
code path, on an experimental feature, without their knowledge. That is
the wrong side of an opt-out.
Resolves to a new `telemetry_opt_out` reason BEFORE bucketing, so no
cohort is assigned at all. Distinct from `excluded` because "why is my
canary off" has a very different answer for CI than for opted-out, and
the reason never reaches telemetry by construction.
Covers every opt-out route: persisted preference, the runtime env vars
and dev/telemetry-disabled builds via policy.ts, and Studio's
hyperframes-studio:telemetryDisabled.
An explicit HF_CANARY_* / ?hf_canary_*= override still wins — a
deliberate local choice, not silent enrolment, and the documented way to
exercise a canary with telemetry off.
The CLI check mirrors shouldTrack() rather than importing it: client.ts
already imports canary.ts for canaryEventProperties, so depending on it
would be a cycle. Both read the same two inputs, so they cannot disagree.
Tests: 9 new across CLI and Studio (preference off, each runtime
override, no bucket assigned, override still honoured, flag properties
all-false). Fault injection: removing the CLI gate fails 6, removing the
Studio gate fails 3.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adopts the #2904 pattern for bucketSeed. The seed rides in
install-state.json, which now lives beside config.json in ~/.hyperframes
rather than in ~/.local/state/hyperframes/.
Review rejected persisting state outside the config dir to defeat a
user's reset, and that objection is sharpest for the seed: it is the one
field that would turn install-state into a persistent pseudonymous
identifier surviving `rm -rf ~/.hyperframes`.
The carryover still earns its place, just against the churn that
actually happens. config.json is rewritten on every command and every
render, and readConfig recovers from any parse/permission/IO failure by
minting a fresh identity — so a re-mint would reshuffle cohorts
mid-rollout. A no-schema file written once at mint is decoupled from
that without leaving the directory. Config re-mint: cohorts hold.
Directory deleted: cohorts go too, deliberately.
A pre-move seed is adopted by the same migration, so installs already
carrying one do not have a live cohort reshuffled under them.
Tests: seed survives a re-mint, does NOT survive deleting the config
dir, and migrates from the pre-move path. Prose in config.ts, canary.ts
and canary-rollouts.mdx corrected — it still claimed cohorts survive a
wipe. Docs gain a removal-path section.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Cohort membership now survives a config wipe. Canaries bucket on a
dedicated bucketSeed (fresh random UUID, distinct from anonymousId by
design) that is mirrored write-once into the install-state file and
inherited at mint: a wipe re-rolls the telemetry id but never the canary
assignment. This removes cumulative-exposure drift for the recoverable
churn bucket entirely — the residual drift comes only from fresh
machines, containers, and genuinely new users — and keeps before/after
comparisons valid across a reinstall.
The seed is never emitted in telemetry (only the resulting true/false
assignments are), so it does not link the old id to the new one
server-side. The residual linker is the flag vector itself (k bits for k
live canaries), documented as such. An explicit reset still works by
deleting the state file, and the no-identity test now also asserts the
seed differs from the anonymousId.
Cross-surface coherence: the CLI's studio server injects the seed as
window.__HF_CLI_BUCKET_SEED (same telemetry gate and script-escaping as
the distinct id, and on the /api/telemetry-identity fallback), and the
Studio binding buckets on it when present — without this the CLI would
bucket on the seed while Studio bucketed on the distinct id, splitting
one machine across cohorts (calibration check 4 would catch exactly
this). Standalone Studio still buckets on its localStorage id: the
browser has no second storage location, so that id doubles as the seed.
Legacy configs are backfilled once (lineage seed if the state file has
one, else minted) and persisted immediately — an unpersisted seed would
re-roll cohorts every process. Safe to ship in the same release as the
first canaries: no prior release emitted canary properties, so the
bucketing-unit change is unobservable.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The "what cannot be fixed" framing predated the state-file rollover: the
drift still exists, but a re-rolled install no longer re-enters a failed
path, and install_predecessor_found splits the drift into recoverable vs
unrecoverable. Window-length policy comes from the residual, not raw
turnover.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributors in the wild have no access to the project's analytics backend,
so the contributing doc must not point them at it: drop the internal
dashboard link, the backend-specific SQL blocks, and the vendor naming.
The calibration check definitions stay (public, fixed terms of the
experiment); the queries live with the dashboard tiles that run them.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Telemetry carries the assignment but not the decision reason, so a manual
HF_CANARY_* toggle mid-window reads as a cohort flip. Rule it out before
treating a small-nonzero stability read as a mechanism bug; a reason
property is deliberately deferred until the check actually comes back dirty.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
As written, check 4's condition (dual-surface AND value flipped anywhere)
was a strict subset of check 3's (value flipped) — if check 3 read zero,
check 4 was vacuously zero and added no independent signal. Compare the
value each surface actually reported instead, and state plainly that any
disagreement is also a check-3 flip: this check's job is attributing such
a flip to binding divergence. Matching fix applied to the live PostHog
tile (insight jQi7QdW1, dashboard 1918875).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Verified against packages/*/src env reads: 57 distinct HF_*/PRODUCER_*
toggles pre-existing this branch (the raw grep said 59, but two of those
are HF_CANARY_TEST_* fixtures introduced by this branch's own tests).
The number is cited externally now, so it should match what the repo
actually has.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The four pre-registered checks now exist as PostHog tiles. Without the link
the doc describes queries someone has to re-type; with it the doc and the
dashboard are one artifact.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Registers two canaries that gate nothing — `calibration-10` (10%) and
`calibration-50` (50%) — so the rollout mechanism can be proven against real
traffic before any real feature depends on it. Zero behavioural risk: they are
read by nothing.
They answer what the unit tests structurally cannot. The tests bucket
generated UUIDs and weight every install equally; real render volume is
heavily skewed toward a few heavy installs, and real install ids churn (~25x
more distinct ids over 30 days than in any single day on the desktop render
population).
Four checks, pre-registered in the docs so the read is not post-hoc:
1. ACCURACY — does 10% land at 10%, install-weighted AND event-weighted?
2. DRIFT — how fast does CUMULATIVE exposure climb above target as ids churn?
The instantaneous share is flat by construction; the set of installs
enrolled at some point is not.
3. STABILITY — does any install ever change cohort? Must be zero. Percentages
are held FIXED for the window precisely so a flip is unambiguously a bug;
during a real ramp a false->true flip would be correct instead.
4. CROSS-SURFACE — do the CLI and Studio bindings agree for the same install?
A CLI-launched Studio adopts the CLI id, and 16,961 installs currently
share an id across both surfaces, so this is measurable.
Plus an independence check: overlap between the two calibration canaries
should be ~p1*p2 (~5%), not ~min(p1,p2) (~10%, which would mean every canary
lands on the same unlucky cohort).
The docs also record what calibration CANNOT fix: per-install cohorts never
flip, but a person who wipes their config gets a new id and a fresh roll.
Preventing that needs stable identity across resets, and both candidates were
rejected — hardware fingerprinting correlates the cohort with hardware (fatal
for a rendering experiment, and it survives uninstall) and account identity
covers only ~3.6% of local rendering installs. The drift is therefore a
measured, accepted limit, and the point of calibrating is to size it and pick
canary window lengths accordingly.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Replaces the single `canaries: "a,b"` telemetry property with PostHog's own
flag shape, one property per registered canary:
$feature/canary-de-parallel-router: "true" | "false"
PostHog treats `$feature/<key>` as a first-class flag property, so breakdowns,
funnels split by cohort and the experiment surfaces work on a canary with
nothing configured server-side. The decision still happens locally: the render
path forbids render-time network calls, behaviour must not depend on analytics
being reachable, and neither the CLI nor Studio ships posthog-js (both
hand-roll a batch POST, so there is no SDK to evaluate a real flag with).
Decide locally, analyse natively.
Two decisions worth recording:
- BOTH ARMS ARE EMITTED. A non-enrolled install reports "false" rather than
omitting the property. Absent means "this build predates the canary", which
is a different fact from "this install is control" — collapsing them makes a
ramp unreadable, because you cannot separate a control group from an old
version.
- KEYS ARE NAMESPACED with a `canary-` infix. A real PostHog flag namespace
already exists in this project, owned by the web app (`enable-chat-tab`, set
by posthog-js from `$lib=web` events). Namespacing guarantees a canary key
can never alias a real flag key and have the two fight over one property.
Values are the strings "true"/"false" to match how PostHog records boolean
flag values, so the property is directly comparable to a real flag.
98 core / 1437, 166 cli / 2194, 269 studio / 2982 green; tsc clean across all
three packages.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds the Studio (browser) binding so a canary can span the CLI and the editor,
and fixes a bundling mistake the studio test suite caught.
## The binding
Same public API as the CLI — `isCanaryEnabled("name")` — so a call site reads
identically whether it runs in Node or the browser. Three inputs differ:
- UNIT ID: `resolveStudioDistinctId()`, which already adopts
`window.__HF_CLI_DISTINCT_ID` when the CLI launched Studio. A CLI-launched
Studio therefore lands in the SAME cohort as the CLI: a rollout spanning
render and editor is coherent for that user instead of enrolling their
terminal but not their editor. A test pins that the id is passed through
unmodified — prefixing or re-hashing it would silently break that parity.
- OVERRIDE: no `process.env` in a page, so `?hf_canary_<name>=on` mirrored
into sessionStorage. Session scope is deliberate. A URL is the right carrier
(shareable — "support: open this link"), but persisting a URL-borne override
to localStorage would let one click silently pin a browser into a cohort
forever, long after anyone remembers why. Closing the tab is the reset;
`=reset` clears it explicitly.
- EXCLUSION: `navigator.webdriver` stands in for the CLI's `is_ci`. Automated
browsers mint a fresh localStorage id per run, so they would hop cohorts
between runs — noise in the signal, nothing learned about real users. An
override still reaches them, which is how you test a canary under Playwright.
Studio's `trackEvent` now attaches `canaries` to every event, mirroring the CLI.
## The bundling fix
Importing the `@hyperframes/core` barrel into studio browser code broke two
unrelated hook test files with an esbuild TextEncoder invariant violation. The
barrel re-exports the whole core surface (parsers, lint, studio-server), so it
drags a Node-oriented dependency graph into a browser bundle — the test
failure was the symptom, the bundle bloat was the bug.
`@hyperframes/core` now exposes `./canary` and `./canary-registry`, declared in
packages/core/package-subpaths.json (the generated source of truth for exports —
hand-editing package.json is reverted by the sync script) and marked
`environments: [browser, bun, node]`. Both the studio AND cli bindings import
the leaf modules; the CLI gets the same benefit for a different reason, since
this resolves on the startup path — the reason the producer is lazily loaded.
Verified: the two hook files pass again; 269 studio files / 2982 tests, 98
core / 1433, 166 cli / 2194 green, `bun run lint` clean including the subpath
check. Fault-injection confirms both design decisions are pinned — swapping
session for local storage fails the scope test, prefixing the unit id fails the
CLI/Studio cohort-parity test.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Follow-up to the canary primitive.
Telemetry: every event now carries a `canaries` property listing the cohorts
the install is enrolled in, attached in `trackEvent` so it lands on ALL
events rather than renders only — a staged rollout is only as useful as the
ability to split any metric by cohort. Resolved after the shouldTrack guard,
so opted-out installs never pay for it, and omitted entirely (not null or "")
when the install is in no canary, since PostHog treats those as real values.
Test hardening, after validating the shipped code against 60k synthetic and
101 real fleet install ids:
- Pin FNV-1a against canonical vectors, AND assert the shipped canaryBucket
actually uses that hash. Without the second assertion the first is
tautological — it would only prove the test's own copy is correct while
canary.ts drifted to a different hash, silently reshuffling every live
cohort. Fault-injection confirms only this assertion catches a hash change;
the distribution tests stay green because a perturbed hash is still
well-distributed.
- Tighten the share test from a 0.6x-1.4x band to +/-1 percentage point.
Measured error was 0.16pp at n=60k, so the old band would have passed a
badly skewed hash.
- Add chi-square uniformity across all 100 buckets (chi2 89.0 vs 148.2
critical at p=0.001). A lumpy hash yields roughly the right total share
while overloading some buckets, so the share test alone cannot catch it.
- Assert N concurrent canaries enrol binomially rather than in lockstep:
8 canaries at 10% put ~43% of installs in none and zero in all eight,
matching binomial(8, 0.1). Correlated slices would put ~10% in all eight.
Also verified 88,443 of 88,448 fleet install ids are well-formed UUIDs; the
5 that are not fail closed, which is the intended direction.
Docs: docs/contributing/canary-rollouts.mdx, registered in docs.json (an
unregistered page is invisible in the nav).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds a reusable staged-rollout primitive so a change can ship to a stable
slice of installs instead of all-or-nothing.
The gap it fills: the repo carries ~49 HF_*/PRODUCER_* booleans and every one
is binary — a feature is either off (and therefore unexercised on real
traffic) or on for everyone (and therefore a fleet-wide bet). The
parallel-drawElement router sat in that gap for weeks: default-off produced
almost no signal, and flipping it default-on would have exposed 100% of
eligible installs at once.
Shape:
- packages/core/src/canary.ts — pure evaluator. No fs, no network, no
`process`; the caller supplies the unit id and overrides, so it imports
cleanly into the CLI, producer, engine, studio-server, the browser-side
studio bundle and the embeddable player. FNV-1a rather than node:crypto for
the same reason.
- packages/core/src/canaryRegistry.ts — every rollout in one table (name,
percentage, owner, description, sunsetAfter), so "what is rolling out, to
whom, owned by whom" is answerable without grepping 49 env vars.
- packages/cli/src/telemetry/canary.ts — supplies the three things only the
CLI knows: anonymousId, the HF_CANARY_<FEATURE> override, and is_ci.
Day-to-day API is `isCanaryEnabled("name")`.
Three properties the tests pin, because getting them wrong is subtle:
- Slices are INDEPENDENT per feature: the bucket hashes `feature:unitId`, not
the id alone. Bucketing on the id would hand every concurrent experiment to
the same unlucky cohort and make two rollouts unreadable apart.
- Ramping is INCLUSIVE: `bucket < percentage`, so widening 10 -> 25 keeps the
original cohort and before/after comparisons survive the ramp.
- It fails CLOSED: no unit id, unknown name, or CI install means not enrolled.
A canary exists to bound blast radius, so "we don't know who this is" must
never mean "enrol everyone".
Registry entries also carry a sunset date, and a test fails once one is past
due — a canary that outlives its rollout is a permanent fork of the product
with none of the review a permanent fork would get.
Ships with de-parallel-router registered at 0%: inert, and ready to ramp in a
patch release once #2840 lands.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The old demo argued that a locked frame crops the story. The sharper claim is
what a cut throws away: a move carries the spatial relationship between two
shots, a cross-fade carries none.
Five stages each get their own colour and the halves diverge on how the next
colour arrives -- in place, or by travelling to it. 30-47% of pixels differ,
against 3% for every version where the WITHOUT half moved the content.
That 3% is the trap now documented in the rule: translating the scene and
translating the camera are the same transform, so sliding content past a fixed
frame is not a static comparison, it is the same shot rendered twice.
Embed is cache-busted to -v2; docs images carry immutable year-long cache, so
reusing the filename would serve returning visitors the old render.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CodeQL (incomplete multi-character sanitization, code-scanning/803) on
the script-stripping regex added in c61a24b51 — a failing check, and
correct: a single replace can reform the pattern it just removed, since
`<scr<script>ipt>` leaves a whole `<script>` behind.
The security framing does not apply — the stripped string is counted and
discarded, never rendered, inserted, or served — but the incompleteness
is real for this use: a reformed tag survives into the match pass and
perturbs the element count the routing gate reads. Suppressing a gate
over a technicality when the fix is four lines is the wrong trade.
Now loops to a fixed point. Terminates by construction: each iteration
either strictly shortens the string or changes nothing and exits.
Regression covers the reform case and an unterminated `<script>` that
must not spin; fault injection confirms the reform test fails under the
old single pass.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review findings on #2891. Two of them bite directly on this PR's own
purpose — making the fleet element-count distribution readable — so they
are fixed rather than noted.
countElementTags counted `</` + letter anywhere, including inside inline
JS. A compiled comp containing `const h = "</div>"` or a template literal
building `</span>` inflated the count once per occurrence. Compiled comps
embed large inline scripts, so the bias is systematic, not noise, and it
lands entirely on the ~83% of renders with no probe session — precisely
the cohort this PR exists to characterize. Script and style bodies are
now stripped before matching; losing their own closing tags costs 1-2
counts against a threshold in the thousands.
The new elementCount fell back to 0 when its page.evaluate threw,
following the tweenCount pattern beside it. For this field that pattern
is wrong: evaluate failures concentrate on the huge-DOM compositions the
field is meant to observe, and a 0 there is indistinguishable from a
legitimately empty comp, so the fleet p50/p99 would absorb both silently.
It is now undefined on failure, the INIT console line omits the token
entirely rather than emitting a zero, and the parser reports absent —
mirroring the live/static provenance split the routing resolver already
uses.
Also documented: the "every render reaches this path" claim holds only
for renders that survive to end of init, so the tail is survivor-biased
and should be read as a lower bound; and the two element-count fields now
say plainly which is which — composition_element_count gates routing,
observability_init_element_count is the observational counterpart — so
the follow-up analysis can't query the wrong one.
Nits: envInt is integer-only per its name, both live-DOM reads use
getElementsByTagName (live collection length, no NodeList materialized on
the 40k-node tail), and the attribution block notes that it runs with
routing off by design.
Fault injection confirms the new tests bite: disabling script stripping
fails 4, and the zero-vs-undefined case is pinned separately.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
