fix(messaging): persist own-message IDs so bubbles stay right-aligned after reload #210

Merged

rawdaGastan merged 1 commit from development_fix_bubble_own_after_reload into development

2026-05-06 12:28:21 +00:00

rawdaGastan commented

2026-05-06 12:19:29 +00:00

Member

Summary

Fixes the post-reload regression of #62: after F5, every message bubble was rendering as bubble-other (left-aligned, no checkmark) because the optimistic-send patch in archipelago.rs only fired in handle_send, while the chat-load path refetched messages from the server with sender_public_key = "system" and no way to identify which were authored locally.

This PR persists the SIDs of locally-authored messages in a per-context localStorage set and falls back to a contains() check inside chatmessage_to_messagedata whenever the existing own_user_key match doesn't fire. Once auth lands and the server stamps a real sender_public_key, the existing key-match path takes precedence and the persisted-IDs path becomes a no-op.

Also bundles a 9-line mut patch to server/src/main.rs that unblocks the showcase wasm build (11 pre-existing E0596 errors on development were preventing make run from compiling).

Closes #192

Changes

archipelagos/messaging/Cargo.toml — added "Storage" to the web-sys features list.
archipelagos/messaging/src/services/messaging_service.rs:
- Added pub struct OwnMessageIds (HashSet + ordered Vec, FIFO-capped at MAX_OWN_IDS_PER_CONTEXT = 5_000).
- Added pub fn load_own_message_ids(context_name) / pub fn save_own_message_ids(context_name, &ids) with cfg-gated localStorage on wasm32 (key hero_messaging_own_msg_ids:<context>) and a no-op on native.
- Extended chatmessage_to_messagedata to take a 4th &OwnMessageIds argument; is_own = (key match) || own_ids.contains(&msg.sid.to_string()); sender_name = "Me" whenever is_own.
archipelagos/messaging/src/archipelago.rs:
- Loads the persisted set on mount via use_signal(|| services::load_own_message_ids(&context_name)).
- Threads &own_message_ids.peek() into all 4 chatmessage_to_messagedata call sites (initial fetch, SSE Resync, SSE ChatmessageNew, send path).
- On successful send: inserts msg.sid into the set and writes through to localStorage. Existing optimistic patch (msg_data.is_own = true; msg_data.sender_name = "Me") preserved.
server/src/main.rs — added mut to 9 use_signal bindings (arch_transitioning, island_transitioning, paradigm_visible, wasm_visible, ai_visible, backend_visible, cta_visible, cards_visible, active_section) so the showcase wasm build compiles.

Test Results

cargo test -p hero_archipelagos_messaging --lib: 5/5 passed.

services::messaging_service::own_message_ids_tests::test_insert_dedupes — own-id set dedupes on repeat insert.
services::messaging_service::own_message_ids_tests::test_insert_fifo_cap — FIFO eviction at MAX_OWN_IDS_PER_CONTEXT.
services::messaging_service::chatmessage_conversion_tests::test_own_id_match_returns_is_own_true — sid in set => is_own=true even when sender is "system".
services::messaging_service::chatmessage_conversion_tests::test_other_party_in_dm_remains_other — DM regression guard: "system" sender NOT in own-ids => is_own=false (the option-B false-positive the issue calls out).
services::messaging_service::chatmessage_conversion_tests::test_real_key_takes_precedence — once auth lands, key match wins regardless of own-ids.

cargo check --workspace --exclude hero_archipelagos_messaging: clean.

cargo check -p archipelagos_server --target wasm32-unknown-unknown: clean (after the mut patch).

Live verified in browser: make run -> http://localhost:8886 -> messaging island -> sent messages render bubble-own, F5, re-open chat, messages still render bubble-own. localStorage entry hero_messaging_own_msg_ids:<context> populated. No re-fetch storm on send.

Test plan

Unit tests cover refetched own-id match (test_own_id_match_returns_is_own_true)
Unit tests cover DM false-positive guard (test_other_party_in_dm_remains_other)
Unit tests cover forward-compat with auth (test_real_key_takes_precedence)
Unit tests cover FIFO cap (test_insert_fifo_cap)
Native + wasm32 builds compile clean
Manual browser repro: send, F5, reload bubbles render bubble-own
Manual browser repro: localStorage entry populated and scoped per-context

Notes

An early version of the wiring made the chat-load use_effect reactively subscribe to own_message_ids so the message list would re-derive when the set grew. That caused a re-fetch storm on every send (visible as a screen jump). Replaced with .peek() everywhere — the set only matters on the initial chat load after F5; mid-session, the optimistic patch in handle_send already does the right thing.
localStorage is per-origin per-browser. Different machine / private window will see historical messages render bubble-other until real auth lands — acceptable degradation given that auth is the proper fix.
Once the server stamps a real sender_public_key, the persisted-IDs path becomes dead code. Retire in a follow-up: delete OwnMessageIds, drop the parameter from chatmessage_to_messagedata, one-time-clear hero_messaging_own_msg_ids:* keys on next load.

Out of scope

Server-side fix to stamp sender_public_key with X-Hero-Context (option C in the issue) — tracked in hero_osis #40.

