Extract Y::Sync::Engine; make Y::ActionCable::Sync an adapter over it by jpcamara · Pull Request #55 · jpcamara/yrby

jpcamara · 2026-07-17T18:40:00Z

Extracts the sync state machine from Y::ActionCable::Sync into a transport-neutral Y::Sync::Engine and reworks the ActionCable class into an adapter over it.

Y::ActionCable::Sync had the y-websocket state machine baked into the channel, routing results through transmit, stream_from, and ActionCable.server.broadcast. Those three calls were the only thing tying the protocol to ActionCable. Pulling the state machine out lets a second transport (a raw WebSocket, or REST plus a pub/sub bus) be another adapter over one core instead of a reimplementation of the reliability logic.

Y::Sync::Engine (core yrby, lib/y/sync/engine.rb) holds the state machine with no transport. It takes load/change hooks, and handle(key, encoded, bytes) returns a Result(reply, broadcast, ack). Ack-tracked delivery, causal-gap detection, and integrated-only serving live here because they are the yrs calls (update_ready?, update_advances?, handle_sync_message, compacted_state_update). It also exposes sync_step1 and full_state (gap-free state for a request/response joiner). The engine adds no mutable protocol state, so sharing one across connections is as safe as its hooks are; the adapter builds one per channel instance, with hooks that capture that channel.

Y::ActionCable::Sync becomes the cable adapter: envelope decode, size cap, validations, and routing the engine's Result through transmit/broadcast. Its hooks still run on_load/on_change in the channel instance's context via instance_exec, so on_change can reach current_user, params, and channel methods. It drops from 357 to 305 lines.

MSG_KIND_* are canonically on the engine now and aliased under the concern, since they were reachable as Y::ActionCable::Sync::MSG_KIND_* and an app or adapter referencing them should keep working.

Causal-gap resync logging stays in the adapter, driven by the engine's ack (sync_log_gap_resync if result.ack == :gap). The log line names the document key and sync_log_context, both cable concerns, so the engine reports the outcome and the transport decides how to record it.

Core yrby goes to 0.7.0 and the yrby-rails floor rises to yrby >= 0.7.0. The bump has to happen here rather than at release prep: the demo resolves both gems by path, so a floor above the tree's own version makes its bundle unsolvable. The changelog entry stays under [Unreleased] until release prep stamps it.

Behavior is unchanged, and sync_test.rb passes with no edits at all. A new sync_engine_test.rb pins the core directly (11 tests): record+relay, record-before-relay ordering, raising-recorder rejection, lost-ack retry, gap→resync→recover, handshake reply-not-broadcast, gap-free full_state, awareness relay, and junk frames. Each update is a captured Yjs delta, since a Doc is read-only from Ruby.

Full suite: 183 runs, 0 failures; rubocop clean. The engine ships in core yrby, not duplicated in yrby-rails.

cursor · 2026-07-17T18:40:04Z

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

…lity, native primitive Supersedes the boolean accept_causal_gaps flag with the complete design discussed, so the whole architecture is reviewable in one place. causal_gap_policy dial (default :reject, unchanged): - :reject rebuild + gap check; resync a gap (current behavior) - :accept_strict record a gap but withhold the ack until it integrates, so the sender's retransmits keep an open gap self-signaling - :accept ack-on-durable via an INVERTED write path: append + relay + ack with no doc rebuild (O(1)-ish vs O(history)); dedup is delegated to the store's idempotency Serving stays gap-free under every policy (handle_sync_message / compacted_state_update exclude pending) — the policy changes only what is stored and acked, never what is served. Store contract for accept modes (documented + reference store in README): lossless on_load (preserves pending), idempotent append (content hash), and compaction guarded with doc.pending? so an open gap is never compacted away. Unhealable-gap repair loop: on join / SyncStep1 with an open gap, the server solicits the missing dependency from that client (any live client that has it heals the gap) — no separate strike subsystem. A truly-unhealable gap surfaces via on_gap rather than healing. Observability: on_gap class hook (fired with the doc key when a gap is observed) plus an info log, replacing reject mode's resync-storm signal. Hook errors are swallowed so observability can't break frame handling. Native Doc#update_adds_content? (yrby core): true if an update adds any content (integrated OR pending). update_advances? flips false->true only on the FIRST pending struct, so a second gap on an already-pending doc reads as a duplicate; the new primitive stays correct. The concern prefers it and falls back to a full-state comparison on an older core. Rust unit tests included and passing. All Ruby tests (38) and Rust tests (35, incl. 3 new) pass locally; rubocop, cargo fmt, and clippy clean. Still a proposal, default :reject. Engine port (Y::Sync::Engine, #55) not included: it's on an unmerged branch and I won't stack PRs; the policy logic is factored to port mechanically. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

