fix(unified-messages): show reply box with "Unknown message" when parent is missing (#4245)

[Yeraze]

Closes #4245

Problem

The reply preview in Unified Messages was gated on parent being truthy (UnifiedMessagesPage.tsx:605). parent comes from byPacketId.get(msg.replyId), so when the replied-to packet was never received by any source — or has since been purged — the entire block was skipped and the message rendered identically to one that was never a reply. The non-null replyId was dropped on the floor.

Fix

Resolution is now a three-state pure helper, resolveReplyPreview, exported so the distinction is testable without standing up a full page render:

state	condition	renders
none	replyId == null	no reply box (unchanged)
resolved	parent found	quoted sender + text (unchanged)
unknown	replyId set, parent absent	reply box with "Unknown message"

The placeholder is italic and slightly dimmer (--unknown modifier) so it reads as absent rather than as a real message whose body happens to be the words "Unknown message".

Note on replyId != null

The check is deliberately != null rather than truthiness — packet id 0 is falsy but is still a valid reply target, and a truthiness test would silently reclassify it as a non-reply. There's a test pinning this specifically, since it's the obvious way for this to regress.

Validation

• New UnifiedMessagesPage.replyPreview.test.ts — 5 tests, success: true
• npx tsc --noEmit — no errors
• npx eslint on both changed source files — clean

tsc caught one real mistake during development: after switching to the helper, a bare parent reference in the JSX silently resolved to the global Window instead of erroring as undefined. Worth knowing that identifier is booby-trapped in this file.

There was no existing test file for this page; this adds the first one.

🤖 Generated with Claude Code

https://claude.ai/code/session_016bxVewVnissUKiGZmhf9FW

[github-actions]

Claude finished @Yeraze's task —— View job

---

PR Review: fix(unified-messages): show reply box with "Unknown message" when parent is missing

Tasks:

• Gather context and read changed files
• Review UnifiedMessagesPage.tsx changes
• Review test file
• Review CSS and localization
• Post final review

---

This is a clean, well-reasoned fix. The three-state discriminated union approach is exactly the right model for this problem. Notes below range from strong positives to a few minor observations.

Code Quality

resolveReplyPreview helper (src/pages/UnifiedMessagesPage.tsx:87–94) — extracting this into a pure, exported function is the right call. Discriminated unions with kind strings are idiomatic TypeScript and make exhaustive handling at call sites straightforward. The Pick<Map<number, UnifiedMessage>, 'get'> signature is a nice touch: it minimally constrains the input, making the helper trivially testable without constructing a full Map.

!= null guard (UnifiedMessagesPage.tsx:91) — the explicit == null check instead of truthiness is correct and the test at line 43–49 in the test file pins the regression properly. The PR description's note about packet id 0 being falsy is exactly the kind of reasoning that should be visible here.

JSX rendering (UnifiedMessagesPage.tsx:628–650) — the switch from parent && (...) to replyPreview.kind !== 'none' with an inner conditional is clean. The exhaustive check is implicit (the else branch handles unknown), which is fine given only three states. If a fourth state were ever added the TypeScript compiler wouldn't catch a missing branch here, but that's a minor theoretical concern for this scope.

tsc catch of Window aliasing — the PR description's note about parent resolving to the global Window is a genuine gotcha worth knowing. The fix sidesteps it correctly by never binding parent in the JSX render scope.

Test Coverage

The test file covers all five meaningful cases:

• null → none ✓
• undefined → none ✓
• id found → resolved with parent ✓
• id missing → unknown ✓
• empty map → unknown (not none) ✓
• id 0 as a valid reply target ✓

The use of as never for the stub message (UnifiedMessagesPage.replyPreview.test.ts:19) is unconventional but pragmatically correct — the helper only reads the object back from the map, so the type casting is purely to satisfy the map's type parameter. A Partial<UnifiedMessage> cast would be a bit safer against future refactoring that changes what resolveReplyPreview reads, but this is very low risk for a pure lookup helper.

One gap: there are no tests for the JSX rendering of the unknown state (italic/dimmed styling, absence of a sender label). Given that resolveReplyPreview is already well-tested, this is a reasonable trade-off for a first test file on this page.

CSS

The modifier class approach (unified-reply-preview__text--unknown, unified.css:365–368) follows BEM conventions used throughout the file. The styling choice — italic + reduced opacity (0.6 vs 0.8) — effectively communicates "absent" vs "real content." The opacity override on the --unknown class overrides the base unified-reply-preview__text opacity (0.8) correctly since both rules target the same element and specificity is equal (last rule wins).

Localization

The en.json key unified.messages.unknown_parent is correctly added. The inline fallback 'Unknown message' in the JSX call (UnifiedMessagesPage.tsx:646) is a sensible safety net. No issues.

Minor Observations

1. Fallback in t() call — t('unified.messages.unknown_parent', 'Unknown message') passes the fallback as a string. In i18next this is valid, but the second argument is typically the options object. The string form works here because i18next accepts a string as a shorthand default, but for consistency with how other t() calls in this file look, it's worth noting (not blocking).

2. replyPreview computed inside feedMessages.map (UnifiedMessagesPage.tsx:577) — this runs byPacketId.get(msg.replyId) on every render. This is fine: byPacketId is already memoized, and map lookups are O(1). No concern.

3. Arrow character ↳ (UnifiedMessagesPage.tsx:630) — the hardcoded arrow is consistent with the existing unified-reply-preview__arrow element that was already in the file before this PR. Not introduced here.

Summary

No bugs found. The fix is correct, the abstraction is clean, and the tests are well-targeted. The == null vs truthiness rationale is solid and the regression test for packet id 0 is exactly the right safeguard. Approved.

[github-actions]

Claude finished @Yeraze's task —— View job

---

PR Review: fix(unified-messages): show reply box with "Unknown message" when parent is missing

Tasks:

• Gather context and read changed files
• Review replyPreview.ts utility
• Review test file
• Review UnifiedMessagesPage.tsx changes
• Review CSS and localization
• Post final review

---

This is a clean, well-structured fix. The latest synchronize commit refactored resolveReplyPreview out of the page module into src/utils/replyPreview.ts — an important improvement over the initial implementation — and the PR is now in good shape overall.

Code Quality

resolveReplyPreview in src/utils/replyPreview.ts — moving this helper out of the page module was the right call. The PR description explains why well (Fast Refresh restriction on co-exporting components and non-components from the same module), and the file-level comment captures that reasoning for future readers. The generic <TMessage> parameter is a nice touch: it keeps the utility decoupled from UnifiedMessage's shape and makes it trivially testable without constructing a full page message object.

ReplyPreviewState discriminated union (replyPreview.ts:13–16) — the three-state union is idiomatic TypeScript. TypeScript's narrowing on kind means the JSX rendering benefits from exhaustiveness checking at the call site. The resolved arm carries parent: TMessage rather than re-looking it up, which is the correct pattern.

!= null guard (replyPreview.ts:35) — explicit null check rather than truthiness is correct. The replyId == null form covers both null and undefined in a single expression, and the inline comment explaining the packet-id-0 concern is exactly the kind of defensive documentation this should have.

JSX rendering (UnifiedMessagesPage.tsx:606–628) — the switch from parent && (...) to replyPreview.kind !== 'none' is clean. The else branch implicitly handles unknown, which is fine for three states. If a fourth kind were ever added, TypeScript wouldn't warn about the missing branch at this JSX site — but that's a theoretical concern at the current scope.

parent / Window collision avoided — by never binding a parent variable in the page's render scope, the PR sidesteps a real gotcha: in browser contexts parent refers to window.parent and is always truthy, so the old parent && (...) guard would have silently misbehaved in subtle ways before the fix if any refactoring had introduced that name. Worth knowing.

Test Coverage

The tests in src/utils/replyPreview.test.ts cover all meaningful states:

Case	Covered
null → none	✓
undefined → none	✓
id found → resolved with parent object	✓
id set, parent missing → unknown	✓
empty map + set id → unknown (not none)	✓
id 0 as valid reply target (falsy guard)	✓

The stub message object at line 14 uses a plain object literal rather than as never or Partial<UnifiedMessage> — this works cleanly for a generic helper. Since resolveReplyPreview never reads any fields of TMessage directly (only stores and returns it), this is safe and idiomatic.

One gap: there are no tests for the JSX rendering of the unknown state — the italic/dimmed styling, the absent sender label, the presence of the reply box wrapper. Given that the helper logic is already well-covered, this is a reasonable trade-off for a first test file on this page.

CSS

unified-reply-preview__text--unknown (unified.css:365–368) follows BEM conventions used throughout the stylesheet. opacity: 0.6 overrides the base unified-reply-preview__text opacity (0.8) at equal specificity, so last-rule-wins applies correctly. The italic + reduced opacity combination reads clearly as "absent" vs real quoted content.

Localization

unified.messages.unknown_parent is correctly added to public/locales/en.json. The inline fallback 'Unknown message' in t('unified.messages.unknown_parent', 'Unknown message') (UnifiedMessagesPage.tsx:624) is a sensible safety net. Note: in i18next the second argument to t() can be either an options object or a default string — the string form is supported and works here, though it's worth being aware that other call sites in the file may pass options objects differently.

Minor Observations

1. No sender label in the unknown state — The resolved arm shows shortSenderLabel(replyPreview.parent) + message text. The unknown arm shows only the "Unknown message" placeholder with no sender. This is correct behavior given no parent is available, and visually the ↳ arrow still signals "this is a reply."

2. replyPreview computed inline inside feedMessages.map (UnifiedMessagesPage.tsx:555) — byPacketId.get(msg.replyId) is O(1) on every render iteration. Since byPacketId is already memoized upstream, there's no performance concern here.

Summary

No bugs found. The refactor to src/utils/replyPreview.ts is an improvement over the initial implementation. The fix is correct, the abstraction is clean, the != null guard is well-reasoned and pinned by a regression test, and the three-state discriminated union is the right model for this problem. Approved.