Remove dead _updateParentReferencesFromUpdate helper

[sacOO7]

Summary

Removes the _updateParentReferencesFromUpdate helper from LiveMap along with its sole call site inside overrideWithObjectState. The helper was dead weight: every code path that invoked it was immediately followed by RealtimeObject._rebuildAllParentReferences (spec'd at RTO5c10), which unconditionally clears every parentReferences map and re-derives them from scratch based on the final post-sync pool state. The helper's intermediate updates were always overwritten before any external observer could see them.

Why this was dead code

_updateParentReferencesFromUpdate had exactly one call site: LiveMap.overrideWithObjectState. That method, in turn, is invoked from only three places — all of them inside the OBJECT_SYNC processing flow under RTO5c:

Caller	Spec
RealtimeObject._applyObjectSync (existing object)	RTO5c1a1
LiveCounter.fromObjectState factory	RTO5c1b1a
LiveMap.fromObjectState factory	RTO5c1b1b

After RTO5c1/RTO5c2 finish, RTO5c10 (_rebuildAllParentReferences) executes unconditionally:

1. RTO5c10a: clearParentReferences() on every object in the pool.
2. RTO5c10b: re-iterate every LiveMap and re-establish parent refs via addParentReference for each objectId-valued entry.

So any parent refs set by _updateParentReferencesFromUpdate inside overrideWithObjectState were wiped by RTO5c10a and re-derived by RTO5c10b before sync completion was visible to subscribers.

Tombstone branch — also redundant

In the tombstone path (update = this.tombstone(objectMessage)), LiveObject.tombstone() calls clearData(). For LiveMap, clearData() is overridden to iterate every entry and call removeParentReference per RTLO4e5 — before the data is zeroed. By the time overrideWithObjectState reached _updateParentReferencesFromUpdate, all parent refs were already cleaned. The helper would iterate the 'removed' diff keys and call removeParentReference again on the same objects — a harmless no-op (removeParentReference is idempotent when the parent is absent per RTLO3f3a) but wasted work.

What changed

src/plugins/liveobjects/livemap.ts:

1. Removed the call site inside LiveMap.overrideWithObjectState (was lines 461–462).
2. Removed the method definition (was lines 1099–1137, 39 lines including JSDoc).
3. Scope-tightened previousDataRef: moved its declaration inside the else branch of overrideWithObjectState since it's only consumed there (by _updateFromDataDiff). The tombstone branch never reads it.

Total: 1 insertion, 44 deletions.

Other parent-ref maintenance is unaffected

Outside the sync flow, parentReferences continues to be maintained by the per-operation handlers, each with its own spec coverage:

• MAP_SET → RTLM7a2f (remove old) and RTLM7i (add new)
• MAP_REMOVE → RTLM8a2e
• MAP_CLEAR → RTLM24e1c
• LiveMap tombstone → RTLO4e5
• Post-sync rebuild → RTO5c10b1

None of these call the removed helper.

Spec impact

None. RTLM6 (overrideWithObjectState) doesn't mention parentReferences maintenance, and the operation handlers above already cover every parent-ref mutation the SDK is required to perform.

Test plan

• npm run build:liveobjects — passes (TypeScript compiles cleanly).
• npm run lint — passes (only pre-existing warnings in react-hooks fakes, unrelated).
• Repo-wide ripgrep for _updateParentReferencesFromUpdate returns zero hits after the change.
• CI to run the full mocha suite (npm test), in particular test/realtime/liveobjects.test.js, which exercises the OBJECT_SYNC flow extensively.
• Reviewer to verify that sync scenarios involving nested LiveMap references and path-based subscriptions (RTPO19) still emit the expected events — these end-states depend on RTO5c10, which is unchanged.

Risk

Low. The diff is a pure deletion in one file, the method is private (no external callers possible), and the call-graph audit confirms every invocation path runs RTO5c10 immediately afterward. Rollback is a single-commit revert.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Refactor
  • Optimized internal handling of object state processing to reduce code complexity and improve efficiency.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 03b71cd1-fbf1-46f3-abde-4fd899e28f6e

📥 Commits

Reviewing files that changed from the base of the PR and between 524a516 and bad7f09.

📒 Files selected for processing (1)

• src/plugins/liveobjects/livemap.ts

---

Walkthrough

The LiveMap.overrideWithObjectState method is refactored to reorganize variable initialization and remove parent reference updates. The previousDataRef snapshot is now created only in the non-tombstoned path, and the call that previously applied parent-reference modifications is removed. A minor structural adjustment is made to _isMapEntryTombstoned.

Changes

LiveMap state override refactoring

Layer / File(s)	Summary
Refactor overrideWithObjectState to remove parent reference updates src/plugins/liveobjects/livemap.ts	update is declared before the tombstone check; previousDataRef snapshot creation is moved into the non-tombstoned path only; the post-diff parent reference update call is removed, so the method returns the computed update without applying parent-reference modifications in this override flow.
Adjust _isMapEntryTombstoned ending src/plugins/liveobjects/livemap.ts	The terminating lines of _isMapEntryTombstoned are adjusted for structural placement.

🎯 2 (Simple) | ⏱️ ~8 minutes

Suggested reviewers

• mschristensen
• VeskeR

🐰 A rabbit hops through the code so clean,
Where tombstones rest and parents unseen,
No reference updates in the override's way,
Just cleaner logic, brighter each day! ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 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: removing the _updateParentReferencesFromUpdate helper method from LiveMap, which is the primary objective and primary code change in this PR.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch cleanup/remove-update-parent-references-from-update

---

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]

