[icehaunter]

Motivation

Live shape requests spend most of their long-poll lifetime parked in a receive while waiting for either a shape change or timeout. Under high concurrency, parking inside the larger request-handling function kept more heap live than the small wait state actually needed, which made the per-request memory floor significant.

Changes

• Move the blocking live-request receive into a narrow wait_for_live_change/2 helper that only holds the change reference and timeout while parked.
• Convert receive outcomes into small event values and handle response construction after the wait returns.
• Keep garbage collection immediately before the parked receive, but run it inside the narrow helper so collection happens with a smaller root set.
• Remove a dead SystemMonitor timer fallback branch that Elixir 1.20 reports as unreachable under warnings-as-errors.
• Add changesets for sync-service and electric-telemetry.

Decisions

This keeps the waiter in the existing request process instead of introducing a central waiter registry. That avoids adding a new register/deregister bottleneck while still reducing the heap retained by parked live requests. The existing behavior is preserved for new changes, shape rotation, out-of-bounds checks, read-only timeout handling, and ordinary no-change responses.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 60.19%. Comparing base (1b611ca) to head (be7aaa5).
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4680      +/-   ##
==========================================
- Coverage   60.20%   60.19%   -0.01%     
==========================================
  Files         410      410              
  Lines       44378    44377       -1     
  Branches    12581    12580       -1     
==========================================
- Hits        26716    26711       -5     
- Misses      17584    17588       +4     
  Partials       78       78              

Flag	Coverage Δ
electric-telemetry	71.42% <ø> (-1.95%)	⬇️
elixir	71.42% <ø> (-1.95%)	⬇️
packages/agents	72.64% <ø> (ø)
packages/agents-mcp	77.70% <ø> (ø)
packages/agents-mobile	80.67% <ø> (ø)
packages/agents-runtime	83.72% <ø> (-0.03%)	⬇️
packages/agents-server	75.63% <ø> (+0.16%)	⬆️
packages/agents-server-ui	8.32% <ø> (ø)
packages/electric-ax	51.06% <ø> (ø)
packages/experimental	87.73% <ø> (ø)
packages/react-hooks	86.48% <ø> (ø)
packages/start	82.83% <ø> (ø)
packages/typescript-client	91.83% <ø> (ø)
packages/y-electric	56.05% <ø> (ø)
typescript	60.02% <ø> (+0.01%)	⬆️
unit-tests	60.19% <ø> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[claude]

Claude Code Review

Summary

Refactors the live long-poll wait in Electric.Shapes.Api from one large receive/after block inside hold_until_change/1 into a narrow wait_for_live_change/2 helper plus per-outcome handle_live_change_result/2 clauses, and removes a dead schedule_garbage_collect/1 fallback clause in SystemMonitor. It is a clean, behavior-preserving refactor with changesets and full test coverage.

What's Working Well

• Faithful behavior preservation. Every receive outcome maps 1:1 to the previous logic: :new_changes, both :shape_rotation variants, :out_of_bounds_timeout (including the check_for_disk_updates/1 fallback), and the :long_poll_timeout cond (read-only / :up / timeout-error branches). The selective receive still only matches ^ref-tagged messages, so unrelated mailbox messages are left in place exactly as before.
• Readability win. Splitting the monolithic receive into small tagged event values + dedicated handler clauses is easier to follow than the deeply nested original, and it keeps the GC-before-park comment close to the receive.
• Correct dead-code removal. The removed defp schedule_garbage_collect(state), do: state is verifiably unreachable: both call sites — init/1 (state seeded with garbage_collect_timer: nil) and line 142 (%{state | garbage_collect_timer: nil} |> schedule_garbage_collect()) — guarantee the nil clause matches. Removing it is safe and clears the Elixir 1.20 warnings-as-errors failure.
• Changeset present for both @core/sync-service and @core/electric-telemetry, matching the touched publishable packages.

Issues Found

Critical (Must Fix)

None.

Important (Should Fix)

Does the structural split actually lower retained heap during the park?

File: packages/sync-service/lib/electric/shapes/api.ex:867-893

The PR's stated goal is that parking inside a narrow helper "keeps the parked receive from retaining the larger request-handling frame." But hold_until_change/1 calls the helper as a non-tail call — the result of wait_for_live_change(ref, long_poll_timeout) is piped into handle_live_change_result(request), so request is still consumed after the wait returns.

Because request is needed after wait_for_live_change/2 returns, the BEAM must keep it live in hold_until_change/1's stack frame across the parked receive. :erlang.garbage_collect/0 scans the whole process stack, so request (the dominant term here) stays a GC root and is retained regardless of which frame the receive physically lives in. handle_live_request/1 reaches hold_until_change/1 via a tail call, so there is no larger ancestor frame being dropped either.

In other words, the memory win looks like it comes from the :erlang.garbage_collect/0-before-park (which already existed in the original), not from moving the receive into a sub-function. The refactor is still worthwhile for readability and is at worst neutral, but I would suggest confirming with a profiling/benchmark measurement that the structural change moves the per-request memory floor, rather than assuming the narrower frame is what helps. If measurements show no delta from the split alone, consider updating the code comment/PR description so future readers do not over-attribute the saving to the frame boundary. Happy to be corrected if you have before/after :erlang.process_info(pid, :memory) numbers showing the split itself matters.

Suggestions (Nice to Have)

• Minor: the two Api.Error.must_refetch() handler clauses ({:shape_rotation, new_handle} and :shape_rotation) duplicate the error construction. Not worth changing unless you are already touching them — the current form reads fine.

Issue Conformance

No linked issue (flagged per convention — a tracking issue or incident link would help contextualize the memory-floor motivation), but the PR description is thorough and clearly enumerates the preserved behaviors. Implementation matches the described scope with no scope creep. Codecov reports all modified lines covered and no failing tests.

Previous Review Status

N/A — first review of this PR.

---

Review iteration: 1 | 2026-07-02

[robacourt]

LGTM

[github-actions]

This PR has been released! 🚀

The following packages include changes from this PR:

• @core/sync-service@1.7.6

Thanks for contributing to Electric!