feat(producer): lower parallel-DE router floor to 700 frames + power-state telemetry

[vanceingalls]

What

1. HF_DE_PARALLEL_MIN_FRAMES default 2000 → 700 — the frame floor above which the (default-off) HF_DE_PARALLEL_ROUTER routes auto multi-worker renders to verified parallel drawElement streaming.
2. on_battery / low_power_mode added to render_complete and render_error telemetry (pmset-based, darwin-only, sampled per event, null-safe).

Why 700

The 2000 default was an explicit safe-high guess ("targets the long tail the inversion's benchmark didn't cover"). A controlled crossover sweep re-calibrated it: fixed content-per-frame synthetic comps, three profiles × {350, 700, 1200, 2000, 3000}f × {single, par2, par3} × 3 reps, with resolved worker counts and capture modes verified per run (earlier sweeps were invalidated by silent worker clamps and a CDN-fetch confound — the harness now records both).

par3 vs single, within-regime, total wall-clock:

frames	capture-heavy	DOM-init-heavy	subcomp-init-heavy
350	+5%	+15%	+10%
700	+11%	+22%	+19%
1200	+19%	+27%	+24%
2000	+26%	+28%	+31%
3000	+31%	+28%	+34%

The subcomp profile was built specifically to reproduce the "workers re-pay init" failure the 2000 floor guarded against: 24 external sub-compositions, 92k tweens, ~2.5s pollSubCompositionTimelines per worker. It still parallelizes +19% at 700f — workers initialize concurrently, so duplicated init costs CPU, not wall-clock. Below ~700f the win thins toward +10% while paying three hardware-GPU browsers, so a floor remains. par2 lost to par3 in every cell of every profile; the router's existing 3-worker pin is confirmed unchanged.

Benchmarked on an M4 Pro MacBook — deliberately, since the DE path only engages on macOS+hardware-GPU and the real fleet is laptops.

Why the power-state telemetry

Those same sweeps caught the SAME render flipping between ~9.6 and ~17.2 ms/frame with no existing telemetry able to explain it (not load, not thermal) — laptop power management is a ~1.8× perf dimension we currently can't segment by. The router soak that gates flipping HF_DE_PARALLEL_ROUTER on needs this field to interpret wild-fleet perf: renders now report whether they ran on battery and in Low Power Mode.

Safety

• Router stays default-off; this only tunes what it does when enabled, and the frame-weighted stake is real: the 700–2000 band is ~29% of all DE capture wall-clock currently left unparallelized.
• Self-verify + screenshot-fallback net unchanged; 24 GB RAM floor and heap auto-scaler clamps unchanged (small laptops fail safe to fewer workers).
• Tests: power-state parsing/sampling unit tests (darwin, off-darwin, pmset-failure); 146/146 orchestrator tests pass.

🤖 Generated with Claude Code

[miga-heygen]

SSOT -- new surface inventory

Symbol	Kind	Scope	Notes
PowerState	interface	cli/telemetry/system.ts	{ on_battery: boolean | null, low_power_mode: boolean | null }
getPowerState()	exported fn	cli/telemetry/system.ts	Samples pmset on darwin, null elsewhere
parsePmsetPowerSource()	exported fn	cli/telemetry/system.ts	Exported for test seam only
powerStateFields()	private fn	cli/telemetry/events.ts	Converts PowerState nulls to absent-property undefined for PostHog
parallelStreamingAvailable	new required param	shouldPreferParallelDrawElement	Gates the router on streaming actually being viable
HF_DE_PARALLEL_MIN_FRAMES default	changed	executeRenderPipeline	2000 -> 700

No duplicated decisions across files. The 700 literal appears twice in renderOrchestrator.ts (one for undefined/empty env var, one for NaN/Infinity fallback) -- same pattern as the prior 2000. A named const would be tidier but it is localized to a single validation chain and not a real SSOT risk.

Comments and docstrings updated to reflect the new calibration rationale. No stale references to the old 2000 threshold remain in the touched files.

CI status

Check	Result
Build, Lint, Format, Semantic PR title	pass
Producer: unit + integration tests	pass
CLI smoke, npx shim (all 3 OS)	pass
CodeQL, Fallow audit, SDK, Studio, Player, Runtime contract	pass
Regression shards (7/9 complete)	pass (2 still running)
Typecheck	FAIL

Blocking: Typecheck failure -- missing required gpu in test

The two new trackRenderComplete calls in the power-state opt-out test are missing the required gpu: boolean property:

events.test.ts(728): error TS2345: Property 'gpu' is missing in type
  '{ durationMs: number; fps: number; quality: string; docker: false; }'
events.test.ts(741): same

