perf(ui): drop dead-code parent_state clone in apply_delta/update_room_state (#246)

[sanity]

Follow-up to #311 / #246. Ivvor reports memory spikes on cold open dropped from the 16GB-crashing pre-#311 level to 3-4GB-but-stable. Real user-visible win on the "doesn't crash my iPad anymore" axis, but 3-4GB is still big — this PR chips at the next per-event allocation source.

What changes

apply_delta_inner and update_room_state_inner in room_synchronizer.rs each cloned the full room_data.room_state to pass as the parent_state argument to apply_delta / merge. That argument is dead-code at the macro level:

• Every field's summarize / delta impl in common/src/room_state/ takes _parent_state (underscore-prefixed = unused, verified across ban.rs, configuration.rs, direct_messages.rs, member_info.rs, member.rs, message.rs, secret.rs, upgrade.rs, version.rs).
• freenet-scaffold-macro 0.2.2 generates an apply_delta that ignores its outer _parent_state arg entirely. It computes a fresh let self_clone = self.clone(); per field and uses that as the per-field parent.

So the caller-side full-state clone never reaches code that reads it. Substituting &ChatRoomStateV1::default() is provably equivalent. Saves one full-state clone per network delta / state update.

Test coverage

New regression test merge_with_default_sentinel_parent_matches_merge_with_self_clone_parent empirically pins the invariant — it runs merge both ways and asserts byte-equality of the post-merge state. If a future refactor starts reading the outer parent_state (in the macro, or in a field's summarize / delta), this test fails and the optimization needs revisiting.

cargo test -p river-ui --bins: 242/242 pass. WASM check green. cargo fmt --check clean.

Honest sizing — what this does NOT fix

This is a ~10% per-merge reduction. The dominant allocation source remains the macro-internal let self_clone = self.clone(); per field inside apply_delta (~9× full-state clones per merge for ChatRoomStateV1). Fixing that would mean modifying freenet-scaffold-macro to hoist self_clone outside the field loop — single shared clone reused across all fields — which is a separate-crate PR.

This PR is worth landing on its own merits (provably safe, focused diff, real-but-bounded reduction), with the bigger macro refactor tracked as a follow-up if the residual spike still bothers users after this lands.

Deploy notes

UI-only change. Single file: ui/src/components/app/freenet_api/room_synchronizer.rs. No delegate/contract WASM modified, no migration entry needed, no riverctl republish needed.

🤖 Generated with Claude Code

[sanity]

PR Review (Light tier — appropriate for a 1-file, +83/-5 perf change)

• Tier: Light — single-file UI-only perf change. Codex external pass + skeptical-reviewer adversarial read.
• CI: in progress on c556b795 at review time. Per no-merge-with-red-ci, must be green on the current HEAD before merge.

Findings

Reviewer	Result
Codex external	Clean, no findings. Verified the substitution is safe under current macro + field impls.
Skeptical	Confirmed the substitution is correct. One Medium finding: the round-1 regression test was tautological on near-default data — state_a only diverged from default() in recent_messages.messages, so a hypothetical macro regression that forwards outer _parent_state to a field's apply_delta would still see equivalent values across both paths and the test would silently pass.

Round 2 (c556b795) — addresses the Medium finding

Round-2 commit is purely test + comment, zero production-code changes (verified via diff-of-the-diff). Per the skill's "trivial mechanical findings" allowance, a Light re-check is sufficient — that's what this comment is.

• Test now sets state_a with a non-default Configuration (max_members = 3 vs default 200, max_recent_messages = 5 vs default 100) AND a non-owner AuthorizedUserBan. Those are the exact fields MembersV1::apply_delta, MessagesV1::apply_delta, and MemberInfoV1::apply_delta read from parent_state. The test now genuinely discriminates the regression class it claims to guard.
• Comment on the merge call site spelled out that safety on the merge path depends on BOTH the macro's apply_delta ignoring outer _parent_state AND every field-level summarize/delta declaring it unused — the previous comment deferred to the apply_delta_inner rationale, which only covered one of those two facts.

Non-blocking observations (skeptical's other points)

• Scope: similar &room_state.clone() patterns exist in cli/src/api.rs:1656, 1750, 1902, 1961 and a few other UI sites. Deliberately deferred — this PR targets the open-time hot path. Worth a follow-up if the CLI's allocation profile turns out to matter.
• Macro version pinning: the optimization's safety is tied to freenet-scaffold-macro 0.2.2. The Cargo.toml currently uses caret semver "0.2.2". The regression test catches any future macro change that would break the substitution, so a hard =0.2.2 pin isn't required — but worth keeping the test green-on-CI as the guard.
• ChatRoomStateV1::default() cost: trivial — constructs 9 default sub-field types, all small. Strictly cheaper than cloning a real room_state that's grown beyond defaults (which is the cold-open scenario).

Verdict

Ready to merge pending CI green on c556b795. Substitution provably safe under current code. Round-1 finding addressed. Honest sizing: ~10% per-merge reduction (macro-internal per-field clones still dominate); won't bring 3-4 GB residual spike down to MB but every win compounds.

[AI-assisted - Claude]