The concern baked the y-websocket state machine into an ActionCable channel and routed results through transmit / stream_from / ActionCable.server.broadcast. Those three calls were the only thing tying the protocol to ActionCable. Y::Sync::Engine (core yrby) is the state machine with no transport: it takes load/change hooks and turns a decoded frame into a Result — reply to sender, broadcast to peers, ack outcome. Reliability (ack-tracked delivery, causal-gap detection, integrated-only serving) lives there because it's the yrs calls themselves. The concern is now the cable adapter: envelope decode, size cap, validations, and routing the Result. Its hooks still run on_load/on_change in the channel instance's context via instance_exec lambdas, so on_change can still reach current_user and friends. Behavior is unchanged — the full sync_test.rb passes as-is (one moved constant reference). New sync_engine_test.rb pins the core directly (11 tests): record+relay, record-before-relay, raising-recorder rejection, lost-ack retry, gap->resync, handshake, gap-free full_state, awareness relay, junk. Every update is a real captured Yjs delta, since a Doc is read-only from Ruby. This is the extraction the yrby-over-MessageBus spike proved out: a second transport (REST + pub/sub) becomes a second adapter over the same engine. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Found by gpt-5.6-sol review: yrby-actioncable depended on yrby >= 0.3.1, but the concern now references Y::Sync::Engine, which is new in core 0.7.0. Installing the new actioncable gem against an older released core (0.6.0 and down) would NameError on the first frame. Bump core to 0.7.0, actioncable to 0.4.0, and the dependency floor to yrby >= 0.7.0. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017TeaovH2jyHARHJSyQ8Afo

# Conflicts: # CHANGELOG-rails.md # CHANGELOG.md # lib/y/action_cable/sync.rb

Y::ActionCable::Sync::MSG_KIND_* were reachable, so moving them outright would raise NameError in any app or adapter that referenced them. They are canonically on the engine now and aliased back under the concern, which also lets sync_test.rb go back to the original constant, so the protocol suite really does pass unmodified. The Result contract said exactly one of reply/broadcast is always set, which :noop contradicts, and promised delivery to OTHER clients, while this adapter broadcasts to a stream that includes the sender. It now states the branch matrix, that routing precedes acking, and that recipient policy belongs to the transport. The thread-safety note is qualified by the hooks, since the engine stores arbitrary closures. The gemspec credited the channel with gating on update_ready?; that is the engine's job now.

A line-by-line pass over every comment this PR touches. The accuracy fixes: on_load said it runs once per key when it runs per handshake and per update; on_change said it records every change when an already-stored retry skips it; the module header, sync_receive, and the hook validation credited the concern with checks the engine now makes; sync_log_context claimed it only reaches drop logs, but gap-resync logs use it too; y.rb still named the yrby-actioncable gem. The engine's own docs claimed a matrix that omitted unreadable updates, and its thread-safety note left out that two concurrent calls can both classify one delta as new, so a recorder has to tolerate duplicates. The rest is prose: refactor narration aimed at a reviewer, an em dash, a repeated antithesis, unverifiable claims about cost and about who is subscribed, and a changelog line I left unwrapped.

lib/y/version.rb said 0.7.0 while the changelog entry sat under [Unreleased], which contradict each other; every past bump in this repo is its own release-prep commit that renames the section at the same time. Core goes back to 0.6.1 here. The yrby-rails floor stays at yrby >= 0.7.0: that is the version which will ship the engine, and it is what orders the two releases. sync_send_ack re-implemented the engine's list of ackable outcomes. It now takes the Result and asks ack?, so adding an outcome cannot make the cable adapter and the engine disagree about what gets acked.

Reverting it broke the demo's bundle: yrby-rails requires the engine at yrby >= 0.7.0, and the demo resolves both gems by path, so the floor has to be satisfiable in the tree before core 0.7.0 is published. The version now carries a comment saying why it leads the last release.

jpcamara · 2026-08-09T12:27:02Z

Holding this until the Discourse work is ready to consume it. The extraction is sound (parity verified, comments audited, CI green), but until a second transport actually uses Y::Sync::Engine, it adds an indirection nobody needs, and merging it would put main on an unreleased core 0.7.0 that yrby-rails then requires.

Picking it back up alongside the Discourse plugin, which is the consumer that justifies it.