Both calls need gpu: false (or gpu: true) added:

// line ~728
trackRenderComplete({ durationMs: 1, fps: 30, quality: "high", docker: false, gpu: false });
// line ~741
trackRenderComplete({ durationMs: 1, fps: 30, quality: "high", docker: false, gpu: false });

This is the only CI failure.

Side-effect audit: power-state sampling

The getPowerState() implementation is solid:

1. Platform gate: early-returns { null, null } off-darwin. No guessing on Linux/Windows.
2. Two independent try/catch blocks: pmset -g batt for power source, pmset -g for low-power mode. One failing does not poison the other.
3. Timeout: both execSync calls carry { timeout: 2000 }. pmset is near-instant (<10ms), so 2s is a generous safety net without risking a hang.
4. Opt-out guard: powerStateFields() checks shouldTrack() before calling getPowerState(), preventing subprocess spawns for opted-out installs. There is a dedicated regression test for this ("does NOT spawn pmset when telemetry is disabled"). This is especially important because the spread happens at the call site, BEFORE trackEvent's own shouldTrack guard.
5. Null-to-undefined conversion: power.on_battery ?? undefined correctly converts null (undetectable) to property-absent for PostHog, which treats null as a real value. Clean.

No concerns here.

parallelStreamingAvailable gate

The concept is sound: the router's entire value is the streaming path, so pinning 3 workers without streaming benefit is pure cost. Verified that shouldUseStreamingEncode on main already accepts the 5th forceParallelStream parameter (default false), so the call site passing true is correct. The gate checks the duration cap, format exclusions, and streaming config -- all the reasons streaming might be off despite the router wanting it.

Test coverage: the new "withholds the bet when parallel streaming can't run" test verifies the gate in shouldPreferParallelDrawElement. The call-site wiring (computing parallelStreamingAvailable from shouldUseStreamingEncode) is integration-level and covered by the existing router integration tests.

Spec vs implementation

PR claim	Verified?
Lowers HF_DE_PARALLEL_MIN_FRAMES default from 2000 to 700	Yes -- both fallback values changed
Adds on_battery/low_power_mode to render telemetry	Yes -- spread into both render_complete and render_error events
pmset-based, darwin-only, sampled per event	Yes -- platform gated, not cached, per-event call
Router stays default-off	Yes -- HF_DE_PARALLEL_ROUTER === "true" check is unchanged

All claims match the implementation.

Verdict

Changes Requested -- fix the Typecheck failure (add gpu to both test calls), then this is ready. The power-state telemetry is well-implemented and the router threshold change is well-motivated by the benchmark data.

-- Miga

[james-russo-rames-d-jusso]

Reviewed at cddc90ae — restacked mid-review from b99c8038 with cddc90ae3 fix(cli): add required gpu prop to power-state test calls, closing what I had been going to flag as the Typecheck blocker.

Floor drop 2000→700 is the meat; power-state telemetry (powerStateFields() at packages/cli/src/telemetry/events.ts:20-27) is a clean spread with the shouldTrack() guard correctly landed AT the call site so opted-out installs no longer pay two pmset spawns per render. Router-pin fix classified as NARROW — new parallelStreamingAvailable field on the eligibility predicate at renderOrchestrator.ts:1334-1343; pin itself (ROUTER_WORKER_COUNT = 3 at line 2537-2544) is unchanged. Battery telemetry emitted on both render_complete (events.ts:301) and render_error (events.ts:382).

Blockers

None at the current head. The Typecheck failure on the prior head (b99c8038, run 30340557122) — 2 new test call sites at events.test.ts:675, 688 in the describe("power-state sampling respects the telemetry opt-out") block missing the required gpu: boolean field on the payload — has been self-fixed by cddc90ae3 before I posted this review. The commit message even calls out "the fix already existed on the stacked branch, so only this base branch was broken" — noted.

Concerns

1. Empirical harness referenced in code but missing from the tree. renderOrchestrator.ts:2329-2340 cites the block-level rationale for the 700f floor and points to plans/drawelement-fast-capture/de-crossover-bench.sh as the source of the +17–21% at 700f rising to +28–34% at 3000f win-margin numbers and the 24-sub-composition, ~2.5s pollSubCompositionTimelines per worker profile. But find /tmp/pr-2838/plans shows only 2026-07-13-studio-flat-inspector-validation-session.md — no drawelement-fast-capture/ directory, no de-crossover-bench* anywhere. A measurement claim (crossover sweep) needs a reproducible measurement — either commit the harness + a result artifact under plans/drawelement-fast-capture/, or drop the reference and let the comment stand on its own summary.

