fix(engine): realistic worker memory budget + sizing/feedback telemetry

[vanceingalls]

Summary

Addresses the auto-scaler heap-OOM report from PRINFRA-300's cluster (0.7.66, darwin/arm64 18c/24GB: auto-selected 6 workers → FATAL ERROR: Reached heap limit Allocation failed), plus the observability gaps that made that cluster expensive to triage.

1. Realistic per-worker memory budget

MEMORY_PER_WORKER_MB was 256 — ~6× under a real Chrome-under-capture footprint (SwiftShader compositor + raster threads + parent-side frame buffering). Memory-constrained hosts could overcommit (the wild 16GB black-slab report; the 24GB heap-OOM). Now 1536. Effect: memory becomes a real constraint on ≤8GB hosts (2–3 workers instead of 6); unchanged on ≥24GB hosts where CPU/config bind first.

2. Heap-aware sizing — advisory + loud warning, deliberately NOT enforced yet

The 24GB OOM wasn't system memory — it was Node's default ~4GB V8 heap. New computeWorkerSizing computes a heap budget ((heap_size_limit − 1GB reserved) / 640MB per worker) and:

• logs an actionable warning when the chosen count exceeds it (names NODE_OPTIONS=--max-old-space-size=8192 and the safe --workers N),
• emits it to telemetry (below).

