Uh oh!
There was an error while loading. Please reload this page.
[AIT-1092] chore(uts): reconcile LiveObjects (objects) test specs with main spec - #499
Merged
Merged
Conversation
sacOO7force-pushed
the
chore/fix-uts-liveobjects-spec
branch
from
June 30, 2026 19:25
161ca87 to
34b9ee5CompareThere was a problem hiding this comment.
Pull request overview
This PR updates the uts/objects test-spec pseudocode to match the newer canonical LiveObjects spec terminology and behaviors, ensuring derived SDK tests are generated from spec-faithful, internally consistent inputs.
Changes:
- Renames internal CRDT types and related specs to
InternalLiveMap/InternalLiveCounter, while keeping public API/blueprint naming intact. - Updates
RealtimeObject.get()preconditions/behavior to the ensure-active-channel flow (reattach on DETACHED; reject on FAILED), and consolidates RTO25/RTO26 precondition tests intorealtime_object.md. - Improves test correctness and determinism (negative-assertion quiescence barriers, ack serial helpers, siteCode exposure, and several behavioral expectation fixes).
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| uts/objects/unit/value_types.md | Renames blueprint value types to LiveCounter/LiveMap and updates assertions/requirements accordingly. |
| uts/objects/unit/realtime_object.md | Updates get() behavior to ensure-active-channel semantics; consolidates RTO25/RTO26 precondition tests; adds sync-wait correctness assertions. |
| uts/objects/unit/public_object_message.md | Renames _derivedFrom to spec-neutral derivedFrom in local-only retained create metadata. |
| uts/objects/unit/path_object.md | Updates internal CRDT naming in delegation/prose/spec references. |
| uts/objects/unit/path_object_subscribe.md | Adds quiescence-control patterns to make negative subscription assertions non-flaky; adjusts depth/path stimuli. |
| uts/objects/unit/path_object_mutations.md | Updates internal CRDT naming and blueprint type names for write delegation tests. |
| uts/objects/unit/parent_references.md | Updates internal CRDT naming for parentReferences and rebuild behavior tests. |
| uts/objects/unit/objects_pool.md | Updates internal CRDT naming in pool initialization/sync logic assertions. |
| uts/objects/unit/live_object_subscribe.md | Adds quiescence barriers and clarifies noop/tombstone subscription behaviors. |
| uts/objects/unit/internal_live_map.md | Renames and updates InternalLiveMap CRDT tests; adjusts several semantics explanations/assertions. |
| uts/objects/unit/internal_live_map_api.md | Renames map API tests and repoints precondition-test references to consolidated sections. |
| uts/objects/unit/internal_live_counter.md | Renames and updates InternalLiveCounter CRDT tests. |
| uts/objects/unit/internal_live_counter_api.md | Renames counter API tests and repoints precondition-test references to consolidated sections. |
| uts/objects/unit/instance.md | Updates internal CRDT naming for Instance delegation and event assertions; adds quiescence control for unsubscribe tests. |
| uts/objects/PLAN.md | Updates architecture summary and file plan to reflect new naming and consolidated precondition sections. |
| uts/objects/integration/objects_sync_test.md | Updates integration test prose to InternalLiveMap naming. |
| uts/objects/integration/objects_lifecycle_test.md | Updates integration test prose for InternalLive* naming and blueprint type naming. |
| uts/objects/helpers/standard_test_pool.md | Exposes canonical SITE_CODE and ack_serial; documents negative-assertion quiescence; updates naming. |
Comments suppressed due to low confidence (1)
uts/objects/unit/internal_live_map.md:1301
- Grammar: use “an InternalLiveMap” (vowel sound) instead of “a InternalLiveMap”.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
sacOO7
commented
Jul 3, 2026
Uh oh!
There was an error while loading. Please reload this page.
sacOO7
marked this pull request as draft
July 6, 2026 09:50
sacOO7 added a commit
to ably/ably-js
that referenced
this pull request
Jul 7, 2026
…dit gaps Reconciles the derived objects unit tests with the revised test specs (ably/specification#499) and folds in the fixes from the objects UTS audit: - RTO23e (ensure-active-channel): new test for get() re-attaching a DETACHED channel; the get()-on-FAILED test relabeled to RTO23e/get-rejects-failed-0 with the duplicate old RTO23b body folded away. - RTO25b access preconditions now exercised via [...root.keys()] on both DETACHED and FAILED (new keys()-based FAILED test). - RTO20e gains the mid-sync assertions (operation unsettled, value unchanged while SYNCING); RTO20e1 Test ID corrected to fails-on-channel-detached-0. - RTO24c1 rewritten to the spec's depth semantics (depth: 1 covers only the subscription's own path); the two path_object_subscribe depth tests go spec-verbatim with single-candidate COUNTER_INC stimuli (RTO24b2a1 fix), retiring their inline adaptations. - Quiescence controls added to negative-assertion tests (RTINS16f, depth tests, gated RTO24b2b); RTINS16g asserts on the delivered event's object instead of the pre-existing handle. - Audit fixes: RTO3 tests a fresh unsynced pool (zero-value root); RTO4b captures notifyUpdated to verify the update diff and the previously vacuous objectMessage assertion; RTO5c9 regains the serial-replay steps; update.noop assertions restored at 7 rejection sites; statusCode 400 asserts restored; object_id/RTPO13c5 tags aligned with spec Test IDs. - standard_test_pool exports the canonical SITE_CODE and ackSerial() constants; replay tests reference them instead of hardcoded literals. (Implementing the spec's original "ack-{m}:{i}" value surfaced a spec defect - it sorts before the pool's "t:0" entry timeserials under string LWW and gets rejected as stale; the sort-safe t:{m+1}:{i} form is used and the spec-side fix is prepared on the spec branch.) - RTLO4b4c1 exercises the genuine noop suppression path via an already-merged COUNTER_CREATE (ably-js applies missing-number increments as NaN - new RTLC9h deviation, documented). - deviations.md: retired entries superseded by the reconciled specs (get()-reattach, depth semantics, RTLM24 strict-> comparison), added the RTLC9h entry and registry entries for the four skipped tests, plus the null/undefined language-mapping umbrella. Suite: 309 passing, 0 failing, 4 pending (all documented).
sacOO7 added a commit
to ably/ably-js
that referenced
this pull request
Jul 7, 2026
…dit gaps Reconciles the derived objects unit tests with the revised test specs (ably/specification#499) and folds in the fixes from the objects UTS audit: - RTO23e (ensure-active-channel): new test for get() re-attaching a DETACHED channel; the get()-on-FAILED test relabeled to RTO23e/get-rejects-failed-0 with the duplicate old RTO23b body folded away. - RTO25b access preconditions now exercised via [...root.keys()] on both DETACHED and FAILED (new keys()-based FAILED test). - RTO20e gains the mid-sync assertions (operation unsettled, value unchanged while SYNCING); RTO20e1 Test ID corrected to fails-on-channel-detached-0. - RTO24c1 rewritten to the spec's depth semantics (depth: 1 covers only the subscription's own path); the two path_object_subscribe depth tests go spec-verbatim with single-candidate COUNTER_INC stimuli (RTO24b2a1 fix), retiring their inline adaptations. - Quiescence controls added to negative-assertion tests (RTINS16f, depth tests, gated RTO24b2b); RTINS16g asserts on the delivered event's object instead of the pre-existing handle. - Audit fixes: RTO3 tests a fresh unsynced pool (zero-value root); RTO4b captures notifyUpdated to verify the update diff and the previously vacuous objectMessage assertion; RTO5c9 regains the serial-replay steps; update.noop assertions restored at 7 rejection sites; statusCode 400 asserts restored; object_id/RTPO13c5 tags aligned with spec Test IDs. - standard_test_pool exports the canonical SITE_CODE and ackSerial() constants; replay tests reference them instead of hardcoded literals. (Implementing the spec's original "ack-{m}:{i}" value surfaced a spec defect - it sorts before the pool's "t:0" entry timeserials under string LWW and gets rejected as stale; the sort-safe t:{m+1}:{i} form is used and the spec-side fix is prepared on the spec branch.) - RTLO4b4c1 exercises the genuine noop suppression path via an already-merged COUNTER_CREATE (ably-js applies missing-number increments as NaN - new RTLC9h deviation, documented). - deviations.md: retired entries superseded by the reconciled specs (get()-reattach, depth semantics, RTLM24 strict-> comparison), added the RTLC9h entry and registry entries for the four skipped tests, plus the null/undefined language-mapping umbrella. Suite: 309 passing, 0 failing, 4 pending (all documented).
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
uts/objects/unit/internal_live_map.md:1301
- Grammar: use “an InternalLiveMap” (vowel sound) instead of “a InternalLiveMap”.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
sacOO7 added a commit
that referenced
this pull request
Jul 10, 2026
Review comments (PR #499): - Grammar: "a Internal*" -> "an Internal*" at the 4 flagged sites plus a 5th the review missed (internal_live_map.md tombstone prose). - Add objectsGCGracePeriod: 86400000 to the 3 minimal mock CONNECTED connectionDetails (RTO23a, RTO23e-DETACHED, RTO23e-FAILED) — the other 12 blocks already carried it, and 86400000 is the spec's own default (RTO10b3); omitting it invited derived-test failures for fixture reasons unrelated to each test's intent. - Link the LiveObjects REST API docs (built from the OpenAPI spec) in the V2-format REST provisioning section. Naming audit (LiveMap/LiveCounter vs InternalLive*): - RTPO3 heading + requirement row said path resolution walks "LiveMaps"; per RTPO3a1 the walked objects are InternalLiveMaps — the section body was renamed but the heading/table lagged. - build_object_state comment: createOp validation happens on the internal CRDT object — "a LiveCounter rejects" -> "an InternalLiveCounter rejects" (comment was added after the mass rename, so no sweep could catch it). Behavioral audit: - internal_live_map_api.md header over-claimed RTLM24 and RTLM13: the file has no MAP_CLEAR test (RTLM24 is the inbound apply, tested in internal_live_map.md — the main spec defines no public clear()) and no values() test (RTLM13 is untested everywhere, a known gap). Header corrected to RTLM5, RTLM10–RTLM12, RTLM20–RTLM21, RTLMV4, RTLCV4 — verified against the tests actually present. All claims cross-validated against objects-features.md (RTLM5/10-13/ 20/21/24, RTO10b1/b3, RTPO3a1, RTLC6d/RTLM6d).
Align the uts/objects test specs with the canonical objects-features.md on main, and fold in the test-correctness fixes from the objects UTS audit. Reconciliation with main: - Rename the internal CRDT type LiveMap/LiveCounter -> InternalLiveMap/ InternalLiveCounter (271 refs), and rename the four CRDT spec files to internal_live_*.md, repointing all cross-references. Blueprint .create() factories, public PathObject/Instance, and LiveMapUpdate/ LiveMapValue are deliberately left unchanged. - get() precondition RTO23b -> RTO23e: get() now performs ensure-active-channel (RTL33) -- re-attaches on DETACHED, rejects 90001 only on FAILED -- instead of unconditionally throwing 90001. - Rename blueprint value types LiveMapValueType/LiveCounterValueType -> LiveMap/LiveCounter (static .create() factories), matching main. - Consolidate RTO25 (access) / RTO26 (write) preconditions as dedicated sections in realtime_object.md, fixing 8 references to two precondition files that never existed; drop the mislabeled RTO25b-via-get() tests. - public_object_message: replace the ably-js-private _derivedFrom with a spec-neutral derivedFrom (RTLCV4g5/RTLMV4j5 keep this retained Create local-only and unnamed). Test-correctness fixes (objects UTS audit): - Quiescence-barrier pattern for negative subscription assertions. - Tombstone / echo-dedup handling; map-clear (RTLM24) and LWW-reject (RTLM9b) semantics; path depth-coverage (RTO24b2a1) fixes. - ACK-serial helper + SITE_CODE in standard_test_pool; explicit REST per-op cardinality; assorted spec-ID and expected-event corrections. Validated against main objects-features.md and cross-checked with ably-js; spec naming is preferred over ably-js where the two differ.
…specs
Implementing the reconciled objects specs against ably-js surfaced a
serial-ordering defect: the canonical ack_serial value "ack-{msgSerial}:{i}"
and the "s:1"/"s:2" serials in the RTO24a/RTO24c1 tests sort BEFORE the
standard pool's "t:0" entry timeserials under the string LWW comparison
(RTLM9), so a strictly-derived SDK rejects those operations as stale -
every local apply-on-ACK MAP_SET on a pool entry fails, and the RTO24c1
root MAP_SET never fires its subscription.
- ack_serial redefined to the sort-safe "t:" + (msgSerial + 1) + ":" + i
form, with a NOTE documenting the ordering constraint.
- All 8 inline auto-ACK harness copies now call ack_serial(msg.msgSerial, i)
instead of duplicating the literal, per the helper's own "never hardcode"
convention; the 3 literal "ack-0:0" uses in the echo-dedup and
ack-after-echo tests reference ack_serial(0, 0).
- RTO24a/RTO24c1 serials "s:1"/"s:2" changed to "t:1"/"t:2"; the adjacent
rationale comments rewritten to state both the site-level newness and
entry-level LWW ordering facts.
- Removed the stale note-only Test IDs RTLC12b/increment-requires-publish-0
and RTLM20d/echo-messages-false-0 (their sections carry no steps or
assertions - the preconditions live in realtime_object.md RTO26) so tag
sweeps stop reporting them as permanently missing tests.
- Added the missing RTLCV3c/no-validation-at-create-0 section to
value_types.md (no validation at blueprint creation; rejection happens
at evaluation per RTLCV4a).
Validated by the derived ably-js suite (ably/ably-js#2257): 309 passing,
0 failing with these values; the original values reproduce 7 failures.…l helpers
Correct three self-inconsistencies in the objects UTS unit specs (verified
against the ably-js reference tests and SDK):
- SI-1: counter object-states double-counted the created value. Counter sync is
additive (RTLC6c+RTLC6d/RTLC16: data = count + createOp.count), so `count:N` +
`createOp:N` materialised 2N and contradicted every consumer asserting N. Set
the residual `count` to 0 (createOp keeps the initial value) across all 16
affected fixtures in standard_test_pool, objects_pool, parent_references and
path_object_subscribe.
- SI-2: inbound MAP_SET/MAP_REMOVE on existing pool entries used bare serials
("99"/"100"/"101") that sort before the pool baseline "t:0" under lexicographic
LWW (RTLM9e), so they were rejected as stale and silently defeated the tests.
- SI-3: RTO20f reused the apply-on-ACK serial, which RTO9a3 echo-dedup discards
before the newness check (value stays 110, contradicting its own assertion of
120 and the sibling RTO20 dedup test). Use a serial that is not the ACK serial
but sorts below it, so it genuinely exercises RTLC7c.
Add a named serial vocabulary to the Canonical Constants so the LWW ordering
intent is explicit and used at every synthetic inbound serial: POOL_SERIAL
(baseline), remote_serial(i) (remote op that wins vs the baseline) and
below_ack_serial(i) (a non-ACK serial that loses to the ACK serial).…e RTPO19e2 comment
standard_test_pool.md: a createOp is a full ObjectOperation whose
`action` and `objectId` are mandatory (OOP2) and validated by SDKs
before merging — yet every fixture in the module writes the terse form
`createOp: { counterCreate: {...} }` and build_object_state copied it
verbatim, producing wire-invalid states as literally written. Both SDK
suites already compensate (ably-java's builders embed action and inject
objectId; ably-js spells both out in every fixture). Codify that:
build_object_state now fills in missing `objectId` (from the state's
objectId) and `action` (COUNTER_CREATE/MAP_CREATE by payload), keeping
all terse fixtures valid without edits. Explicit fields are preserved,
so deliberately-mismatched negative fixtures remain expressible.
path_object_subscribe.md: rewrite the stale RTPO19e2 comment (it
described an older two-message MAP_SET flow) to match the actual
fixture: a single OBJECT_SYNC updating counter:score via replaceData
(100 -> 200, RTLC6), with root intentionally omitted — per RTO5c2a the
root must never be removed from the pool, so the update stays reachable
and dispatches with message omitted.Review comments (PR #499): - Grammar: "a Internal*" -> "an Internal*" at the 4 flagged sites plus a 5th the review missed (internal_live_map.md tombstone prose). - Add objectsGCGracePeriod: 86400000 to the 3 minimal mock CONNECTED connectionDetails (RTO23a, RTO23e-DETACHED, RTO23e-FAILED) — the other 12 blocks already carried it, and 86400000 is the spec's own default (RTO10b3); omitting it invited derived-test failures for fixture reasons unrelated to each test's intent. - Link the LiveObjects REST API docs (built from the OpenAPI spec) in the V2-format REST provisioning section. Naming audit (LiveMap/LiveCounter vs InternalLive*): - RTPO3 heading + requirement row said path resolution walks "LiveMaps"; per RTPO3a1 the walked objects are InternalLiveMaps — the section body was renamed but the heading/table lagged. - build_object_state comment: createOp validation happens on the internal CRDT object — "a LiveCounter rejects" -> "an InternalLiveCounter rejects" (comment was added after the mass rename, so no sweep could catch it). Behavioral audit: - internal_live_map_api.md header over-claimed RTLM24 and RTLM13: the file has no MAP_CLEAR test (RTLM24 is the inbound apply, tested in internal_live_map.md — the main spec defines no public clear()) and no values() test (RTLM13 is untested everywhere, a known gap). Header corrected to RTLM5, RTLM10–RTLM12, RTLM20–RTLM21, RTLMV4, RTLCV4 — verified against the tests actually present. All claims cross-validated against objects-features.md (RTLM5/10-13/ 20/21/24, RTO10b1/b3, RTPO3a1, RTLC6d/RTLM6d).
sacOO7force-pushed
the
chore/fix-uts-liveobjects-spec
branch
from
July 10, 2026 11:53
6abea18 to
b7b62d1ComparesacOO7
marked this pull request as ready for review
July 10, 2026 11:53
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
The canonical LiveObjects spec (
specifications/objects-features.md) onmainhas moved ahead of the version theuts/objectstest specs were originally written against. This PR reconciles the test specs withmainand folds in the test-correctness fixes identified during the objects UTS audit, so the derived SDK tests are generated from a spec-faithful, internally-consistent source.Source of truth:
objects-features.mdonmain, cross-checked against the ably-jsliveobjectsimplementation. Where ably-js naming differs from the spec, the spec wins.What changed
1. Internal CRDT naming →
InternalLiveMap/InternalLiveCountermainrenames the internal CRDT graph objects to disambiguate them from the public API and the create-blueprints. Applied across 271 references (constructions,ISassertions,.diff,#methoddelegations, and prose).The four CRDT spec files were renamed to match (the skill resolver derives the test class name from the filename, so generated classes become
InternalLive*automatically):unit/live_counter.mdunit/internal_live_counter.mdunit/live_map.mdunit/internal_live_map.mdunit/live_counter_api.mdunit/internal_live_counter_api.mdunit/live_map_api.mdunit/internal_live_map_api.mdLeft unchanged on purpose: blueprint
.create()factories, publicPathObject/Instance,LiveObject(base), andLiveMapUpdate/LiveCounterUpdate/LiveMapValue.2.
get()precondition:RTO23b→RTO23e(ensure-active-channel)RTO23b(throw90001on DETACHED/FAILED) was replaced byRTO23e(perform ensure-active-channel,RTL33):RTO23b)RTO23e/RTL33)90001PathObject9000190001(viaRTL33c)Read/subscribe access methods are a separate check (
RTO25b, still90001on DETACHED/FAILED) — verified against ably-js (get()→ensureAttached(); access methods →throwIfInvalidAccessApiConfiguration()).3. Blueprint value types →
LiveMap/LiveCounterLiveMapValueType/LiveCounterValueTyperenamed toLiveMap/LiveCounter(immutable blueprints with static.create()), matchingmainand the ably-js public exports (LiveMapValueType as LiveMap,LiveCounterValueType as LiveCounter).4. RTO25 / RTO26 preconditions consolidated into
realtime_object.mdThe spec centralised the per-method preconditions into
RTO25(access) /RTO26(write), and the per-method clauses now read "replaced by RTO25/RTO26". Eight notes pointed readers to two files (rto25_access_preconditions.md,rto26_write_preconditions.md) that never existed.realtime_object.md(RTO25a, RTO25b ×2, RTO26a, RTO26b ×2, RTO26c), grouped after theget()tests.realtime_object.mdwith section qualifiers.RTO25b-via-get()tests; the genuineRTO25btests now exercise a real access method (root.keys()) on DETACHED/FAILED.5.
public_object_message: spec-neutralderivedFromReplaced the ably-js-private field name
_derivedFromwithderivedFrom(+ a comment): the spec (RTLCV4g5/RTLMV4j5) keeps the retainedCounterCreate/MapCreatelocal-only and unnamed, so the UTS pseudocode should not borrow an SDK-internal field name. The assertions already read spec-presentPAOOPfields only.6. Test-correctness fixes (objects UTS audit)
Independent of the
maindelta, folded in here:RTLM24) and LWW-reject (RTLM9b) semantics; path depth-coverage (RTO24b2a1) fixes.ack_serialhelper + exposedSITE_CODEinstandard_test_pool; explicit REST per-op cardinality; assorted spec-ID and expected-event corrections (e.g.RTPO13c5,RTPO3c2→92005,RTINS16g).Validation
diffof working-copy vsmainobjects-features.md, normalising the two renames to isolate the single behavioural change (RTO23b→RTO23e).LiveObjectpreserved; every asserted internal field is spec-named.get()/ access / write precondition behaviour and the value-type exports against ably-js.Notes / out of scope
RTO26bcovers DETACHED/FAILED/SUSPENDED but the suite only tests DETACHED/FAILED. This is a pre-existing coverage gap, not a regression; happy to addRTO26b/write-throws-suspended-0if wanted.uts/directory still reference the old filenames; they are historical records and were intentionally left untouched.