[Mathih13]

One Module transaction can write group events for many recipients. A 40-member Raid roster list is one example. A loot roll is another. For each recipient with a session, the Gateway copied the whole cached `game_group_event` table to find that recipient's rows. The coordinator pump repeated the same copy once per recipient, so the work grew with recipients times cached rows.

Now each source database keeps one snapshot of the current transaction. The first insert callback of a transaction reads the table once and groups its rows by recipient. Each recipient takes its rows from that snapshot. A recipient with no session still causes no read. The insert-order guarantee and the per-database cursor do not change.

The existing `by_recipient` index is on `recipient_identity`, so it does not help here. This change adds no index and no schema change.

Verification, all in `lyracore-gateway`:

• `cargo test -p lyracore-gateway group_event_order` passes. It includes `one_transaction_reads_the_table_once_for_every_recipient`. The test fires 40 recipients' callbacks in reverse order, counts one table read, and checks that each recipient gets its own rows in insert order.
• `the_next_transaction_reads_the_table_again` shows that a next transaction with the same timestamp gets its own read.
• The existing ordering and cursor tests pass unchanged. `the_realm_private_relays_ride_the_recipient_keyed_dispatchers` passes.
• `cargo fmt --check` is clean.

Closes #607

Model and harness: Claude Sonnet 5.5 in Claude Code.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 28 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 679051f6-982b-44e5-b65e-56ce1b6f4bcd

📥 Commits

Reviewing files that changed from the base of the PR and between f454a98 and 2bb13e0.

📒 Files selected for processing (1)

• gateway/src/stdb/world_view.rs

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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

[Mathih13]

Code review: Standards and Spec

Range: main...19d8297 (1 commit, gateway/src/stdb/world_view.rs only). Spec: #607. I ran cargo test -p lyracore-gateway group_event_order locally and all 4 tests pass.

Standards

Sources: CODING_STANDARDS.md, AGENTS.md, CONTEXT.md, and the Fowler smell baseline.

No hard violations. The comments explain the constraint and carry no issue numbers. The comment on GroupEventCursor::take is updated to match. The poisoned-lock handling matches take. The lock order is cursor first, then snapshot. Nothing takes them in the other order, so there is no deadlock risk.

P2: the many-recipient test models one cursor per row, not per viewer (one_transaction_reads_the_table_once_for_every_recipient). The loop makes GroupEventCursor::default() for each row. In production, each viewer holds one cursor. With a fresh cursor per row, both callbacks for a member relay both rows, so relayed has 80 entries where it should have 40. contains hides the duplicate. The test passes, but it does not check the real path, where the second callback finds its row taken and does not read the snapshot. Fix: keep one cursor per recipient (for example a HashMap<u64, GroupEventCursor>), then assert that each member is relayed exactly once with [first, first + 1]. Standard: "Test behavior through a caller-facing interface... Derive expected values from the Spec."

P3: possible Duplicated Code. TransactionRows::collect repeats take's logic: filter on created_at, then add row if the table lacks it. take still filters on recipient_guid and created_at, and the snapshot already did both. Two places now own the "table lacks the row" fallback. This is a judgement call. You could keep take as the only owner of filtering and the fallback, and have the snapshot only group by recipient. Standard: "Remove a displaced path when the replacement covers it."

P3, note only: the fallback costs a read for each row. If the cached table ever lacks a row, holds fails for each later row of that transaction, and each one reads the table again. That is the old cost, not a regression, and the documented promise ("the cached table holding the whole transaction") rules it out. No action needed unless that promise is weak.

Spec

Spec: #607. "Done when: each source database collects one transaction's rows once, and each recipient takes its slice from that snapshot. The ordering guarantee and the per-database cursor from #605 stay the same. A test proves one table read per transaction for many recipients."

• Met: one collection for each source database. wire_group_events runs once for each source (Shard(shard), RealmCore) and makes one GroupEventSnapshot per call.
• Met: each recipient takes its slice. recipient_rows returns by_recipient[recipient_guid].
• Met: ordering and cursor unchanged. take still sorts by id and advances the cursor for each source. Both existing tests pass unchanged.
• Met, with the P2 above: the test proves one read. It asserts reads == 1 over 40 recipients. It does not use the per-viewer cursor that the spec says must "stay the same", so it shows one read but does not show the full relay result.
• Scope creep: none. No index and no schema change, which matches the issue's note on by_recipient.
• Edge cases checked: a next transaction with the same created_at re-reads, because its new ids are not held (covered by the_next_transaction_reads_the_table_again). A recipient with no session still causes no read.

---

Summary: Standards has 3 findings. The worst is P2: the many-recipient test uses a fresh cursor per row and hides duplicate relays. Spec has 0 missing requirements and 1 partial: the read-count test does not use the real per-viewer cursor. None of the findings blocks merge. I recommend fixing the P2 test.

---

Generated by Claude Code

[Mathih13]

2bb13e0 fixes the routine review and an independent Opus review:

• P2: the many-recipient test keeps one cursor per member and asserts each member relays its two rows once, and its second callback relays nothing.
• P3: the snapshot now holds only the table read, grouped by recipient. GroupEventCursor::take is the only owner of transaction selection, ordering and the row fallback.
• The Mutex is gone, because one pump thread owns the on_insert closure. Production and the tests both go through GroupEventSnapshot::take.
• The note on a repeated read when the cache lacks a row: no change. That is the old cost, and the SDK applies the whole transaction to the cache before it fires the callbacks.