[AIT-287] Implement new rules for discarding ops buffered during sync

[lawrence-forooghian]

Note: This PR is based on top of #118; please review that one first.

Implements ably/specification@997584f. Integration tests ported from ably/ably-js@9b2224d.

Summary by CodeRabbit

• Tests

  • Expanded test coverage for Live Objects channel attachment and object operation buffering scenarios.

• Refactor

  • Improved internal handling of object operation buffering during channel synchronization and attachment events to align with protocol specifications.

[coderabbitai]

Walkthrough

The changes refine buffer management for OBJECT operations during Live Objects synchronization and channel attachment. The internal logic now preserves buffered operations when continuing existing sync sequences while clearing them appropriately during new syncs and channel attachment. Test coverage is expanded to validate this behavior across various flag combinations.

Changes

Cohort / File(s)	Summary
Core Sync Buffer Logic Sources/AblyLiveObjects/Internal/InternalDefaultRealtimeObjects.swift	Added testsOnly_bufferedObjectOperationsCount property; refined buffer clearing logic to preserve operations when continuing sync sequences; restructured channel attachment state transitions to handle SYNCING and non-SYNCING cases separately with consistent buffer clearing at attachment time.
Unit Test Updates Tests/AblyLiveObjectsTests/InternalDefaultRealtimeObjectsTests.swift	Renamed test doesNotModifyStateWhenHasObjectsIsTrue to handlesHasObjectTrue; added buffering behavior assertions during sync; updated spec point annotations (RTO4d/RTO4c); added explicit checks for buffer clearing after attachment.
Integration Test API & Expansion Tests/AblyLiveObjectsTests/JS Integration Tests/ObjectsIntegrationTests.swift	Changed injectAttachedMessage signature from boolean hasObjects parameter to ARTProtocolMessageFlag flags with default empty set; updated all 5+ call-sites to use new flag-based API; expanded test scenarios to verify ATTACHED message handling with HAS_OBJECTS flag clears local state across various flag combinations (including RESUMED).

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Poem

🐰 The buffers now flow with graceful precision,
When syncs continue, we guard each decision,
Attached channels clear what must go,
With flags held high, the operations flow!
Through testing we hopped, each scenario tight—
Live Objects leap forward, buffering just right! 🚀

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly refers to the main change: implementing new rules for discarding operations buffered during sync, which is the core focus of the PR objectives and all file changes.
Docstring Coverage	✅ Passed	Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch AIT-287-new-rules-for-discarding-buffered-events

---

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.}

[lawrence-forooghian]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

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

87-87: Remove the generic comment on the testsOnly_ property.

Line 87 adds a generic explanatory comment for a testsOnly_ declaration; repo guidance asks to omit these in Sources/**/*.swift.

✂️ Suggested cleanup

-    /// Returns the number of buffered object operations if in the SYNCING state, or nil otherwise.
     internal var testsOnly_bufferedObjectOperationsCount: Int? {

As per coding guidelines, Sources/**/*.swift: “For testsOnly_ property declarations, do not add generic explanatory comments (their meaning is understood)”.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Sources/AblyLiveObjects/Internal/InternalDefaultRealtimeObjects.swift` at
line 87, Remove the generic explanatory comment added for the testsOnly_
property; locate the testsOnly_ declaration in InternalDefaultRealtimeObjects
(the testsOnly_ property in InternalDefaultRealtimeObjects.swift) and delete the
comment above it so the property has no generic explanatory comment per
Sources/**/*.swift guidelines.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Tests/AblyLiveObjectsTests/JS` Integration
Tests/ObjectsIntegrationTests.swift:
- Around line 607-622: This test case is missing the required spec-point
attribution comment; add an exact-format comment (either // `@spec`(<section
reference>) or // `@specPartial`(<section reference>)) immediately above the
.init( ... ) test declaration that references the relevant spec point governing
ATTACHED without HAS_OBJECTS behavior, choosing `@spec` vs `@specPartial` per the
guideline and avoid duplicating the same `@spec` tag elsewhere for the same spec
point; ensure the comment is placed in the same test block that contains the
action closure (the anonymous .init with description "on ATTACHED without
HAS_OBJECTS clears local state") so tools can associate it with that test.

---

Nitpick comments:
In `@Sources/AblyLiveObjects/Internal/InternalDefaultRealtimeObjects.swift`:
- Line 87: Remove the generic explanatory comment added for the testsOnly_
property; locate the testsOnly_ declaration in InternalDefaultRealtimeObjects
(the testsOnly_ property in InternalDefaultRealtimeObjects.swift) and delete the
comment above it so the property has no generic explanatory comment per
Sources/**/*.swift guidelines.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 6f3ee39a-112d-4dbd-b002-94cd53865bdc

📥 Commits

Reviewing files that changed from the base of the PR and between cbb49fd and 65dda76.

📒 Files selected for processing (3)

• Sources/AblyLiveObjects/Internal/InternalDefaultRealtimeObjects.swift
• Tests/AblyLiveObjectsTests/InternalDefaultRealtimeObjectsTests.swift
• Tests/AblyLiveObjectsTests/JS Integration Tests/ObjectsIntegrationTests.swift