I think it's worth having a top-level comment on overrideWithObjectState's and fromObjectState's documentation to explain that they are only called from within the sync and thus in the case of a LiveMap why they do not need to fix up their children's parentReferences, because otherwise somebody looking at the implementation is going to think there's a bug — "this is updating the object's data but it's not updating parentReferences, why?"

[lawrence-forooghian]

Also could you please re-check the spec point references in the PR description and commit message against spec main? I asked Claude to do a quick check and it said there are some divergences

[lawrence-forooghian]

You could also argue that clearData() suffers from the same problem, i.e. when it's called within the context of a sync then its fixing-up of parentReferences is also redundant. The difference is that sometimes clearData()'s fixing-up is needed e.g. in the case when applying an OBJECT_DELETE, so it would need to accept a flag to tell it whether to do this work. What are your thoughts? I don't think it's something we need to do here, though.

[sacOO7]

You could also argue that clearData() suffers from the same problem, i.e. when it's called within the context of a sync then its fixing-up of parentReferences is also redundant. The difference is that sometimes clearData()'s fixing-up is needed e.g. in the case when applying an OBJECT_DELETE, so it would need to accept a flag to tell it whether to do this work. What are your thoughts? I don't think it's something we need to do here, though.

I have cross checked multiple times whether _updateParentReferencesFromUpdate is needed or not. Currently the operation performed by OBJECT_SYNC is overridden by parentReferences reset at the end of OBJECT_SYNC, so it's truely not needed. Though, I can update doc comments at appropriate places.

[Copilot]

Pull request overview

Removes the dead _updateParentReferencesFromUpdate private helper from LiveMap and its sole call site in overrideWithObjectState. Every invocation path of overrideWithObjectState is part of the OBJECT_SYNC flow (RTO5c), which immediately follows up with RealtimeObject._rebuildAllParentReferences (RTO5c10) — clearing and re-deriving all parent references from pool state — making the helper's work redundant. previousDataRef is also scope-tightened into the else branch where it's actually consumed.

Changes:

• Remove call to _updateParentReferencesFromUpdate from overrideWithObjectState.
• Delete the private _updateParentReferencesFromUpdate method.
• Move previousDataRef declaration inside the non-tombstone branch.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[lawrence-forooghian]

You could also argue that clearData() suffers from the same problem, i.e. when it's called within the context of a sync then its fixing-up of parentReferences is also redundant. The difference is that sometimes clearData()'s fixing-up is needed e.g. in the case when applying an OBJECT_DELETE, so it would need to accept a flag to tell it whether to do this work. What are your thoughts? I don't think it's something we need to do here, though.

I have cross checked multiple times whether _updateParentReferencesFromUpdate is needed or not. Currently the operation performed by OBJECT_SYNC is overridden by parentReferences reset at the end of OBJECT_SYNC, so it's truely not needed. Though, I can update doc comments at appropriate places.

Not sure I understand your reply here — I wasn't disputing the changes of this PR, I was making a further point about clearData()