Cut macOS widget memory and visualizer CPU - #49
Merged
Conversation
Native SDK branch macos-memory-shared-renderer-prep (600d6cf6) carries the autorelease-pool, analytic-rounded-clip, and Metal tiled-image memory work; the tiled-image change still needs live verification. The briefs scope the two follow-up passes: the error-propagation seams (partially landed via #43) and the receipt sweep over every numeric limit. The shared-renderer experiment plan lives in the handoff doc accompanying this work.
The branch is now the complete working set for the macOS memory / shared-renderer work: docs/macos-memory-handoff.md is the entry point (diagnosis so far, phased experiment plan, falsification gates), and myclock/ is the clean isolated-benchmark fixture the bakeoff harness and Phase 0/1 measurements use.
…partially rescinded Myclock on Mac15,6 (M3 Pro, macOS 26.5.2) measures 125 MB with 85 MB dirty owned-unmapped-graphics on this exact branch state, while bare Metal/IOSurface window probes on the same machine cost 9.7/8.5 MiB. The Air's Phase 0 probes were honest but window-scoped; the wall is in what Weaver's renderer does, not in the Metal entry fee. New evidence-led investigation order appended for the next agent.
The handoff doc keeps the correction (the finding that rescinds the Phase 0 gate); the investigation of what allocates the 85 MB is separate work for the Mac15,6 machine and lives in docs/gpu-ledger-wall-brief.md, with its own end state, evidence-led order, and stop rule.
Bare probes on Mac15,6 turn the ~85-96 MB owned-unmapped-graphics ledger on and off with no weaver code: sustained Metal command submission at >=1 Hz commits a ~95 MB per-process driver arena (presentation and window not required; offscreen clears reproduce it), reclaimed within seconds of submission silence. IOSurface-on-CALayer presentation with no in-process Metal never touches it, even at 60 Hz updates. Weaver pins the arena forever because renderFrame presents unconditionally at 60 Hz. Receipt, probe matrix, and the recommendation recorded in the brief; probe sources archived under .zig-cache/macos-memory/gpu-ledger-wall/. The shared-renderer decision stays with Dara per the brief's stop rule.
The handoff's stopped plan is un-stopped: the GPU-ledger brief's probes named the 85 MB (per-process Metal submission working set, paid by whoever submits, ~0 for device-less IOSurface widgets), and Dara re-approved the shared-renderer architecture on that receipt. Phase 1 is the next work; its gate expectation is restated for M3-class machines where the arena inflates the old Air-calibrated totals.
One file scoped to the 2026-07-30 session: the subscription-fee mental model, the probe method that produced the on/off receipt (sustained- cadence probes, exact-config matching, ledger-category cross-checks, PID discipline), what the finding settles, and the corrections it makes to prior session records. Durable state stays in the handoff and the GPU-ledger brief; this is the lessons file.
Phase 1 runs on Mac15,6 where the arena exists. The gate becomes categorical: no submission arena in any widget process (graphics ledger ~0), widget total judged as content cost. Widget side submits no Metal; event-driven presenting stays out of the spike; host shape expectations from the N-layer probe recorded.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR adds provider-backed signals and signal-bound text rendering, updates clock and audio examples, separates retained canvas updates from tree generations, adds runtime memory and tracing diagnostics, improves macOS measurement tooling, and records error, GPU, memory, and limit investigations. ChangesProvider signal support
Runtime rendering and measurement
Investigation documentation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Widget
participant Signal
participant Reconciler
participant CanvasRuntime
Widget->>Signal: read provider value
Signal->>Reconciler: notify subscribed text binding
Reconciler->>CanvasRuntime: project same-view canvas update
CanvasRuntime->>CanvasRuntime: update canvas_generation
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (7)
runtime/src/tree.zig-256-256 (1)
256-256: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve
canvas_generationin transaction snapshots.
cloneIntocopiesgenerationbut notcanvas_generation.resetEmptyalso does not initializecanvas_generation. A snapshot allocated bybeginBatchcan therefore restore an undefined canvas revision afterabortBatch.Copy the field in
cloneInto. Initialize it inresetEmpty. Add a canvas-only batch-abort test.Proposed fix
fn cloneInto(self: *const Tree, destination: *Tree) Error!void { resetEmpty(destination, self.allocator); destination.root = self.root; destination.generation = self.generation; + destination.canvas_generation = self.canvas_generation; destination.next_node_lifetime = self.next_node_lifetime;fn resetEmpty(tree: *Tree, allocator: std.mem.Allocator) void { tree.root = null; tree.generation = 0; + tree.canvas_generation = 0; tree.next_node_lifetime = 1;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@runtime/src/tree.zig` at line 256, Preserve canvas_generation across transaction snapshots: update cloneInto to copy the source canvas_generation, initialize canvas_generation in resetEmpty, and add a batch-abort test covering canvas revision restoration through beginBatch and abortBatch.docs/receipt-sweep-brief.md-37-39 (1)
37-39: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winSpecify the language for the shell fence.
markdownlint-cli2reports MD040 at Line 37. Mark this block as shell code.Proposed fix
-``` +```sh rg -n 'pub const max_|const max_|_cap|Limit|MAX_' runtime/src cli/src sdk/src🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/receipt-sweep-brief.md` around lines 37 - 39, Specify the shell language on the fenced code block containing the rg command by changing its opening fence to use sh, while leaving the command unchanged.Source: Linters/SAST tools
docs/receipt-sweep-brief.md-84-84 (1)
84-84: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winRemove the stale unreceipted status.
The supplied
runtime/src/main.zig:76-89context already contains receipt comments formax_imagesandmax_image_load_attempts. Move these entries to the receipted list, or record only the remaining verification work.Proposed fix
-- `runtime/src/main.zig:73-74` `max_images = 16`, `max_image_load_attempts = 3`. +- `runtime/src/main.zig:76-89` `max_images = 16`, `max_image_load_attempts = 3` — receipt present; verify synchronization only.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/receipt-sweep-brief.md` at line 84, Update the receipt status entry for max_images and max_image_load_attempts in the documentation to remove them from the unreceipted list, since runtime/src/main.zig already contains receipt comments; move the entries to the receipted list or retain only any genuinely outstanding verification work.myclock/widget.tsx-1-19 (1)
1-19: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winThis example sits outside
examples/and does not exercise the PR feature.Every other example lives under
examples/, for exampleexamples/visualizer/widget.tsx. This widget usesuseProvider("time"), notuseProviderSignal, so it adds no coverage for provider signals. Combined with the machine-specific paths inmyclock/tsconfig.json, the directory looks like a local scratch workspace. Move it toexamples/myclock/if it is intended to ship, or remove it from the PR.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@myclock/widget.tsx` around lines 1 - 19, Remove the standalone myclock widget from the PR, or relocate it under examples/myclock/ with the corresponding configuration updates if it is intended as a shipped example; do not retain it as a machine-specific scratch workspace outside examples. If kept, update the widget to use useProviderSignal so it exercises the provider-signals feature.sdk/test/reconciler.test.mjs-324-329 (1)
324-329: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAssert that the text node actually unmounted before you capture the baseline.
Line 324-325 calls
context.hideText()and awaits one microtask. If the render scheduler needs more than one tick,textWritesAfterUnmountis captured while the text node is still mounted, and line 329 then passes for the wrong reason. Add a positive assertion that the swap to<panel>happened.💚 Proposed fix
context.hideText(); await Promise.resolve(); + assert.ok(fixtureOperations.some(([name, type]) => name === "createNode" && type === "panel"), + "the fixture did not re-render to <panel>; the unmount assertion below would be vacuous"); const textWritesAfterUnmount = fixtureOperations.filter(([name]) => name === "setText").length; fixtureProviderCallback('{"provider":"audio","value":{"rms":0.7,"bands":[0.9]}}'); assert.equal(context.readRms(), 0.7); assert.equal(fixtureOperations.filter(([name]) => name === "setText").length, textWritesAfterUnmount); + assert.equal(fixtureOperations.some(([name]) => name === "reportError"), false); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdk/test/reconciler.test.mjs` around lines 324 - 329, In the test around context.hideText(), positively verify that the rendered text node has been replaced by the <panel> element before capturing textWritesAfterUnmount. Keep awaiting the scheduler as needed, then assert the swap using the existing fixture/render state symbols before establishing the baseline and continuing with the provider callback assertions.docs/macos-memory-handoff.md-525-530 (1)
525-530: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFix the shell continuation and comment placement.
Place the explanatory comment before the command or after the complete command. Keep each continuation backslash as the final character on its line.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/macos-memory-handoff.md` around lines 525 - 530, Fix the command example around the macOS renderer bakeoff invocation by moving the “or: software” explanation before the command or after the completed command, and ensure every line-continuation backslash remains the final character on its line.docs/error-propagation-brief.md-74-80 (1)
74-80: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winUpdate the image-limit documentation. The pinned Native revision uses a 1 MiB decoded-RGBA limit. A 256×256 RGBA image uses 262,144 bytes (256 KiB) and passes this limit.
weaver checkparses local image dimensions and reports the decoded byte calculation; it does not decode pixel data. Runtime failures report dimensions and the requested byte count. Restrict the downscale and placeholder guidance to images that exceed 1 MiB.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/error-propagation-brief.md` around lines 74 - 80, Update the image-limit section to document the pinned Native revision’s 1 MiB decoded-RGBA limit, correct the 256×256 RGBA calculation to 262,144 bytes (256 KiB) and state that it passes. Clarify that weaver check parses local dimensions and reports decoded-byte calculations without decoding pixels, while runtime failures report dimensions and requested bytes; restrict downscale and placeholder guidance to images exceeding 1 MiB.
🧹 Nitpick comments (9)
scripts/macos-audio-cost.py (1)
80-85: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCapture all CPU times in one snapshot at each boundary.
Lines 80-85 run a separate
pscommand for each PID. Each CPU delta therefore has a different measurement interval. WindowServer uses another interval. The output describes aligned one-second CPU deltas, but these calls do not produce aligned samples.Read
pidandcputimefor all Widget, host, and WindowServer PIDs in onepsinvocation before and after the sleep. Record timestamps adjacent to each snapshot.As per coding guidelines, “Treat widget memory and CPU performance as core product requirements; target performance that matches or exceeds Rainmeter.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/macos-audio-cost.py` around lines 80 - 85, Update the CPU sampling flow around process_cpu_time so each boundary performs one ps invocation covering all widget, host, and WindowServer PIDs, then records the timestamp immediately adjacent to that snapshot. Derive cpu_before, cpu_after, window_server_cpu_before, and window_server_cpu_after from the corresponding shared snapshots so every delta uses aligned measurement intervals.Source: Coding guidelines
docs/gpu-ledger-wall-brief.md (1)
46-51: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRecord the existing software-path switch.
The supplied
scripts/macos-renderer-bakeoff.pyharness already setsWEAVER_FORCE_SOFTWARE=1for the software candidate. Replace “find the forcing mechanism” with the exact variable and command so the measurement procedure remains reproducible.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/gpu-ledger-wall-brief.md` around lines 46 - 51, Update the “Software-candidate discriminator” procedure in gpu-ledger-wall-brief.md to replace the instruction to find the forcing mechanism with the exact existing switch WEAVER_FORCE_SOFTWARE=1 and the command used by scripts/macos-renderer-bakeoff.py. Preserve the software-candidate measurement and comparison steps.sdk/index.d.ts (1)
37-41: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Signal<T>and theuseProviderSignaloverloads are duplicated in two hand-maintained files.
sdk/index.d.tslines 37-41 and 96-100 restate whatsdk/src/reconciler.tslines 58-62 and 335-339 already declare. The two copies can drift silently, because nothing compares them. Consider generatingindex.d.tsfrom the source declarations, or add a test that asserts bothSignalshapes and all provider overloads stay identical.Also applies to: 96-100
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdk/index.d.ts` around lines 37 - 41, Remove the duplicated hand-maintained Signal and useProviderSignal declarations from sdk/index.d.ts by generating them from the corresponding declarations in sdk/src/reconciler.ts, or add an automated test that compares both Signal shapes and every provider overload to prevent drift.sdk/CONTRACT.md (1)
98-108: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the
weaver checkerror text foruseProviderSignal.Line 99-100 records the exact check error for
useProvider.cli/src/index.tsnow emits a second form for the signal hook:useProviderSignal("audio") requires subscribe: ["audio"] in the widget config. Add that form so agents can match both messages from the contract alone.📝 Proposed documentation update
Rules: hooks follow React's rules (top level, stable order). `useProvider` requires the provider in `config.subscribe` — checked at `weaver check`, error: `useProvider("time") requires subscribe: ["time"] in the widget config`. +`useProviderSignal` carries the same requirement and reports it with the hook +name that used it, e.g. `useProviderSignal("audio") requires subscribe: +["audio"] in the widget config`.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdk/CONTRACT.md` around lines 98 - 108, Update the CONTRACT.md documentation near the existing useProvider weaver check error to also record the exact useProviderSignal error form, including the provider name placeholder and required subscribe configuration. Keep both messages available for agents to match.cli/test/cli.test.mjs (1)
186-190: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the case where both hooks use the same unsubscribed provider.
The new behavior at
cli/src/index.tsline 1673-1676 emits one error per hook per provider. This test covers onlyuseProviderSignal("memory"). Add a fixture in whichuseProvider("memory")anduseProviderSignal("memory")both appear without the subscription, then assert both messages. That is the only assertion that pins the per-hook split.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cli/test/cli.test.mjs` around lines 186 - 190, Extend the CLI test fixture around missingSignalSubscription so it includes both useProvider("memory") and useProviderSignal("memory") without the memory subscription, then assert that stderr contains the distinct missing-subscription error for each hook. Preserve the existing exit-status assertion and ensure both messages are checked.sdk/src/reconciler.ts (1)
677-690: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
bindHostTextrebinds whenever the caller passes a fresh mapped signal.
instance.textBinding === bindingcompares object identity.signal.map(...)returns a new object on every call, so a widget that maps inside the render body unsubscribes and resubscribes on every component render.examples/visualizer/widget.tsxline 43 does this. The behavior is correct, but the churn is avoidable.Consider documenting in
sdk/CONTRACT.mdthat a mapped signal used as a<text>child should be created once, for example withuseRef, or memoizemapresults per source-and-projector pair.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdk/src/reconciler.ts` around lines 677 - 690, Document in sdk/CONTRACT.md that mapped signals used as <text> children should be created once and reused, such as by storing them with useRef or memoizing map results per source-and-projector pair, so bindHostText does not repeatedly unsubscribe and resubscribe when renders produce fresh mapped signal objects.examples/visualizer/widget.tsx (1)
100-103: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueThe RMS strip keeps origin-anchored geometry while the bar meter is now centered.
Lines 58-59 center the bar meter with
meterX. The strip at line 102 still starts at x = 0 and spanssegments * 13 - 1 = 311px. If the canvas width differs from 311 px, the two elements no longer share an edge. Center the strip with the same computation, or state in a comment that the offset is intentional.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/visualizer/widget.tsx` around lines 100 - 103, Update the RMS strip drawing loop in the widget rendering logic to use the same centered horizontal offset as the bar meter’s meterX calculation, so its geometry remains aligned across varying canvas widths; preserve the existing segment colors and dimensions.sdk/test/reconciler.test.mjs (1)
279-291: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the new signal error paths.
The PR introduces three developer-visible errors that no test exercises:
A bound <text> must contain exactly one Signal childatsdk/src/reconciler.tsline 520.<text> children must be strings, numbers, or one Signalat line 525.Signal.subscribe(listener) requires a functionandSignal.map(project) requires a functionat lines 1174, 1179, 1195, and 1199.These messages are part of the public contract that agents match against. A test that asserts each string prevents silent wording drift.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdk/test/reconciler.test.mjs` around lines 279 - 291, Add tests in sdk/test/reconciler.test.mjs covering each new developer-facing validation path: bound text with zero or multiple Signal children, invalid non-string/number/non-Signal text children, and non-function arguments to Signal.subscribe and Signal.map. Assert the exact public error strings, including “A bound <text> must contain exactly one Signal child”, “<text> children must be strings, numbers, or one Signal”, “Signal.subscribe(listener) requires a function”, and “Signal.map(project) requires a function”.Source: Coding guidelines
cli/src/index.ts (1)
1529-1530: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive the provider list from one source, and report unknown provider names.
Two points:
- Line 1529 declares the provider union and line 1655 repeats the same five names as a runtime array. If a provider is added to only one of them, the visitor silently ignores it and
weaver checkloses coverage for that provider.- The condition at line 1655 skips any literal that is not a known provider.
useProviderSignal("memmory")passescheckwith no error, then fails at runtime with the subscribe message, which points the developer at the wrong fix.♻️ Proposed change
- type ProviderName = "time" | "cpu" | "memory" | "audio" | "media"; + const providerNames = ["time", "cpu", "memory", "audio", "media"] as const; + type ProviderName = typeof providerNames[number]; const usedProviders = new Map<ProviderName, Set<"useProvider" | "useProviderSignal">>();if (ts.isCallExpression(node) && ts.isIdentifier(node.expression) && (node.expression.text === "useProvider" || node.expression.text === "useProviderSignal")) { const argument = node.arguments[0]; - if (argument && ts.isStringLiteral(argument) && ["time", "cpu", "memory", "audio", "media"].includes(argument.text)) { + if (argument && ts.isStringLiteral(argument) && (providerNames as readonly string[]).includes(argument.text)) { const provider = argument.text as ProviderName; const hooks = usedProviders.get(provider) ?? new Set<"useProvider" | "useProviderSignal">(); hooks.add(node.expression.text); usedProviders.set(provider, hooks); + } else if (argument && ts.isStringLiteral(argument)) { + errors.push(locationMessage( + node.getSourceFile(), + argument, + `${node.expression.text}("${argument.text}") names no known provider; available providers: ${providerNames.join(", ")}`, + )); } }Based on the coding guideline "messages must contain enough context and actionable information to fix the problem without reading the implementation".
Also applies to: 1652-1660
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cli/src/index.ts` around lines 1529 - 1530, Derive the runtime provider iteration in the visitor around usedProviders from the single ProviderName source instead of repeating the five provider literals, keeping the type and runtime list synchronized when providers change. Update the unknown-provider handling so unsupported literal names are reported with an actionable error identifying the invalid provider and expected providers, rather than being silently skipped.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/error-propagation-brief.md`:
- Around line 143-150: Extend the suggested acceptance test to assert atomicity
for both paths: when the initial render fails, verify no partial generation is
exposed; when a hot swap fails, verify the previously active bundle remains
unchanged and active. Keep the existing budget-error, logging, and visible
error-surface assertions intact.
- Around line 61-65: Update validateLoweredTreeBudgets in the weaver check flow
to inspect the lowered representation rather than authored JSX, so generated
text, canvases, and retained nodes are counted. Enforce all four
budgets—max_nodes, max_children, max_text_bytes, and max_canvases—and report
both the configured limit and requested amount for every failure.
- Around line 98-107: Update “Seam 6 — canvas prerequisites are unchecked” to
acknowledge that cli/src/index.ts already performs static validation via
canvasAncestorProblem, emitting CanvasNeedsUnclippedAncestors and
CanvasNeedsOpaqueAncestors through weaver check. Limit the remaining issue to
runtime logging when the canvas surface is denied dynamically, and replace the
stale CanvasNeedsExplicitSize line-1373 reference with the validator’s 1527–1681
range.
In `@docs/gpu-ledger-session-2026-07-30.md`:
- Around line 39-43: The conclusion in docs/gpu-ledger-session-2026-07-30.md
lines 39-43 should apply only to measured 1 Hz submissions, while preserving the
finding that 0.2 Hz submissions do not establish the arena; avoid generalizing
to every live widget. Update docs/gpu-ledger-wall-brief.md lines 143-147 by
replacing “any live widget” with the measured submission-rate condition, with no
other changes required.
- Around line 12-17: docs/gpu-ledger-session-2026-07-30.md lines 12-17: either
add measurements using 2–5 second idle gaps that directly support the reclaim
interval, or narrow the statement to the evidenced release window.
docs/gpu-ledger-wall-brief.md lines 126-128: apply the same evidence-bound
wording to the recommendation, without asserting an unsupported 2–5 second
interval.
In `@docs/gpu-ledger-wall-brief.md`:
- Around line 109-118: Revise the measurement table around the “Graphics ledger
(steady)” column to use a semantically accurate label and add explicit metadata
for sample count, measurement window, and aggregation type. Annotate each
affected value, including the initial, 0.2 Hz, and post-stop rows, as a mean,
range, peak, or final sample without changing the reported measurements.
- Around line 11-12: Update the branch metadata in the document header and
corresponding session record to clearly identify the measurement commit: either
make the header match the recorded Weaver commit or explicitly label the
pre-measurement and post-measurement commits in the `78f8cdf → 0db488a`
transition so the reported results have an unambiguous source state.
In `@docs/macos-memory-handoff.md`:
- Around line 131-136: Rewrite the background statement around the “owned
unmapped graphics” baseline to scope it to M3 Pro-class hardware and sustained
GPU submission workloads, rather than every widget or rendering path
universally. Explicitly note that the wall is near zero on the M2 Air, and avoid
presenting ~130 MB as a universal per-widget floor.
- Around line 112-119: Update the “Remaining work, in order” section to remove
the completed tile-validation tasks and record the post-patch visual
verification and 841/841 test result. Revise the later test-status section
around the existing checkpoint so it consistently states that the test passed
after the tile patch, or explicitly marks that checkpoint as non-final if
another validation is authoritative.
- Around line 32-38: Update the Native SDK setup instructions in the macOS
memory handoff to check out commit 6a8e6178 in detached mode instead of
following the mutable macos-memory-shared-renderer-prep branch, preserving the
existing repository and submodule setup steps.
- Around line 498-505: Update the Phase 1 paragraph so it no longer says the
gates are unchanged or predicts a 20s–30s MB widget process. Make acceptance
depend on retaining approximately zero graphics ledger, consistent with the
Phase 1 gate, and describe total footprint as a hardware-specific measurement
rather than a fixed estimate.
In `@docs/receipt-sweep-brief.md`:
- Around line 140-142: Update the acceptance-check command in the receipt-sweep
brief to reuse the complete inventory patterns, including _cap, Limit, and
camelCase max names, across runtime, CLI, and SDK sources. Add the SDK
soft-limit sweep and validate for every discovered definition that an adjacent
receipt or protocol/OS-bound comment exists at the definition site, rather than
only counting matching names.
- Around line 107-120: Update the “Mirrored constants” section to list the CLI
image-budget constants maxImageStreamBytes, nativeImagePixelByteLimit, and
nativeWidgetSourceByteLimit alongside their runtime counterparts, including
max_image_stream_bytes and max_image_rgba_bytes from runtime/src/main.zig.
Ensure the acceptance check explicitly covers runtime/src/main.zig while
preserving the existing node and depth mirror entries.
In `@myclock/tsconfig.json`:
- Around line 12-20: Resolve the committed myclock scratch workspace by either
moving it under examples/ as a proper example or removing it from the PR. For
myclock/tsconfig.json lines 12-20, if retained, replace the machine-specific SDK
paths with portable paths relative to baseUrl and match examples/visualizer's
tsconfig pattern; for myclock/widget.tsx lines 1-19, move it under examples/ and
switch its implementation to useProviderSignal if it demonstrates this PR,
otherwise remove it.
In `@runtime/native-sdk`:
- Line 1: Wait for SunkenInTime/native#27 to merge, then update the
runtime/native-sdk submodule pointer from the current PR head to the resulting
merge commit before merging this change.
In `@runtime/src/main.zig`:
- Around line 694-710: Update the canvas loop around canvas_state to iterate
with each slot index, check model.tree.canvas_occupied for that index, and skip
unoccupied slots before reading canvas_state.owner or calling nodeConst.
Preserve the existing handling for occupied canvas slots.
- Around line 687-690: Update the same-view update handling around the msg
switch and projectSameViewUpdate so canvas_generation changes from timer or
provider callbacks are detected and the new command batch is submitted even
without an active frame callback. Preserve the existing behavior for
canvas_frame messages and ensure non-frame setCanvasCommands commits either
trigger the canvas update directly or request a frame.
In `@scripts/macos-audio-cost.py`:
- Around line 201-203: Validate visualizer_source before applying the two
replacements in the installation flow: require exactly one occurrence each of
the Visualizer name marker and the offset marker. If either count is not one,
fail with the selected revision, the missing or invalid marker, and remediation
to use a compatible revision or update the source markers; only proceed with
replacement after both validations pass.
In `@sdk/src/reconciler.ts`:
- Around line 1182-1185: Update emit in the signal implementation to invoke
every listener through runWidgetCallback, ensuring one throwing listener does
not stop subsequent listeners and errors are handled consistently. Iterate over
a snapshot of listeners so subscription changes during emission do not alter the
current dispatch.
- Around line 517-529: Update the static `<text>` handling in the reconciler to
filter `vnode.children` with `isRenderable` before mapping them to strings,
allowing conditional `false`, `null`, and `undefined` children. Keep rejecting
other non-string/non-number children, and include `received ${typeof child}` in
that validation error.
---
Minor comments:
In `@docs/error-propagation-brief.md`:
- Around line 74-80: Update the image-limit section to document the pinned
Native revision’s 1 MiB decoded-RGBA limit, correct the 256×256 RGBA calculation
to 262,144 bytes (256 KiB) and state that it passes. Clarify that weaver check
parses local dimensions and reports decoded-byte calculations without decoding
pixels, while runtime failures report dimensions and requested bytes; restrict
downscale and placeholder guidance to images exceeding 1 MiB.
In `@docs/macos-memory-handoff.md`:
- Around line 525-530: Fix the command example around the macOS renderer bakeoff
invocation by moving the “or: software” explanation before the command or after
the completed command, and ensure every line-continuation backslash remains the
final character on its line.
In `@docs/receipt-sweep-brief.md`:
- Around line 37-39: Specify the shell language on the fenced code block
containing the rg command by changing its opening fence to use sh, while leaving
the command unchanged.
- Line 84: Update the receipt status entry for max_images and
max_image_load_attempts in the documentation to remove them from the unreceipted
list, since runtime/src/main.zig already contains receipt comments; move the
entries to the receipted list or retain only any genuinely outstanding
verification work.
In `@myclock/widget.tsx`:
- Around line 1-19: Remove the standalone myclock widget from the PR, or
relocate it under examples/myclock/ with the corresponding configuration updates
if it is intended as a shipped example; do not retain it as a machine-specific
scratch workspace outside examples. If kept, update the widget to use
useProviderSignal so it exercises the provider-signals feature.
In `@runtime/src/tree.zig`:
- Line 256: Preserve canvas_generation across transaction snapshots: update
cloneInto to copy the source canvas_generation, initialize canvas_generation in
resetEmpty, and add a batch-abort test covering canvas revision restoration
through beginBatch and abortBatch.
In `@sdk/test/reconciler.test.mjs`:
- Around line 324-329: In the test around context.hideText(), positively verify
that the rendered text node has been replaced by the <panel> element before
capturing textWritesAfterUnmount. Keep awaiting the scheduler as needed, then
assert the swap using the existing fixture/render state symbols before
establishing the baseline and continuing with the provider callback assertions.
---
Nitpick comments:
In `@cli/src/index.ts`:
- Around line 1529-1530: Derive the runtime provider iteration in the visitor
around usedProviders from the single ProviderName source instead of repeating
the five provider literals, keeping the type and runtime list synchronized when
providers change. Update the unknown-provider handling so unsupported literal
names are reported with an actionable error identifying the invalid provider and
expected providers, rather than being silently skipped.
In `@cli/test/cli.test.mjs`:
- Around line 186-190: Extend the CLI test fixture around
missingSignalSubscription so it includes both useProvider("memory") and
useProviderSignal("memory") without the memory subscription, then assert that
stderr contains the distinct missing-subscription error for each hook. Preserve
the existing exit-status assertion and ensure both messages are checked.
In `@docs/gpu-ledger-wall-brief.md`:
- Around line 46-51: Update the “Software-candidate discriminator” procedure in
gpu-ledger-wall-brief.md to replace the instruction to find the forcing
mechanism with the exact existing switch WEAVER_FORCE_SOFTWARE=1 and the command
used by scripts/macos-renderer-bakeoff.py. Preserve the software-candidate
measurement and comparison steps.
In `@examples/visualizer/widget.tsx`:
- Around line 100-103: Update the RMS strip drawing loop in the widget rendering
logic to use the same centered horizontal offset as the bar meter’s meterX
calculation, so its geometry remains aligned across varying canvas widths;
preserve the existing segment colors and dimensions.
In `@scripts/macos-audio-cost.py`:
- Around line 80-85: Update the CPU sampling flow around process_cpu_time so
each boundary performs one ps invocation covering all widget, host, and
WindowServer PIDs, then records the timestamp immediately adjacent to that
snapshot. Derive cpu_before, cpu_after, window_server_cpu_before, and
window_server_cpu_after from the corresponding shared snapshots so every delta
uses aligned measurement intervals.
In `@sdk/CONTRACT.md`:
- Around line 98-108: Update the CONTRACT.md documentation near the existing
useProvider weaver check error to also record the exact useProviderSignal error
form, including the provider name placeholder and required subscribe
configuration. Keep both messages available for agents to match.
In `@sdk/index.d.ts`:
- Around line 37-41: Remove the duplicated hand-maintained Signal and
useProviderSignal declarations from sdk/index.d.ts by generating them from the
corresponding declarations in sdk/src/reconciler.ts, or add an automated test
that compares both Signal shapes and every provider overload to prevent drift.
In `@sdk/src/reconciler.ts`:
- Around line 677-690: Document in sdk/CONTRACT.md that mapped signals used as
<text> children should be created once and reused, such as by storing them with
useRef or memoizing map results per source-and-projector pair, so bindHostText
does not repeatedly unsubscribe and resubscribe when renders produce fresh
mapped signal objects.
In `@sdk/test/reconciler.test.mjs`:
- Around line 279-291: Add tests in sdk/test/reconciler.test.mjs covering each
new developer-facing validation path: bound text with zero or multiple Signal
children, invalid non-string/number/non-Signal text children, and non-function
arguments to Signal.subscribe and Signal.map. Assert the exact public error
strings, including “A bound <text> must contain exactly one Signal child”,
“<text> children must be strings, numbers, or one Signal”,
“Signal.subscribe(listener) requires a function”, and “Signal.map(project)
requires a function”.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a9a005e9-6002-46ce-8916-ae2e6e43b6d9
⛔ Files ignored due to path filters (2)
myclock/dist/bundle.jsis excluded by!**/dist/**myclock/dist/widget.jsonis excluded by!**/dist/**
📒 Files selected for processing (23)
cli/src/index.tscli/test/cli.test.mjsdocs/error-propagation-brief.mddocs/gpu-ledger-session-2026-07-30.mddocs/gpu-ledger-wall-brief.mddocs/macos-memory-handoff.mddocs/macos-visualizer-perf-2026-07-31.mddocs/receipt-sweep-brief.mdexamples/visualizer/widget.tsxmyclock/tsconfig.jsonmyclock/widget.tsxruntime/build.zigruntime/native-sdkruntime/src/js_engine.zigruntime/src/main.zigruntime/src/tree.zigscripts/macos-audio-cost.pyscripts/release-audit.mjssdk/CONTRACT.mdsdk/index.d.tssdk/src/index.tssdk/src/reconciler.tssdk/test/reconciler.test.mjs
Greptile SummaryThis change repins the Native SDK to the visualizer rendering merge and updates the release audit to require that exact revision. The Native SDK integration path was exercised successfully: T-Rex validation blockedDirect Native runtime build and test execution could not run because the required Zig tool is missing from this Linux environment ( Confidence Score: 5/5The PR is safe to merge; no blocking failure remains in the executable integration paths. The release audit, focused SDK/reconciler/CLI contract tests, TypeScript typecheck, and CLI build all completed successfully with the new Native SDK pin. Native runtime rendering could not be compiled in this environment because Zig and macOS rendering support are unavailable.
 T-Rex LogsWhat T-Rex did
Reviews (7): Last reviewed commit: "Repin Native SDK after visualizer merge" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Owner
Author
Owner
Author
Owner
Author
Owner
Author
|
Addressed the CodeRabbit review in
Validation: |
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
Owner
Author
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/macos-memory-handoff.md`:
- Around line 496-501: Update the earlier Phase 1 acceptance gate in the macOS
memory handoff document to remove the conflicting 20s–30s widget-footprint
requirement. Align it with the ledger_tag_graphics_footprint-near-zero criterion
used in the later Phase 1 text, or explicitly mark the legacy gate as superseded
while preserving separate reporting of hardware-specific content cost.
- Around line 521-525: Align the comment immediately above the macOS renderer
bakeoff command with the actual --candidate value, ensuring both identify the
same benchmark; if this is intended to be the software comparison, update the
command to use software, otherwise revise the comment to describe
metal-composite.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3d2281fb-94cf-425c-8fb8-ac8dd7fdd502
📒 Files selected for processing (17)
cli/src/index.tscli/test/cli.test.mjsdocs/error-propagation-brief.mddocs/gpu-ledger-session-2026-07-30.mddocs/gpu-ledger-wall-brief.mddocs/macos-memory-handoff.mddocs/receipt-sweep-brief.mdexamples/myclock/tsconfig.jsonexamples/myclock/widget.tsxexamples/visualizer/widget.tsxruntime/src/main.zigruntime/src/tree.zigscripts/macos-audio-cost.pysdk/CONTRACT.mdsdk/src/reconciler.tssdk/test/contract-tables.test.mjssdk/test/reconciler.test.mjs
🚧 Files skipped from review as they are similar to previous changes (11)
- cli/test/cli.test.mjs
- docs/gpu-ledger-session-2026-07-30.md
- runtime/src/tree.zig
- docs/receipt-sweep-brief.md
- sdk/CONTRACT.md
- docs/error-propagation-brief.md
- examples/visualizer/widget.tsx
- runtime/src/main.zig
- scripts/macos-audio-cost.py
- sdk/src/reconciler.ts
- docs/gpu-ledger-wall-brief.md
Owner
Author
Owner
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
Why
The audio Visualizer was rebuilding TSX/layout state and entering avoidable raster and trace paths on every frame. Those costs came from the generic provider, Canvas, renderer, and trace seams—not from text labels and not from Canvas being the wrong primitive.
Impact
The developer-facing shape stays ordinary TSX: labels remain
<text>, animated meter geometry remains<canvas>, and no visualizer-specific native primitive is introduced.Adjacent production A/B receipts on an Apple M2 measured:
The receipt is in
docs/macos-visualizer-perf-2026-07-31.md.Dependency
4c5c0999; this PR pins the resulting merge commit.Validation
npm cinpm run buildnpm test— 80/80npm run typechecknpm run audit:releaseruntime: zig build test-platform-servicesruntime: zig build -Doptimize=ReleaseFastruntime: zig build testhost: zig build -Doptimize=ReleaseFasthost: zig build test -Doptimize=ReleaseFastSummary by CodeRabbit
New Features
Bug Fixes
Performance