Not enforced because the 640MB/worker figure is derived from one field report — enforcing a guessed budget could silently cut worker counts fleet-wide (Node's default heap limit varies by machine class). The telemetry shipped here (workers_heap_based vs workers vs actual OOM outcomes) is what validates the constant; enforcement is a one-line follow-up once the fleet data supports it.

3. Worker-sizing provenance telemetry

computeWorkerSizing returns the full decision record — boundBy (cpu | memory | frames | max_workers | contention | min_parallel_floor | explicit | too_few_frames), each constraint's value, heap limit — threaded engine → RenderPerfSummary.workerSizing → render_complete props:
workers_bound_by, workers_cpu_based, workers_memory_based, workers_heap_based, workers_frame_based, workers_heap_limit_mb, workers_exceed_heap_advisory.

Fleet-wide "why N workers?" becomes a query instead of a repro.

4. Tie CLI feedback reports to their PostHog telemetry rows

Triaging PRINFRA-300 meant reverse-identifying reporters from hardware fingerprints (os + cores + RAM + version). Now:

• The CLI keeps a ring of the last 5 render job ids (recentRenders in the config file, success + failure).
• hyperframes feedback generates a feedback_id and appends compact join keys to the env string that rides with the forwarded report: fid=<feedback_id> tid=<install distinct_id> renders=<id1>,<id2>! (! = failed render).
• The same keys go on the PostHog survey sent event (feedback_id, recent_render_ids).

A wild report now resolves to the exact render_complete/render_error rows of the renders it describes (render_job_id / observability_render_job_id join), in either direction. All ids are the same anonymous UUIDs the install already sends to PostHog — no new information class leaves the machine.

Test plan

• New computeWorkerSizing tests: parity with calculateOptimalWorkers, boundBy labels (explicit / too_few_frames / contention), heap-advisory flag invariant — engine suite 43/43
• New buildTelemetryJoinKeys tests incl. env-cap budget — CLI suites 48/48 + config 8/8
• Producer captureCost/perfSummary suites 15/15
• tsc --noEmit clean on engine, producer, cli; oxlint + oxfmt --check clean

Related: PRINFRA-300 (cron 81 fault mode #8), PRs #2679/#2681/#2722.

🤖 Generated with Claude Code

[miga-heygen]

SSOT Review: fix(engine): realistic worker memory budget + sizing/feedback telemetry

PR #2723 — 11 files, +387/-23

---

SSOT inventory

Concept	Source of truth	Consumers	Verdict
Worker sizing decision	computeWorkerSizing	calculateOptimalWorkers (legacy wrapper), resolveRenderWorkerCount	Single owner
WorkerSizing / WorkerSizingBound types	parallelCoordinator.ts	captureCost.ts, perfSummary.ts, renderOrchestrator.ts, events.ts	Single owner, exported via index.ts
System-memory budget	MEMORY_PER_WORKER_MB (1536)	computeWorkerSizing	Single owner
Heap budget constants	HEAP_RESERVED_MB (1024), HEAP_PER_WORKER_MB (640)	computeWorkerSizing	Single owner
Recent-render ring	HyperframesConfig.recentRenders	recordRecentRender, buildTelemetryJoinKeys, readConfig	Single owner
Ring cap	MAX_RECENT_RENDERS (5)	recordRecentRender, readConfig	Single owner
Telemetry join keys	buildTelemetryJoinKeys	feedback.ts command	Single owner
RecentRenderRecord type	config.ts	feedback.ts, readConfig	Single owner

What I checked

1. calculateOptimalWorkers backward compatibility. Now a thin wrapper: return computeWorkerSizing(...).workers. The test explicitly verifies: expect(sizing.workers).toBe(calculateOptimalWorkers(900, undefined, config)). All existing callers of calculateOptimalWorkers get identical results. ✓

2. Logic refactor correctness. The sizing logic is a 1:1 restructure of the old function. CPU, memory, and frame calculations moved to the top (before early returns) so the provenance record always has all constraint values. Heap calculation added (new). The totalMemoryMB → totalMemoryMb rename is consistent throughout. ✓

3. boundBy attribution correctness:

   • "explicit" → requested !== undefined. ✓
   • "too_few_frames" → totalFrames < MIN_FRAMES_PER_WORKER * 2. ✓
   • "cpu" / "memory" / "frames" → whichever of the three produces the min (optimalBound). ✓
   • "max_workers" → when effectiveMaxWorkers < optimal and it caps finalWorkers. ✓
   • "min_parallel_floor" → when the min parallel floor raises the count above the optimal. ✓
   • "contention" → adaptive CPU contention scaling overrides to reduce count. ✓

   Tests cover: auto (matching legacy), explicit (clamped to 24), too_few_frames (30 frames → 1 worker), contention (high capture cost), and heap advisory flag. ✓

4. exceedsHeapAdvisory — advisory only, deliberately not enforced. Set via workers > heapBasedWorkers in the finish() helper. The warning in captureCost.ts fires when true, naming both knobs (NODE_OPTIONS=--max-old-space-size=8192 and --workers N). The PR body and code comments are explicit: the 640MB/worker figure comes from one field report, enforcement deferred until fleet telemetry validates it. Correct approach. ✓

5. Sizing provenance threading (end-to-end):

   • resolveRenderWorkerCount gains onSizing?: (sizing: WorkerSizing) => void callback
   • renderOrchestrator.ts captures it: (sizing) => { workerSizing = sizing; }
   • Threaded into buildRenderPerfSummary({ ..., workerSizing })
   • Stored in RenderPerfSummary.workerSizing
   • Consumed by trackRenderMetrics which emits 7 individual fields to PostHog

   Clean threading, no duplication, undefined when htmlInCanvas/low-memory pins short-circuit sizing. ✓

6. Telemetry field mapping consistency:
   workersBoundBy → workers_bound_by, workersCpuBased → workers_cpu_based, workersMemoryBased → workers_memory_based, workersHeapBased → workers_heap_based, workersFrameBased → workers_frame_based, workersHeapLimitMb → workers_heap_limit_mb, workersExceedHeapAdvisory → workers_exceed_heap_advisory. All 7 mapped consistently through the chain. ✓

7. Recent-render ring:

   • recordRecentRender(id, ok) — read-modify-write with readConfigFresh(), appends entry, slices to MAX_RECENT_RENDERS (5). Called on both success (line 1384) and failure (line 1344, guarded by job?.id). ✓
   • readConfig validation — type-guard filters each entry (typeof id === "string", typeof at === "string", typeof ok === "boolean"), slices to cap. Defensive against corrupt config. ✓
   • Single constant MAX_RECENT_RENDERS used in both write and read paths. ✓

8. Feedback join keys:

   • buildTelemetryJoinKeys is a pure function producing fid=<uuid> tid=<install-id> renders=<id1,id2!>. The ! suffix marks failed renders. ✓
   • feedbackId = randomUUID() — fresh per submission, used as both PostHog feedback_id prop and fid= join key in the env string. ✓
   • Size cap test: full ring of UUID render IDs stays under 400 chars (env field caps at 500, doctor summary ~100). ✓
   • recentRenderIds on PostHog → comma-joined string (EventProperties are scalars). ✓

9. Sentinel-value scan:

   • config.recentRenders ?? [] — handles undefined. ✓
   • input.recentRenders?.length — handles undefined/empty. ✓
   • perf?.workerSizing?.boundBy — handles undefined perf/sizing chain. ✓
   • job?.id guard in handleRenderError — handles error before job creation. ✓

Blocking SSOT issues

None found.

Verdict

Approve. The refactor from calculateOptimalWorkers → computeWorkerSizing is a clean extraction that preserves all existing behavior while adding full sizing provenance. The MEMORY_PER_WORKER_MB 256→1536 fix directly addresses the heap-OOM field report, and the heap advisory is correctly deferred to advisory-only until fleet telemetry validates the 640MB/worker constant. The feedback join keys (fid/tid/renders) turn wild bug reports into exact telemetry lookups — nice observability win.

--- Miga

[james-russo-rames-d-jusso]

Reviewed at 12e599a6bac7dea5feec59c71dd0e5aeecab354e. Four discrete pieces: MEMORY_PER_WORKER_MB bump (256→1536), advisory heap-based sizing + provenance telemetry, and CLI-side render-ring + feedback join keys. All coherent and readable.

CI is red on Producer: unit tests — one pre-existing test needs updating to accommodate the new advisory warn firing on explicit --workers requests. That's a real blocker for merge (not a code correctness issue). Inline anchors below name the design decision.

Cross-cutting notes

Memory budget correctness. 256→1536 is a 6× increase, which sounds dramatic but is functionally invisible on ≥24GB hosts: memoryBasedWorkers on 24GB was 48 (max-workers-cap bound, no impact) and is now 8 (also caps at max-workers = 8, still no impact — only boundBy telemetry shifts from max_workers to memory). On 8GB the change is real (16 → 2 auto workers) and matches PR body's "2-3 workers instead of 6." This looks right.

Feedback join-key end-to-end. fid=<uuid> tid=<uuid> renders=<id>,<id>! lands on both the forwarded env string and the PostHog survey sent event as separate props. Both directions of the join (wild-report ↔ telemetry) resolve to the same id space. readConfig() in feedback.ts is a separate CLI process from writeConfig() in render.ts so the module-cache freshness concern is moot.

Failure-path coverage. handleRenderError calls recordRecentRender(job.id, false) (render.ts:1347) alongside trackRenderMetrics in render.ts:1387 for successes. Both paths write. The ! suffix on failed-render ids in buildTelemetryJoinKeys propagates. Good.

ponytail: marker on parallelCoordinator.ts:124 — existing codebase idiom (12+ uses, mainly in core/figma/*, cli/browser/manager.ts, etc.), not a stray. Ignore any "what's that?" reaction.

What I didn't verify

• The HEAP_RESERVED_MB = 1024 figure — same "one field report" derivation as HEAP_PER_WORKER_MB per PR body. Fleet data will validate.
• Actual per-worker RSS under a real capture — trusting the 1536MB claim.
• End-to-end round-trip through PostHog on a real dashboard.

LGTM from my side on the shape once CI goes green and the test-vs-advisory design decision (inline above) is resolved. Stamp routing continues to sit with James.

— Review by Rames D Jusso

[james-russo-rames-d-jusso]

🔴 Blocker (CI): This advisory fires unconditionally on sizing.exceedsHeapAdvisory, which is set whenever workers > heapBasedWorkers — including on explicit --workers requests. The pre-existing test packages/producer/src/services/renderOrchestrator.test.ts:1097 (resolveRenderWorkerCount > respects explicit worker requests) asserts expect(log.warn).not.toHaveBeenCalled() on an explicit --workers 6 call, and now fails on CI (Producer: unit tests red) with the exact log line from this block. Design question: should the advisory fire on explicit requests too? PR body suggests yes — a user who set --workers 6 on a 4-worker-heap machine benefits from the same warning. If yes, update the pre-existing test to expect the advisory warn (toHaveBeenCalledOnce() with a matcher for the heap-limit message). If no, gate this if on requested === undefined before the warn. Either way, CI needs to go green before merge. — Rames D Jusso

[vanceingalls]

Fixed in c6462a0. Went with the gate, plus a structural fix for the flakiness class: the advisory now only fires for auto-sized renders (requestedWorkers === undefined — the field OOM was auto sizing, and explicit --workers N is the operator's own call), and emission moved OUT of resolveRenderWorkerCount to the orchestrator's onSizing consumer. That second part matters beyond this one test: resolveRenderWorkerCount's no-warn unit-test contract would otherwise depend on the test machine's actual V8 heap limit (the auto-request test at line 1230 would flake on small-heap machines even with the explicit gate). Sizing unit tests are now heap-independent by construction; the pre-existing tests pass unmodified (145/145).

[james-russo-rames-d-jusso]

🟠 Concern — advisory-fires wiring not test-locked. The engine test at parallelCoordinator.test.ts verifies the exceedsHeapAdvisory boolean is set correctly, but no producer-layer test asserts this log.warn actually fires with the expected message shape when the boolean is true. Given the whole point of shipping the advisory is to observe field impact before enforcing the 640MB budget, a silent regression of the message content or firing condition would defeat the validation loop. A one-shot test at the resolveRenderWorkerCount level (fake computeWorkerSizing returning exceedsHeapAdvisory: true → assert log.warn gets called with ~${heapBasedWorkers} in the message) would lock this down. — Rames D Jusso

[vanceingalls]

Done in c6462a0 — the warning is extracted into pure buildHeapAdvisoryWarning(sizing, requestedWorkers) and captureCost.heapAdvisory.test.ts locks it with a synthetic WorkerSizing (deterministic, no machine-heap dependence): message contains the chosen count, limit <N>MB, ~<heapBasedWorkers>, both remediation knobs; silent for explicit requests; silent under budget.

[james-russo-rames-d-jusso]

🟠 Concern — telemetry prop pass-through not test-locked. Fields workers_bound_by, workers_cpu_based, workers_memory_based, workers_heap_based, workers_frame_based, workers_heap_limit_mb, workers_exceed_heap_advisory land here from the summary → event body wiring, but nothing asserts they actually reach the emitted PostHog properties. Since the PR body's central thesis is 'fleet data validates the 640MB budget before enforcement,' a silent drop of these fields (e.g. a future rename that misses one hop) would silently invalidate the enforcement decision. Suggest a trackRenderComplete test with all seven fields set on the props → assert they appear on the mocked PostHog capture call payload. — Rames D Jusso

[vanceingalls]

Done in c6462a0 — events.test.ts now asserts all seven workers_* props reach the emitted render_complete payload, plus a second test locking the survey sent join keys (feedback_id, comma-joined recent_render_ids).

[james-russo-rames-d-jusso]

🟡 Nit — single-report derivation, worth an explicit revisit. PR body: 'the 640MB/worker figure is derived from one field report.' The comment above (L122-125) captures this well and the advisory-only stance is right. Suggest adding a small tracking issue / TODO to revisit + potentially enforce once workers_heap_* telemetry has ~2 weeks of fleet data — otherwise this constant risks sitting advisory-only indefinitely and the enforcement follow-up never lands. Not blocking. — Rames D Jusso

[vanceingalls]

Filed PRINFRA-341 (assigned to me) with the concrete decision queries and the three possible outcomes; TODO(PRINFRA-341) added on HEAP_PER_WORKER_MB in c6462a0.

[james-russo-rames-d-jusso]

🟡 Nit — read-modify-write lost-update on concurrent CLIs. The comment above (L112) acknowledges this: 'narrows, but does not eliminate, lost updates against a concurrent CLI process.' In practice the impact is small — a single render's id may be missing from the ring if two hyperframes render invocations write within the same read-modify window. Feedback still works because the join keys carry other ids in the ring plus feedback_id/tid. Fine as-is; noting for reader awareness in case a future user reports 'my last render isn't in the join keys.' — Rames D Jusso

[vanceingalls]

Agreed, leaving as-is — same read-modify-write posture as the existing trial counters, and a dropped ring entry degrades to a slightly less complete join (fid/tid still resolve the install).

[miga-heygen]

$REVIEW_BODY

[james-russo-rames-d-jusso]

Reviewed at c6462a0a2 (delta from R1 at 12e599a6b — one commit).

Every R1 concern addressed cleanly.

R1 finding	R2
Blocker: renderOrchestrator.test.ts:1097 red on explicit --workers advisory	✅ Fixed. buildHeapAdvisoryWarning gates on requestedWorkers === undefined — explicit --workers is now the operator's call and the advisory stays silent. Log block deleted from resolveRenderWorkerCount, moved to the orchestrator's onSizing consumer where it belongs. Test contract preserved.
Concern: no test locks the producer-layer heap advisory log.warn wiring	✅ Fixed. New captureCost.heapAdvisory.test.ts with 3 tests — message contents, silent-for-explicit, silent-when-within-budget. Extracted buildHeapAdvisoryWarning is pure so the tests don't depend on the host's V8 heap.
Concern: no test locks workers_* provenance pass-through to PostHog	✅ Fixed. events.test.ts gains "carries every worker-sizing provenance prop on render_complete" — asserts all 8 workers_* fields flow through trackRenderComplete. Bonus: also locks feedback_id + recent_render_ids on trackRenderFeedback.
Nit: HEAP_PER_WORKER_MB = 640 single-report derivation	✅ Named ticket. TODO(PRINFRA-341): decide enforcement after ~2 weeks of fleet soak. on the constant.
Nit: recordRecentRender concurrency	Already acknowledged.

Both refactor decisions are correctly motivated:

• Moving the advisory out of resolveRenderWorkerCount into the orchestrator's onSizing consumer keeps that function's "no unexpected warns" test contract independent of the test machine's actual V8 heap limit — the new docstring calls this out explicitly, which is exactly where that rationale should live.
• Making buildHeapAdvisoryWarning return undefined (rather than a shouldFire boolean + separate message) is the right shape: the caller just logs whatever comes back, and the firing condition sits with the message it gates.

Symmetry check: renderOrchestrator.ts:2484 passes job.config.workers to resolveRenderWorkerCount, and :2491 passes the same value to buildHeapAdvisoryWarning. Both callers see the same "did the operator explicitly ask" signal. ✓

CI all green. LGTM from my side. Stamps sit with James.

— Review by Rames D Jusso

[miguel-heygen]

Independent exact-head review at c6462a0a2.

Specific strengths:

• packages/producer/src/services/render/captureCost.ts:128 keeps the advisory condition and message in one pure helper; explicit --workers is deterministically silent and auto-sizing remains actionable.
• packages/producer/src/services/renderOrchestrator.ts:2482 computes sizing once, captures the provenance once, and emits at most one advisory from the same requested-workers signal.
• Rames D Jusso's R1 test gaps are closed at the exact head: warning behavior and all success-side sizing props are pinned.

Important (non-blocking): the new sizing provenance only reaches render_complete via packages/cli/src/commands/render.ts:1402. The failure path at packages/cli/src/commands/render.ts:1334 passes only raw options.workers plus existing observability, and trackRenderError at packages/cli/src/telemetry/events.ts:342 has no workers_heap_based, workers_heap_limit_mb, or workers_exceed_heap_advisory fields. For the motivating auto-sized heap OOM, the failure row therefore has workers=undefined and none of the advisory budget, so PRINFRA-341 cannot directly compare the proposed 640MB budget with actual OOM outcomes. This does not block the safer memory budget or warning, but carry the sizing record onto error telemetry before using the soak data to enforce a cap.

Verification: 241 focused tests passed (engine 42, producer 149, CLI 50); engine/producer/CLI typechecks passed; changed-file oxlint and oxfmt checks passed; all required GitHub checks are green.

Verdict: APPROVE
Reasoning: The R1 blocker and test gaps are resolved, behavior is covered, and CI is green. The remaining failure-side telemetry gap is important for the later enforcement decision but does not make this advisory/memory-budget change unsafe.

— Magi