Add regression test for subquery materializer multi-chunk log replay (bug 4)

[robacourt]

Summary

Adds a deterministic regression test for bug 4 from the restore-from-file bug triage in #4648 (bug doc: packages/sync-service/bugs.md).

This is a test-only change. Bug 4's production fix — the two main-log short-circuits in Materializer.read_history_up_to_subscribed/2 — already shipped alongside the bug 1 fix in #4651; it just lacked dedicated regression coverage.

Problem (bug 4)

On startup recovery the subquery materializer replays the source shape's persisted history (snapshot + main log) up to its subscribed offset. Storage.get_log_stream/3 returns one chunk per call for the snapshot, but returns the entire requested range for the main log in a single call. When the source shape's main log spans more than one chunk, a replay loop that keeps advancing through chunk boundaries past the first main-log chunk re-reads main-log entries it has already applied. Re-applying a NewRecord for an existing key raises Key ... already exists, crashing the materializer (and taking the dependent shape's consumer down with it → 409 must-refetch on the next poll).

Solution / coverage

The fix (already merged in #4651) stops iterating as soon as the read steps into the main log. This PR adds the missing regression test:

test/electric/shapes/consumer/materializer_test.exs →
"does not re-read main-log entries when the main log spans multiple chunks"

• Sets chunk_bytes_threshold: 10 to force the source shape's main log across ≥2 chunks (asserts the first main-log chunk ends strictly before the subscribed offset).
• Persists two NewRecord inserts in separate main-log chunks, subscribes past both, and asserts startup replay yields the correct value_counts exactly once.

Test Plan

• Passes with the fix in place (59 tests, 0 failures in the file).
• Fails without the fix: reverting the short-circuits makes the test crash with ** (RuntimeError) Key "public"."test_table"/"3" already exists from read_history_up_to_subscribed/2 — confirming it's a genuine failing-first regression test.
• mix format --check-formatted clean.

Notes

An end-to-end variant in oracle_restore_test.exs was prototyped but not included: it does trigger the crash, but the failure self-heals via a client refetch that re-syncs to the oracle, so it passes with and without the fix — false-confidence coverage. The deterministic unit test is the reliable regression guard.

---

Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 60.04%. Comparing base (44eafb6) to head (0cfe76a).
⚠️ Report is 19 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #4670       +/-   ##
===========================================
+ Coverage   43.18%   60.04%   +16.85%     
===========================================
  Files         312      395       +83     
  Lines       25598    43747    +18149     
  Branches     8699    12581     +3882     
===========================================
+ Hits        11054    26266    +15212     
- Misses      14466    17403     +2937     
  Partials       78       78               

Flag	Coverage Δ
packages/agents	72.64% <ø> (ø)
packages/agents-mcp	77.70% <ø> (ø)
packages/agents-mobile	80.67% <ø> (ø)
packages/agents-runtime	83.73% <ø> (?)
packages/agents-server	75.67% <ø> (+0.20%)	⬆️
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.04% <ø> (+16.85%)	⬆️
unit-tests	60.04% <ø> (+16.85%)	⬆️

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	dce5d17
🔍 Latest deploy log	https://app.netlify.com/projects/electric-next/deploys/6a4515728c0d3d0008256d85
😎 Deploy Preview	https://deploy-preview-4670--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

Test-only PR adding a deterministic regression test for bug 4 (subquery materializer re-reading main-log entries when the source shape's main log spans multiple chunks). The production fix already shipped in #4651; this fills the missing coverage gap. Overall: clean, well-targeted, and correct — recommend merge.

What's Working Well

• Genuinely failing-first regression test. The test exercises exactly the short-circuit logic in Materializer.read_history_up_to_subscribed/2. The PR documents that reverting the fix reproduces Key ... already exists, which matches the code path: once iteration steps into the main log, get_log_stream/3 returns the whole remaining range, and re-applying a NewRecord for an existing key raises.
• Self-validating setup. The sanity check (assert LogOffset.compare(first_main_chunk_end, offset_3) == :lt) guards against the scenario silently degrading — if chunk_bytes_threshold: 10 ever stopped forcing at least 2 main-log chunks, the test would fail loudly rather than pass vacuously.
• Follows conventions well. Mirrors the adjacent "replays main-log entries persisted before subscription" test, uses System.unique_integer() for handle isolation (safe under async: true), and the with_pure_file_storage_opts tag is correctly consumed by the with_pure_file_storage setup.

Issues Found

Critical (Must Fix)

None.

Important (Should Fix)

None. No changeset is required (test-only, no shipped behavior change) — correctly omitted.

Suggestions (Nice to Have)

• Idiom (still open, trivial): assert not is_nil(first_main_chunk_end) reads more naturally as refute is_nil(first_main_chunk_end). Non-blocking.

Issue Conformance

No GitHub issue is linked, but the PR body references the originating bug triage (#4648, bugs.md bug 4) and the fix PR (#4651), which is adequate traceability for a follow-up test. The test matches the described scenario precisely: snapshot key "1" (value 10) + two separate main-log chunks inserting keys "2"/"3" (values 20/30), subscribing past both, asserting each value materializes exactly once.

Previous Review Status

Resolved since iteration 1: @alco flagged that the test comments narrated "the buggy iteration" (a reverted code state), which reads as noise now that the bug is fixed. The reframe in 0cfe76a4f addresses this cleanly — all three "buggy iteration" / "exercise the bug" references are gone and the comments now describe the invariant in present tense ("Startup replay must therefore stop iterating…", "This test guards that each persisted entry is applied exactly once"). No regressions introduced; the code under test is unchanged.

Still open from iteration 1: only the trivial refute is_nil idiom suggestion above.

---

Review iteration: 2 | 2026-07-02