* Add accept_causal_gaps option (proposal, off by default) Adds an opt-in mode where a causally-gapped update is recorded immediately as a pending struct and acked, instead of being rejected for a resync. It heals when its missing dependency arrives via that dependency's own reliable- delivery retransmit. Default is unchanged (reject + resync). The motivation is durability and latency: reject-and-resync returns custody of a received edit to the sender until a resync completes, and pays an O(doc) resync round trip for what is usually a transient reorder. Accept mode makes the edit durable on arrival and heals with no round trip. Serving stays gap- free in BOTH modes (handle_sync_message / compacted_state_update exclude pending), so the safety invariant -- never hand a peer un-integrable content -- is untouched. Accept mode changes only what is stored, not what is served. The non-obvious part, surfaced while building this: it is NOT just deleting the reject branch. update_advances? flips false->true only on the FIRST pending struct, so a second gap on an already-pending doc reads as a duplicate and would be silently dropped. The concern uses a lossless full- state comparison (sync_gappy_adds_content?) for the gappy path instead, which correctly distinguishes a new gap from a duplicate retry. A native "adds any struct?" primitive could replace it later. Two costs, both documented and required to run it safely: 1. The store must be lossless: on_load preserves pending, and compaction is guarded with doc.pending? (compacted_state_update strips pending). 2. An open gap is silent, not a loud resync storm: each recorded gap is logged at info, and pending depth should be monitored. Off by default; existing behavior and all existing tests are unchanged. Ports directly to the Y::Sync::Engine refactor (same branch logic). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Expand to the full accept-gaps vision: policy dial, repair, observability, native primitive Supersedes the boolean accept_causal_gaps flag with the complete design discussed, so the whole architecture is reviewable in one place. causal_gap_policy dial (default :reject, unchanged): - :reject rebuild + gap check; resync a gap (current behavior) - :accept_strict record a gap but withhold the ack until it integrates, so the sender's retransmits keep an open gap self-signaling - :accept ack-on-durable via an INVERTED write path: append + relay + ack with no doc rebuild (O(1)-ish vs O(history)); dedup is delegated to the store's idempotency Serving stays gap-free under every policy (handle_sync_message / compacted_state_update exclude pending) — the policy changes only what is stored and acked, never what is served. Store contract for accept modes (documented + reference store in README): lossless on_load (preserves pending), idempotent append (content hash), and compaction guarded with doc.pending? so an open gap is never compacted away. Unhealable-gap repair loop: on join / SyncStep1 with an open gap, the server solicits the missing dependency from that client (any live client that has it heals the gap) — no separate strike subsystem. A truly-unhealable gap surfaces via on_gap rather than healing. Observability: on_gap class hook (fired with the doc key when a gap is observed) plus an info log, replacing reject mode's resync-storm signal. Hook errors are swallowed so observability can't break frame handling. Native Doc#update_adds_content? (yrby core): true if an update adds any content (integrated OR pending). update_advances? flips false->true only on the FIRST pending struct, so a second gap on an already-pending doc reads as a duplicate; the new primitive stays correct. The concern prefers it and falls back to a full-state comparison on an older core. Rust unit tests included and passing. All Ruby tests (38) and Rust tests (35, incl. 3 new) pass locally; rubocop, cargo fmt, and clippy clean. Still a proposal, default :reject. Engine port (Y::Sync::Engine, #55) not included: it's on an unmerged branch and I won't stack PRs; the policy logic is factored to port mechanically. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Fix two Sol review findings in :accept_strict P1: a retry of a still-gappy update returned :applied and got acked, telling the sender to stop retransmitting before the update integrated — defeating :accept_strict's self-signaling. A gappy duplicate now returns :recorded_pending (unacked); only a ready duplicate is acked. P2: sync_observe_gap fired (log + on_gap) before sync_record_change, so a failing on_change logged and metriced a durable gap that was never persisted, re-inflating on every retry. Observe now runs only after the record succeeds. Both covered by new/strengthened tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Replace em dashes with plain punctuation Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017TeaovH2jyHARHJSyQ8Afo * Accept causal gaps unconditionally; drop the policy option A causally-incomplete update is now recorded and acked like any other (ack-on-durable). It parks as a pending struct, is never served, and heals when its missing dependency arrives, usually via the join handshake soliciting it from the next client. The write path no longer rebuilds the document per update: it appends, relays, and acks. Gone with the reject path: the causal_gap_policy option (never shipped), the accept_strict middle mode and its update_adds_content? native primitive (the write path no longer dedups, so the ext/ changes revert entirely), the per-update ready/advances gate, and the sync_log_gap_resync reject logging. A lost-ack retry records again; replay converges because CRDT apply is idempotent. on_gap and the info log surface an open gap at join/serve time. The store contract tightens: on_load must preserve pending, compaction must not run while doc.pending? (Y::Document already quarantines), and on_change must tolerate duplicate deltas. * Replace a leftover em dash * Serve full state, pending included, like Y.js handle_sync_message answered a SyncStep1 with integrated-only state on the theory that a served pending struct poisons the peer. It does not: a peer parks it exactly as this doc did and heals it when the missing dependency arrives, which Y.js's own encodeStateAsUpdate relies on. And the healing mechanism was never the serve-side filter; it is the ack loop, since the missing dependency is an update its own sender still holds unacked and keeps retransmitting. The one place pending must still be excluded is compaction, where folding a log into a snapshot would freeze an un-integrable struct into the base state forever. compacted_state_update, integrated_update, the pending? readers, and Y::Document's quarantine all stay. * State the behavior without referencing what it replaced The delivery-guarantees bullet argued against rejection, a test message asserted what does not happen, and the changelog described an option that never shipped. The changelogs keep recording the delta from the released behavior; everything else states the behavior as the behavior. * Trim the serve comment to the serve path * Catch the serve-flip stragglers in comments and docs Four comments and a README line still described serving as gap-free or pending as never served, which stopped being true when serving went lossless: the test section header, the solicit test, sync_receive's doc, sync_observe_gap's doc, and the observability lead. Pending is served and travels; what stays true is that its content is invisible in the document until the dependency arrives, so they say that. * State the compaction invariant precisely The changelog said compaction must not run while doc.pending?, which overstates it: Y::Document itself compacts while pending by folding clean rows and quarantining pending ones. The invariant is that an acked update must never leave durable storage before it integrates, so a fold must not drop a pending row into a gap-free snapshot. * Drop a changelog line that describes nothing * Update the passages the diff never touched Five comments and README passages still described update validation, gap rejection, and resync-on-gap: the concern's module header, the Scope section's acking and gap bullets, the ephemeral-documents walkthrough, and the acks section. All five now state the current behavior; the Scope gap bullet also gains the healing story and the compaction quarantine. * Compact past an open gap A batch holding a causal gap quarantined every row in it, including rows with no relationship to the gap: the clean-set retry filtered on the pending marker, and a fresh gap row is unmarked, so the retry failed and marked the whole batch. Independent rows then stayed pinned until the gap healed. The gapped path now folds everything integrable into state in one pass and judges each row against the folded result: a row the folded state cannot integrate cleanly carries the gap or builds on it and is quarantined; a row that is ready and adds nothing is fully captured and deleted. A healed gap folds out at the next compaction instead of waiting for its dependency to be joined by a fresh threshold of edits. A gap-only batch leaves state untouched. * Drop the mid-session repair solicit Join and reconnect handshakes already have a client send everything beyond the server's integrated state, and the missing dependency's own sender retransmits it until acked, so the extra server-initiated SyncStep1 on a mid-session sync bought only a narrow window: a gap formed after connect, healed before anyone reconnects, by a client that happened to sync. A regular Yjs server sends no unsolicited SyncStep1s, and now neither does this one. sync_request_resync lost its last caller and is gone; observing an open gap at serve time stays. * Make the README gap docs friendlier and current Rewrite the causal-gap sections in plainer second-person prose: lead with what you get, frame the store contract as guidance for custom stores, and fold the on_gap bullet into the observability paragraph. Drop two leftover claims that updates are checked against durable state before recording; the write path records, relays, and acks. --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