2. 350f-crossover claim vs 700f floor: no per-profile decomposition of the 350–700f margin band. Same code block claims par3 beats single at every size ≥350f but the floor is 700 because "below ~700f the win thins toward ~+10% while still paying 3 hardware-GPU browsers." That 350-frame margin is where a bimodal profile could invert — the comment gives one aggregated range (+17–21% at 700f) but no per-profile floor. Specifically, the "24-sub-composition init-expensive" profile called out at renderOrchestrator.ts:1300-1302 — which was explicitly built to threaten the drop — has zero per-size numbers in the 350–700f band. Worth asking: what's the worst per-profile 350f margin, and does 700 cover the pessimistic case with margin, or was it chosen to make the aggregate look good?

3. Stale test rationale + no coverage on the new default. renderOrchestrator.test.ts:1912-1915 still says benchmark: real-work comps clear 1.25x at ~2,000+ frames — pre-recalibration language. The fixture at renderOrchestrator.test.ts:1839 still hard-codes minFrames: 2000, and no test asserts the new 700 default. Minimum ask: update the comment to match the current threshold + rationale. Better ask: add table-driven cases in the 350–700f band that lock the crossover-vs-floor decision in a place a reviewer can find later — shouldPreferParallelDrawElement({...eligible, totalFrames: 700}) === true and ... totalFrames: 699 === false at minimum.

Nits

• No TODO/FIXME/NOTE markers in the touched files — clean.
• No sibling parallel-DE floor constants that would need to move with this one (sibling HF_DE_SINGLE_MIN_FRAMES=900 at renderOrchestrator.ts:2303 intentionally crosses per docstring at 1314; MIN_FRAMES_PER_WORKER=30 at parallelCoordinator.ts:131 is per-worker chunking, orthogonal).

Questions

None.

What I didn't verify

• CI at the new head cddc90ae — the fix landed after I pulled logs; trusting Typecheck will be green on rerun given the diff is a strict subset of what CI already reported as the failing sites.
• Whether dropping the frame floor changes the memory-pressure profile enough to matter (larger fraction of small renders will now spin 3 hardware-GPU browsers — HF_DE_PARALLEL_MIN_MEM_MB=24576 is orthogonal per the code but the interaction wasn't checked).

— Review by Rames D Jusso

[vanceingalls]

Fixed in cddc90ae3.

gpu: false added to both new trackRenderComplete calls in the power-state opt-out test. Root cause of the split: the fix already existed on the stacked branch (#2840), so the stack typechecked while this base branch did not — exactly the seam @miga-heygen flagged from the other side. It now lives here, where the gpu property originates.

Typecheck clean locally across cli/producer/engine; 39 telemetry tests pass.

On the 700 literal appearing twice — agreed it is the same shape as the prior 2000 (undefined/empty vs NaN/Infinity in one validation chain), so leaving it rather than growing the diff. Happy to extract a named const if you would rather.

[miguel-heygen]

Re-reviewed the authoritative six-file diff at 011f46bc1844f0bc2975170f3552c2671edf29b9 after the restack onto main.

The strongest part is the routing guard: parallelStreamingAvailable is enforced in the predicate (packages/producer/src/services/renderOrchestrator.ts:1370-1394) and computed at the only production caller with the actual 3-worker/forced-stream configuration (:2484-2492), so the 700-frame floor cannot pin three browsers when the streaming path is unavailable. The power telemetry is similarly fail-soft and privacy-aware: non-darwin returns null without spawning (packages/cli/src/telemetry/system.ts:217-238), the opt-out guard sits before the subprocess calls (packages/cli/src/telemetry/events.ts:20-26), and both success and error events receive the fields (:325, :405).

The prior Typecheck blocker is resolved at the correct test call sites. I ran the focused suites at this exact head: CLI telemetry 50/50 and producer orchestrator 147/147 passed. Build, Typecheck, Test, producer unit/integration, CLI smoke, CodeQL, lint, and format are green; the fresh Windows and regression jobs are still pending, so merge should wait for those required checks.

I am not duplicating Rames’s existing non-blocking findings: the code comment still references a benchmark harness absent from the tree (renderOrchestrator.ts:2444), and the predicate test still carries the old 2,000-frame rationale/default rather than pinning 699/700 (renderOrchestrator.test.ts:1840,1913-1915). One additional coverage nit: the new telemetry test asserts the fields on render_complete, but not independently on render_error; the production spread is present at events.ts:405, so this is not blocking.

Verdict: APPROVE
Reasoning: The behavior change is correctly gated at the sole production seam, power sampling fails soft and respects telemetry opt-out, the previous type failure is fixed, and focused tests pass; only already-recorded reproducibility/coverage follow-ups and still-running CI remain.

— Magi