Skip to content

feat(mentions): reference members by truncated-base32 in mention tokens#373

Merged
sanity merged 2 commits into
mainfrom
worktree-fix-mention-base32
Jun 29, 2026
Merged

feat(mentions): reference members by truncated-base32 in mention tokens#373
sanity merged 2 commits into
mainfrom
worktree-fix-mention-base32

Conversation

@sanity

@sanity sanity commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Problem

A mention rendered as @[Ivvor](rv:422a2a8d3edfea2b), but the same member is shown as FPVN6PUN everywhere else in the UI/CLI. The rv: reference was the 16-char hex of the member's 64-bit MemberId, while the rest of the app names members by their 8-char truncated-base32 Display label. Same id, two encodings — inconsistent and confusing.

Approach

Make the mention wire token use the truncated-base32 short label (rv:FPVN6PUN) so mentions read consistently with how members are named everywhere else.

The short label is lossy (40 of 64 bits), so it can't be decoded straight back to a MemberId. Instead a parsed mention now carries a MemberRef:

  • MemberRef::Short(String) — the current form; resolved against the room's known members via MemberRef::matches / resolve. This is the same "short id uniquely names a member within a room" assumption the CLI/UI already rely on (ban-by-short-id, DM recipient short id, etc.).
  • MemberRef::Legacy(MemberId) — a full id decoded from the legacy rv:<hex> form.

Backward compatible. Messages already in circulation carry the hex form; the parser still accepts it (member_ref_from_str tries base32 first, falls back to hex). New tokens are never emitted in hex. Both the hex parse branch and the Legacy variant carry a TODO(mentions) to remove once no legacy token remains in circulation.

The two forms can't collide: the current form is exactly 8 base32 chars (A–Z/2–7), the legacy form is up to 16 hex chars, and base32 is tried first.

The internal, in-session data-member-id DOM attribute (render → click interceptor handoff) keeps using lossless hex — it never persists and isn't the wire token, so it stays full-precision and zero-risk.

Rollout note (forward compat)

During rollout, an old client that sees a new client's base32 token can't parse it and falls back to markdown rendering of the raw token — i.e. it shows @name as a plain link instead of a styled chip. The member name still shows; this is the same graceful degradation already documented for "a client too old to parse the token", and it self-heals as clients update. Because the River UI is served from the web-container contract and riverctl updates via cargo install, the fleet converges after republish.

Testing

  • common/src/mention.rs: round-trip now asserts the base32 form; new tests for legacy-hex parsing, base32/hex disambiguation (incl. the all-digit-base32 case), and MemberRef::resolve. 16 unit tests pass.
  • ui/src/components/conversation.rs: new tests that a current token's chip recovers the full id from the short ref, a legacy chip stays clickable even for an unknown member, and an unknown short ref renders an inert chip with no data-member-id. All 17 UI mention tests pass.
  • riverctl: existing 13 mention CLI tests pass unchanged (they compare against encode_mention(...), not hard-coded hex).
  • cargo fmt clean; cargo clippy introduces no new warnings; river-ui wasm + riverctl + river-core all compile.

Not a contract/delegate change

mentions is a client-only common/ feature (not enabled by the room-contract or chat-delegate crates), so this touches no contract/delegate WASM — no migration entry needed, and git status confirms no .wasm changes.

[AI-assisted - Claude]

🤖 Generated with Claude Code

sanity and others added 2 commits June 29, 2026 09:06
Mention wire tokens now reference a member by their 8-char truncated-base32
Display label (e.g. `rv:FPVN6PUN`) instead of the 16-char hex id
(`rv:422a2a8d3edfea2b`). FPVN6PUN is the same short id River shows everywhere
else to name a member, so mentions now read consistently with the rest of the
app.

The base32 label is lossy (40 of 64 bits), so a parsed mention carries a
`MemberRef` that callers resolve against the room's known members (the same
assumption the CLI/UI already make when naming members by short id) rather
than a decoded `MemberId`.

