Claude finished @Yeraze's task —— View job

PR Review

• Read changed files
• Review Fix 1: node name preservation logic
• Review Fix 2: CSS color changes
• Review test coverage
• Post final feedback

Overall Assessment

Both fixes are correct, well-scoped, and safe to merge. No issues found that would block approval.

Fix 1: Node name preservation (meshcoreManager.ts)

The change (meshcoreManager.ts:1112):

// Before
advName: data.adv_name ?? existing.advName,

// After
advName: data.adv_name || existing.advName,

And in persistContact (meshcoreManager.ts:1473):

// Before
name: contact.advName ?? contact.name ?? null,

// After  
name: contact.advName || contact.name || null,

Verdict: Correct fix. The root cause analysis in the PR description is accurate — ?? only guards against null/undefined, not empty strings. Switching to || properly treats "" as falsy and falls back to the existing stored value.

One minor observation: The logger line at meshcoreManager.ts:1139 still logs data.adv_name ?? '', which could log an empty string and mask the fact that the empty name was discarded. This is cosmetic and doesn't affect correctness, but using updated.advName ?? '' instead would log the actual resolved name and make debug tracing clearer.

Fix 2: CSS color change (MeshCorePage.css:725-726)

/* Before */
.mc-message-route { color: var(--ctp-overlay0); ... }
.mc-message-scope { color: var(--ctp-overlay0); ... }

/* After */
.mc-message-route { color: var(--ctp-surface2); ... }
.mc-message-scope { color: var(--ctp-surface2); ... }

Verdict: Correct. --ctp-surface2 is indeed more muted than --ctp-overlay0 in both dark and light Catppuccin themes — this is a valid visual hierarchy improvement. The adjacent comment explains the intent clearly.

Test coverage (meshcoreManager.contactPersistence.test.ts)

The new regression test at line 268 is well-structured:

• Establishes an initial named contact
• Sends a second advert with adv_name: ''
• Asserts both the in-memory advName and the upsertNode DB call preserve the original name

Minor style note: The test uses await Promise.resolve(); await Promise.resolve(); (double-flush) to settle microtasks, consistent with the rest of the test file. This is acceptable but slightly fragile compared to a proper flush() helper (which is already defined in this file for the notification tests). Consider using await flush() for consistency in future tests — not a blocker for this PR.

Summary

Superseded. The #3769 metadata-contrast half of this PR already shipped in #3778 (time/route/scope set to --ctp-overlay1), so this PR now conflicts on MeshCorePage.css. PRs #3771 and #3772 are also duplicates of each other. The still-relevant part — the #3756 node-name-on-empty-advert fix (??→||) — is being re-submitted as a fresh minimal PR rebased on current main with no CSS changes. Closing in favor of that.