jpcamara mentioned this pull request Jul 18, 2026

Accept causal gaps #56

Merged

jpcamara force-pushed the refactor/sync-engine branch from ffa6f8d to d555ff6 Compare August 4, 2026 05:00

jpcamara force-pushed the refactor/sync-engine branch from d555ff6 to 67b3132 Compare August 4, 2026 05:19

jpcamara and others added 3 commits August 5, 2026 14:56

Replace em dashes with plain punctuation

945cb92

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017TeaovH2jyHARHJSyQ8Afo

jpcamara force-pushed the refactor/sync-engine branch from 67b3132 to 945cb92 Compare August 5, 2026 18:57

jpcamara added 5 commits August 8, 2026 15:20

Merge remote-tracking branch 'origin/main' into refactor/sync-engine

0d3c610

# Conflicts: # CHANGELOG-rails.md # CHANGELOG.md # lib/y/action_cable/sync.rb

Navigation Menu

Extract Y::Sync::Engine; make Y::ActionCable::Sync an adapter over it - #55

Extract Y::Sync::Engine; make Y::ActionCable::Sync an adapter over it#55
jpcamara wants to merge 8 commits into
mainfrom
refactor/sync-engine

jpcamara commented Jul 17, 2026 •

edited

Loading

Uh oh!

cursor Bot commented Jul 17, 2026

Uh oh!

jpcamara commented Aug 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

jpcamara commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cursor Bot commented Jul 17, 2026

Uh oh!

jpcamara commented Aug 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

jpcamara commented Jul 17, 2026 •

edited

Loading