## Summary Fixes the post-reload regression of #62: after F5, every message bubble was rendering as `bubble-other` (left-aligned, no checkmark) because the optimistic-send patch in `archipelago.rs` only fired in `handle_send`, while the chat-load path refetched messages from the server with `sender_public_key = "system"` and no way to identify which were authored locally. This PR persists the SIDs of locally-authored messages in a per-context `localStorage` set and falls back to a `contains()` check inside `chatmessage_to_messagedata` whenever the existing `own_user_key` match doesn't fire. Once auth lands and the server stamps a real `sender_public_key`, the existing key-match path takes precedence and the persisted-IDs path becomes a no-op. Also bundles a 9-line `mut` patch to `server/src/main.rs` that unblocks the showcase wasm build (11 pre-existing E0596 errors on `development` were preventing `make run` from compiling). ## Related Issue Closes https://forge.ourworld.tf/lhumina_code/hero_archipelagos/issues/192 ## Changes - **`archipelagos/messaging/Cargo.toml`** — added `"Storage"` to the `web-sys` features list. - **`archipelagos/messaging/src/services/messaging_service.rs`**: - Added `pub struct OwnMessageIds` (`HashSet` + ordered `Vec`, FIFO-capped at `MAX_OWN_IDS_PER_CONTEXT = 5_000`). - Added `pub fn load_own_message_ids(context_name)` / `pub fn save_own_message_ids(context_name, &ids)` with cfg-gated `localStorage` on wasm32 (key `hero_messaging_own_msg_ids:<context>`) and a no-op on native. - Extended `chatmessage_to_messagedata` to take a 4th `&OwnMessageIds` argument; `is_own = (key match) || own_ids.contains(&msg.sid.to_string())`; `sender_name = "Me"` whenever `is_own`. - **`archipelagos/messaging/src/archipelago.rs`**: - Loads the persisted set on mount via `use_signal(|| services::load_own_message_ids(&context_name))`. - Threads `&own_message_ids.peek()` into all 4 `chatmessage_to_messagedata` call sites (initial fetch, SSE `Resync`, SSE `ChatmessageNew`, send path). - On successful send: inserts `msg.sid` into the set and writes through to localStorage. Existing optimistic patch (`msg_data.is_own = true; msg_data.sender_name = "Me"`) preserved. - **`server/src/main.rs`** — added `mut` to 9 `use_signal` bindings (`arch_transitioning`, `island_transitioning`, `paradigm_visible`, `wasm_visible`, `ai_visible`, `backend_visible`, `cta_visible`, `cards_visible`, `active_section`) so the showcase wasm build compiles. ## Test Results `cargo test -p hero_archipelagos_messaging --lib`: 5/5 passed. - `services::messaging_service::own_message_ids_tests::test_insert_dedupes` — own-id set dedupes on repeat insert. - `services::messaging_service::own_message_ids_tests::test_insert_fifo_cap` — FIFO eviction at `MAX_OWN_IDS_PER_CONTEXT`. - `services::messaging_service::chatmessage_conversion_tests::test_own_id_match_returns_is_own_true` — sid in set => `is_own=true` even when sender is `"system"`. - `services::messaging_service::chatmessage_conversion_tests::test_other_party_in_dm_remains_other` — DM regression guard: `"system"` sender NOT in own-ids => `is_own=false` (the option-B false-positive the issue calls out). - `services::messaging_service::chatmessage_conversion_tests::test_real_key_takes_precedence` — once auth lands, key match wins regardless of own-ids. `cargo check --workspace --exclude hero_archipelagos_messaging`: clean. `cargo check -p archipelagos_server --target wasm32-unknown-unknown`: clean (after the `mut` patch). **Live verified in browser**: `make run` -> `http://localhost:8886` -> messaging island -> sent messages render `bubble-own`, F5, re-open chat, messages still render `bubble-own`. localStorage entry `hero_messaging_own_msg_ids:<context>` populated. No re-fetch storm on send. ## Test plan - [x] Unit tests cover refetched own-id match (`test_own_id_match_returns_is_own_true`) - [x] Unit tests cover DM false-positive guard (`test_other_party_in_dm_remains_other`) - [x] Unit tests cover forward-compat with auth (`test_real_key_takes_precedence`) - [x] Unit tests cover FIFO cap (`test_insert_fifo_cap`) - [x] Native + wasm32 builds compile clean - [x] Manual browser repro: send, F5, reload bubbles render `bubble-own` - [x] Manual browser repro: localStorage entry populated and scoped per-context ## Notes - An early version of the wiring made the chat-load `use_effect` reactively subscribe to `own_message_ids` so the message list would re-derive when the set grew. That caused a re-fetch storm on every send (visible as a screen jump). Replaced with `.peek()` everywhere — the set only matters on the initial chat load after F5; mid-session, the optimistic patch in `handle_send` already does the right thing. - localStorage is per-origin per-browser. Different machine / private window will see historical messages render `bubble-other` until real auth lands — acceptable degradation given that auth is the proper fix. - Once the server stamps a real `sender_public_key`, the persisted-IDs path becomes dead code. Retire in a follow-up: delete `OwnMessageIds`, drop the parameter from `chatmessage_to_messagedata`, one-time-clear `hero_messaging_own_msg_ids:*` keys on next load. ## Out of scope - Server-side fix to stamp `sender_public_key` with `X-Hero-Context` (option C in the issue) — tracked in hero_osis #40.

rawdaGastan added 1 commit

2026-05-06 12:19:29 +00:00

fix(messaging): persist own-message IDs so bubbles stay right-aligned after reload

Build and Test / build (pull_request) Successful in 7m1s

Details

88d1f8295e

Optimistic-send patch in #62 only set is_own=true on the just-sent message;
after F5 every bubble rendered as bubble-other. Fix: persist the SIDs of
locally-authored messages in a per-context localStorage set
(MAX_OWN_IDS_PER_CONTEXT=5_000, FIFO-evicted) and have
chatmessage_to_messagedata fall back to a contains() check on that set
when own_user_key is the empty/system value the server stamps pre-auth.

Once auth lands and sender_public_key is real, the existing key-match
path takes precedence and the persisted-IDs path becomes dead code,
verified by test_real_key_takes_precedence.

Also marks 9 use_signal bindings in server/src/main.rs as mut to unblock
the showcase wasm build, which was failing on development with 11 E0596
errors before any of this change.

#192