Fix O(n) shape removal from the subquery index

[robacourt]

Summary

Removing a shape from the subquery index in the where-clause filter had become O(n) in the number of shapes — and O(shapes × values) once subqueries were seeded. Shape removal runs on the replication path and blocks it, so slow removal caused WAL lag in production (the "Materializer shape invalidated" path). This restores O(1) removal (as in v1.5).

Fixes #4279

Problem

The subquery index is a single ETS :bag table. Several deletions used :ets.match_delete/2 with a wildcard in the key position (e.g. {:membership, handle, :_, :_}, {:node_positive_member, node, :_}). A :bag (hash) table can only use its key index when the key is fully bound, so a partially-bound key forces a full-table scan that grows with the number of shapes. The worst offender was delete_node_members, which scanned every shape's per-(node, value) routing row on each removal — O(shapes × values).

Solution

Switch the table from :bag to :ordered_set and lift the discriminating fields (value, shape, branch, next_cond) out of the value tuples into the keys. In an :ordered_set, a delete whose key has a bound prefix is range-limited (not a full scan), and a point delete by a fully-bound key is O(log n). Concretely:

• unregister_shape/2 replaces its full-table match_delete scans with prefix-bounded select_deletes.
• remove_shape/5 point-deletes the shape's node rows, and derives its node_*_member deletions from the removed shape's own membership rows (scoped by subquery_ref), applying exact point-deletes. This touches only the removed shape's rows — never the other shapes sharing a value.

Removal is now O(V_shape · log n) — proportional to the removed shape's own view size, independent of the total number of shapes, the number of shapes on a node, and the number of shapes sharing a value.

This is a pure internal storage change: every public function and all externally-observable behavior is preserved. The per-change routing hot path (affected_shapes) and the exact-evaluation member? path are equal-or-faster (the :ordered_set prefix select projects in C); table memory is roughly flat (measured slightly lower in a microbenchmark).

Note: a dedicated memory reduction (interning / row-shrinking) is intentionally out of scope here — this PR is the removal-complexity fix only, kept minimal and behavior-preserving. Memory can be tackled separately.

Test plan

• :performance flatness tests prove removal is independent of each dimension — total shapes, shapes-on-node, and shapes-per-value — by measuring removal at two well-separated sizes and asserting the delta stays within noise (observed delta 0 at 1k vs 20k).
• Positive control: removal cost does scale with the removed shape's own view size (pins the complexity class as O(V_shape), so a regression can't hide under a flat budget).
• No-orphan guard: a full Filter.remove_shape of a seeded shape, and draining a shared node one-by-one, leaves the subquery index byte-identical to its pre-add state.
• Existing subquery_index / filter / consumer / shape_log_collector suites green; full mix test green.

🤖 Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.97%. Comparing base (56fa8c3) to head (2e2de45).
⚠️ Report is 5 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #4594       +/-   ##
===========================================
+ Coverage   34.77%   58.97%   +24.19%     
===========================================
  Files         233      385      +152     
  Lines       19839    42653    +22814     
  Branches     7094    12211     +5117     
===========================================
+ Hits         6900    25154    +18254     
- Misses      12905    17424     +4519     
- Partials       34       75       +41     

Flag	Coverage Δ
packages/agents	72.79% <ø> (?)
packages/agents-mcp	77.70% <ø> (?)
packages/agents-mobile	80.67% <ø> (ø)
packages/agents-runtime	82.70% <ø> (?)
packages/agents-server	75.41% <ø> (-0.09%)	⬇️
packages/agents-server-ui	7.50% <ø> (ø)
packages/electric-ax	46.42% <ø> (?)
packages/experimental	87.73% <ø> (?)
packages/react-hooks	86.48% <ø> (?)
packages/start	82.83% <ø> (?)
packages/typescript-client	91.83% <ø> (?)
packages/y-electric	56.05% <ø> (?)
typescript	58.97% <ø> (+24.19%)	⬆️
unit-tests	58.97% <ø> (+24.19%)	⬆️

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.

[netlify]

✅ Deploy Preview for electric-next ready!

Name	Link
🔨 Latest commit	367f582
🔍 Latest deploy log	https://app.netlify.com/projects/electric-next/deploys/6a318c673deebb00087799c0
😎 Deploy Preview	https://deploy-preview-4594--electric-next.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[claude]

Claude Code Review

Summary