Backward compatible: the legacy `rv:<hex>` form is still parsed for messages
already in circulation, via `MemberRef::Legacy`. Both are flagged for eventual
removal (TODO(mentions) on the module and the variant). The two forms can't
collide — current is exactly 8 base32 chars, legacy up to 16 hex, base32 tried
first.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xv4yWFSyF6Kv85zm99zaSv
Address multi-model review findings (all MINOR, no blockers):

- MemberRef::resolve now tie-breaks on the lowest id instead of returning the
  first HashMap match, so a ~2^-40 truncated-label collision resolves to one
  well-defined member regardless of iteration order (was nondeterministic for
  the chip's data-member-id / self-highlight). Same systemic limit the app
  already accepts when naming members by short id.
- Fix REF_SCHEME rustdoc example (was still hex).
- Add coverage: deterministic collision resolution, a single body mixing a
  legacy hex token and a current base32 token, legacy self-mention detection
  via contains_mention_of, a multibyte snapshot name around a base32 ref, a
  legacy self-mention highlight in the UI chip, and an explicit CLI assertion
  that the send path emits base32 and never hex.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xv4yWFSyF6Kv85zm99zaSv
@sanity

sanity commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

Multi-model review — consolidated

Full-tier review (this touches a persisted serialization with back/forward-compat implications). Reviewers: external Codex (gpt-5.5) + four independent Claude adversarial lenses — serialization/disambiguation, code-first correctness, testing coverage, big-picture consistency.

Verdict: no BLOCKER or MAJOR findings. Codex: "No discrete correctness issues were found in the diff." All four Claude lenses confirmed the core invariant holds — the encoder produces 16-char lowercase-hex (legacy) vs 8-char uppercase-base32 [A-Z2-7] (current), the two alphabets are disjoint, and member_ref_from_str tries base32 first, so the forms cannot cross-parse.

Findings addressed (all MINOR), fixed in f6bfbe39

# Finding Lens(es) Fix
1 MemberRef::resolve returned the first HashMap match → on a ~2⁻⁴⁰ truncated-label collision the resolved member (chip data-member-id, self-highlight, self-notification) was nondeterministic skeptical, code-first, testing Tie-break on lowest id (filter().min()), so collision resolution is deterministic + reproducible. Pinned by a new test.
2 REF_SCHEME rustdoc still showed a hex example big-picture Updated to rv:FPVN6PUN.
3 Coverage gaps testing, code-first Added: deterministic-collision resolve, a body mixing legacy-hex + current-base32 tokens, legacy self-mention detection via contains_mention_of, multibyte snapshot name around a base32 ref, legacy self-mention highlight in the UI chip, and an explicit CLI "emits base32, never hex" assertion.

Test counts: river-core 16→20, river-ui 17→18, riverctl 13→14. cargo fmt/clippy clean; no WASM/migration files touched.

Acknowledged, not changed (with rationale)

  • Wrong-member-on-collision degradation. On the ~2⁻⁴⁰ event, the now-deterministic resolution still binds to one of the two colliding members. This is the inherent lossy-label tradeoff and the same limit the app already accepts for ban-by-short-id / DM-by-short-id; not a regression.
  • O(n²) parser worst-case on crafted @[@[@[… input — pre-existing scanning structure (unchanged by this PR), bounded by max_message_size (1000-byte default), not exploitable at default limits.
  • Pre-existing markdown-autolink/sentinel edge in message_to_html_with_mentions (a bare URL immediately abutting a mention token) — orthogonal to the hex→base32 change, untouched here; filing a separate investigation issue.

[AI-assisted - Claude]

@sanity sanity merged commit b663900 into main Jun 29, 2026
7 checks passed
sanity added a commit that referenced this pull request Jun 29, 2026
…base32 mentions)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xv4yWFSyF6Kv85zm99zaSv
sanity added a commit that referenced this pull request Jun 29, 2026
Ships the base32 mention-token change (#373) to crates.io consumers.
river-core gains the MemberRef API; riverctl depends on >=0.1.12. No WASM
change (version is not embedded in delegate/contract WASM), so contract and
delegate keys are unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xv4yWFSyF6Kv85zm99zaSv
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant