Claude finished @Yeraze's task —— View job

PR Review: fix(analysis-map): render cross-source neighbor links (union, not intersection)

• Gather context and understand the request
• Read changed files
• Review code quality and correctness
• Review test coverage
• Post final review

Summary

The fix is correct, well-scoped, and the root cause diagnosis in the PR description is accurate. The implementation uses a sensible fallback pattern and the tests cover the critical scenarios.

Code Review

NeighborLinksLayer.tsx

Correctness ✅

The fallback pattern (positionByKey.get(aKey) ?? positionByNode.get(Number(e.nodeNum))) correctly implements "prefer source-scoped, fall back to any source." The fix directly addresses the bug: previously both endpoints were looked up strictly under edge.sourceId, so any edge where the neighbor node's position was recorded under a different source was silently dropped.

Minor observations:

1. positionByNode / transportByNode use first-wins ordering (if (!map.has(...)) guard at lines 86, 94). This means the first occurrence in the nodes array wins the fallback. If nodes are sorted by source in a consistent order, this is fine — but it may produce visually inconsistent transport coloring when the same nodeNum appears under multiple sources with different transport classes. Low severity for a fallback, but worth noting.

2. eslint-disable-next-line comment at line 144 — the inWindow function is defined inside the component but not in the useMemo deps array. This is the existing pattern (the ts.* values are already in the deps), so the suppression is justified. However, it's worth noting inWindow could be moved into the useMemo body or memoized separately to eliminate the disable comment entirely. This is a pre-existing issue, not introduced by this PR.

3. Transport class resolution logic at line 130 is functional but slightly hard to read:

   const tc: NodeTransportClass = aTx === 'mqtt' || bTx === 'mqtt' ? 'mqtt' : aTx === 'udp' || bTx === 'udp' ? 'udp' : 'rf';

   This is pre-existing code, just moved — no action needed.

4. Four useMemo calls vs. two — the fix introduces positionByNode and transportByNode as separate memos. These iterate nodes a second time. A single memo pass building all four maps would be more efficient, but for the typical node counts seen in Meshtastic deployments this is negligible. Not a blocking concern.

NeighborLinksLayer.test.tsx

Test coverage ✅

The two new test cases directly cover the fix:

• renders a cross-source edge when the neighbor is positioned under a different source (#3792) — the regression test that would have caught the bug.
• still drops an edge when an endpoint has no position on any source — verifies the "no position anywhere" edge case still filters correctly.

The beforeEach reset pattern cleanly isolates test state, and the mutable mockState approach keeps setup readable.

One gap to consider: There's no test asserting that when a node appears under both sources, the source-scoped position takes precedence over the cross-source fallback. This would confirm the "prefer own source" behavior. It's not a blocker — the implementation is clearly correct — but it would harden the intent.

Verdict

Approve. The fix is minimal, targeted, and correct. The root cause is clearly explained, the fallback is sound, and the regression test will catch this class of bug going forward. The minor observations above are all low-severity or pre-existing and don't warrant blocking the merge.