This PR restores O(1) shape removal from the where-clause subquery index (regressed to O(shapes × values) in v1.6, causing production WAL lag — #4279). The fix swaps the backing ETS table from :bag to :ordered_set and lifts the discriminating fields (value, shape, branch, next_cond) out of the value tuples into the key, so deletions that previously needed a full-table scan (wildcard key on a hash table) become prefix-bounded range deletes or point deletes. It's a clean, well-scoped, behavior-preserving storage change. Strong work overall — I found no correctness or security issues. Suggestion #1 from the previous iteration is now resolved.

What's Working Well

• Tight encapsulation. The entire row schema lives inside subquery_index.ex; no other module reads these tag tuples, so the key/value reshaping is fully internal and safe to change.
• Result-shape parity verified. Every accessor (members_for, negated_shapes_for, fallback_for, all_node_shapes, nodes_for_shape, nodes_for_shape_dependency) returns tuples in the same order/shape as the old values_for_key/reduce versions, so affected_shapes routing and exact member? evaluation are unchanged.
• The O(1) claim holds. delete_node_members derives the removed shape's own values from its :membership rows (scoped by subquery_ref) and point-deletes each node-member row — touching only this shape's rows, never the other shapes sharing a value/node. Prefix-bounded select_delete/select on :ordered_set is genuinely range-limited (the partially-bound-key optimization), which is the correct mechanism for this.
• Excellent test coverage. Flatness tests across all three dimensions (total shapes, shapes-on-node, shapes-per-value), a positive control that pins the complexity class to O(V_shape) so a regression can't hide under a flat budget, a byte-identical no-orphan drain guard (snapshot_filter_ets before == after), and a memory regression guard.
• Changeset present and accurate (@core/sync-service patch).

Issues Found

Critical (Must Fix)

None.

Important (Should Fix)

None blocking.

Suggestions (Nice to Have)

1. Resolved — removal-safety invariant now inlined. Commit fa6c72f (docs: inline the membership-present invariant in delete_node_members) replaces the dangling # see spec reference with a full inline statement of the invariant. I verified each claim against the code and they all hold:

• Ordering — Filter.remove_shape/2 runs WhereCondition.remove_shape (-> remove_shape/5 -> delete_node_members) at filter.ex:127, before maybe_unregister_subquery_shape -> unregister_shape/2 deletes the :membership rows at filter.ex:132/143.
• Subset invariant — add_value/remove_value write/delete the :membership row (subquery_index.ex:250/265) alongside the node-member rows (:247/262), so enumerating membership finds every node-member row.
• No concurrent writer — Consumer.stop/3 (shape_cleaner.ex:165) runs synchronously before ShapeLogCollector.remove_shape -> Filter.remove_shape (:170).

The comment also correctly names the "no orphan rows" test as the guard and warns that reordering cleanup would reintroduce orphans. This is exactly the right place for the invariant to live now that the design doc was removed. Nicely done.

2. :ordered_set silently overwrites where :bag accumulated. (unchanged — non-issue)

Keys like {:node_shape, node_id, shape_id, branch_key} are now unique. If add_shape/5 were ever invoked twice for the same (node, shape, branch_key), the old :bag kept two rows (distinct next_condition_id in the value) while the new :ordered_set overwrites to one. This is almost certainly an existing add-once invariant, so likely a non-issue — just flagging it as an assumption the new representation now depends on.

3. Stale comment in the shapes-per-value perf test. (still open)

subquery_index_test.exs — the "removal is O(1) in the number of shapes sharing each value" test comments that "the current :bag impl is O(n^2) even during setup … so @perf_large = 20_000 would time out", and uses 200 vs 1_000 instead. Post-merge the impl is :ordered_set, so this reasoning is historical; a future reader may wonder why this one test uses a smaller scale than the others. Consider updating the comment (and possibly bumping the scale to match the others now that setup is no longer O(n^2)).

4. Perf tests are :performance-tagged (excluded by default). (still open)

test_helper.exs:11 excludes :performance, so these guards don't run in the default mix test / PR CI. That's the right call for noisy reduction-based tests, but it means a future regression to O(n) removal won't be caught automatically. Worth confirming these are wired into a scheduled/perf CI job so the guard actually guards.

Issue Conformance

Directly fixes #4279. The issue is well-specified (production Honeycomb evidence, microbenchmark confirmation, root-cause analysis). The PR meets the primary ask — restoring O(1) add/remove. The issue also speculated about a memory redesign to "reduce memory footprint"; this PR explicitly and reasonably scopes memory out (documented in the PR body), and instead ships a memory-regression guard (< 48 MiB, measured ~38.2 MiB ordered_set vs ~39.0 MiB bag) to prove the reshape doesn't balloon. Deferring the memory optimization to a separate change is sound given this fix is on the replication hot path and benefits from being minimal and behavior-preserving.

Previous Review Status

Iteration 3. One new commit since iteration 2 (fa6c72f), a docs-only change that resolves suggestion #1 — the removal-safety invariant is now inlined in delete_node_members and I've re-verified its accuracy against the code. No code/behavior changes, no regressions. Suggestions #2 (non-issue), #3 (stale test comment), and #4 (perf tests excluded from default CI) remain open and non-blocking. Nothing here blocks merge.

---

Review iteration: 3 | 2026-06-16

[alco]

Brilliant!

[github-actions]

This PR has been released! 🚀

The following packages include changes from this PR:

• @core/sync-service@1.7.2

Thanks for contributing to Electric!