[AIT-254] Buffer operations whilst SYNCING per updated RTO8a

[lawrence-forooghian]

Implement the behaviour described in the RTO8a modification made in ably/specification@1956e2f. That is, buffer messages received from the point at which we become ATTACHED, not just whilst we're receiving a multi-OBJECT_SYNC sync sequence.

Note that currently, these messages will incorrectly get dropped upon receipt of the sync sequence; this is a bug in all implementations and will be addressed in https://ably.atlassian.net/browse/AIT-287.

Summary by CodeRabbit

• Refactor
  • Improved internal handling of object synchronization during sync operations through enhanced buffering and state management of object messages.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

The changes refactor the buffer-during-sync logic to accumulate OBJECT messages from the moment SYNCING state is entered, rather than only after the first OBJECT_SYNC message. A new bufferedObjectOperations field is introduced to temporarily hold OBJECT messages, which are then applied after OBJECT_SYNC completes, before transitioning to SYNCED.

Changes

Cohort / File(s)

Summary

Buffering and State Management Sources/AblyLiveObjects/Internal/InternalDefaultRealtimeObjects.swift

Introduces bufferedObjectOperations to accumulate OBJECT messages during SYNCING state. Updates initialization to include empty buffer and nullable syncSequence. Modifies OBJECT_SYNC handling to compute sync sequence and build pool entries. Shifts buffered message application to after OBJECT_SYNC completes. Adds precondition checks for state guard enforcement when applying buffered operations. Updates transition logic to initialize buffering on SYNCING state entry and apply buffered operations before SYNCED transition.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Poem

🐰 Buffering wisely through the sync,
Messages rest in our careful sink,
Till OBJECT_SYNC gives the go,
Then flush them all, nice and slow!
State machines dance, state guards stand true,
Ably's LiveObjects made brand new! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: implementing buffering of operations during SYNCING per the RTO8a specification update, which aligns with the PR objectives.
Linked Issues check	✅ Passed	The code changes implement the AIT-254 requirement to buffer operations upon entering SYNCING state rather than only after first OBJECT_SYNC, addressing the core objective of aligning with updated RTO8a specification.
Out of Scope Changes check	✅ Passed	All changes are focused on implementing buffering logic during SYNCING state in InternalDefaultRealtimeObjects.swift, directly aligned with AIT-254 objectives without introducing unrelated modifications.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch AIT-254-buffer-when-SYNCING

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[maratal]

It wasn't very deep review, but lgtm, would suggest @umair-ably also take a look.

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In `@Sources/AblyLiveObjects/Internal/InternalDefaultRealtimeObjects.swift`:
- Around line 544-564: The code currently clears
syncingData.bufferedObjectOperations whenever a "new" sync sequence is detected
in the .syncing(state) branch, which drops ops buffered between ATTACHED and the
first OBJECT_SYNC; change the logic in the .syncing(syncingData) handling so
buffering is only discarded when an existing syncSequence is being replaced
(i.e., syncingData.syncSequence != nil && syncingData.syncSequence?.id !=
syncCursor?.sequenceID), otherwise preserve bufferedObjectOperations when
starting the first sequence (syncingData.syncSequence == nil) and only set
syncingData.syncSequence = nil when actually replacing an existing sequence.
- Around line 458-470: The new Syncing type and its stored properties lack
explicit access control; update the declaration for class Syncing and its
members (bufferedObjectOperations, syncSequence, and init) to include explicit
access levels (e.g., internal or private as per project conventions) so they
satisfy SwiftLint explicit_acl — add the chosen access modifier to the class
declaration, both var declarations, and the initializer signature (for example:
internal class Syncing { internal var bufferedObjectOperations: ...; internal
var syncSequence: ...; internal init(...) { ... } }).

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

• Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 347eed5 and 6a34938.

📒 Files selected for processing (1)

• Sources/AblyLiveObjects/Internal/InternalDefaultRealtimeObjects.swift

🧰 Additional context used

📓 Path-based instructions (3)

**/*.swift

📄 CodeRabbit inference engine (.cursor/rules/swift.mdc)

**/*.swift: Specify an explicit access control level (SwiftLint explicit_acl) for all declarations in Swift code (tests are exempt)
When extending a type, put the access level on the extension declaration rather than on each member (tests are exempt)
Prefer implicit .init(...) when the type can be inferred in initializer expressions
Prefer enum case shorthand (.caseName) when the type can be inferred
For JSONValue or WireValue, prefer using literal syntax via ExpressibleBy*Literal where possible
Prefer Swift raw string literals for JSON strings instead of escaping double quotes
When an array literal begins with an initializer expression, place the initializer on the line after the opening bracket

Files:

• Sources/AblyLiveObjects/Internal/InternalDefaultRealtimeObjects.swift

Sources/AblyLiveObjects/**/*.swift

📄 CodeRabbit inference engine (.cursor/rules/swift.mdc)

In AblyLiveObjects library (non-test) code, import modules as: Ably with import Ably, and _AblyPluginSupportPrivate with internal import _AblyPluginSupportPrivate

Files:

• Sources/AblyLiveObjects/Internal/InternalDefaultRealtimeObjects.swift

Sources/**/*.swift

📄 CodeRabbit inference engine (.cursor/rules/testing.mdc)

For testsOnly_ property declarations, do not add generic explanatory comments (their meaning is understood)

Files:

• Sources/AblyLiveObjects/Internal/InternalDefaultRealtimeObjects.swift

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)

• GitHub Check: Xcode, release configuration, macOS (Xcode 16.4)
• GitHub Check: Xcode, release configuration, tvOS (Xcode 16.4)
• GitHub Check: Xcode, iOS (Xcode 16.4)
• GitHub Check: Xcode, release configuration, iOS (Xcode 16.4)
• GitHub Check: Example app, macOS (Xcode 16.4)
• GitHub Check: Xcode, macOS (Xcode 16.4)
• GitHub Check: Xcode, tvOS (Xcode 16.4)
• GitHub Check: Example app, iOS (Xcode 16.4)
• GitHub Check: Example app, tvOS (Xcode 16.4)
• GitHub Check: SPM (Xcode 16.4)
• GitHub Check: SPM, release configuration (Xcode 16.4)
• GitHub Check: Generate code coverage

🔇 Additional comments (3)

Sources/AblyLiveObjects/Internal/InternalDefaultRealtimeObjects.swift (3)

510-514: LGTM: SYNCING now initializes with an empty buffer.

---

567-636: Good: apply buffered ops after sync completion before transitioning to SYNCED.

---

652-657: LGTM: buffer OBJECT messages while SYNCING.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}