Claude finished @Yeraze's task —— View job

PR Review: fix(neighbors): align live map "Show Neighbor Info" with Map Analysis Neighbors view

• Gathering context and reading changed files
• Reviewing code quality and correctness
• Checking test coverage
• Posting final review feedback

Summary

The fix is well-motivated and correctly implemented. Both endpoints now filter neighbor edges using the NeighborInfo record's timestamp (packet-receipt time in ms) instead of the endpoint nodes' lastHeard, which was the source of the divergence between the live map and the Map Analysis view.

Code Quality

sourceRoutes.ts — GET /:id/neighbor-info (lines 1195–1211)

The new filter is clean and correct. One minor ordering note: the .filter(ni => reportSec >= cutoffTime) runs after the .map(ni => {...}) that adds the node/neighbor objects, and then .map(({ node, neighbor, ...rest }) => rest) strips them again. A small potential optimization would be to apply the timestamp filter before the enrichment map (since you already have access to ni.timestamp on the raw record), but this doesn't affect correctness and the current approach avoids restructuring the logical pipeline — acceptable here.

neighborInfoRoutes.ts — legacy GET / (lines 42–52)

The same logic is applied consistently. Note that this file calls databaseService.getLatestNeighborInfoPerNodeScoped (line 12) — a non-Async-suffixed method — which per CLAUDE.md conventions suggests it may be synchronous or a deprecated shim. If this is the legacy route and will be removed or replaced by the source-scoped equivalent, it may not be worth polishing further.

timestamp unit assumption

Both endpoints assume timestamp is stored in milliseconds (they do Math.floor((ni.timestamp ?? 0) / 1000) to get seconds). The PR description confirms this. The test file's makeNeighborRecord also uses Date.now() (ms). This is consistent throughout and is also how meshtasticManager writes the value. No issue, but worth keeping documented for future contributors.

Null-safety on timestamp

(ni.timestamp ?? 0) safely handles nulls by treating them as epoch 0, which will always fail the freshness check — correct behavior (null timestamp = no known report time = don't show the edge).

Potential Issue: filter order causes unnecessary enrichment

In sourceRoutes.ts (lines 1160–1212), the channel-permission filter runs first (good), but the timestamp filter runs after the per-record enrichment .map(ni => {...}) that does the node lookups, position resolution, and transport classification. Since allNodeNums and nodeMap are already batch-fetched before the pipeline (line 1142), the node lookup itself is O(1) from the Map — this is fine in practice. No real performance concern.

Test Coverage

The updated tests in sourceRoutes.neighbor-info.test.ts are thorough and lock in the exact semantics of the fix:

• ✅ Stale report is dropped even when endpoint nodes are still active (line 185–203) — this is the core regression case
• ✅ Fresh report is kept even when lastHeard is stale (line 205–225) — locks in alignment direction
• ✅ Fresh report is kept when reporter's lastHeard is null (line 227–242) — covers [BUG] Meshtastic NeigborInfo: Response received but not shown in UI #3025
• ✅ Fresh report is kept when neighbor's lastHeard is null (line 244–262) — covers fix(virtual-node): stop creating zombie nodes on map (#2602) #2615
• ✅ Stale report is dropped even when firmware lastRxTime is fresh (line 286–308) — documents clock-skew avoidance
• ✅ Custom maxNodeAge setting respected (line 331–348)
• ✅ Channel permission gate retained ([BUG] MeshCore telemetry retrieval fails on every repeater (meshcore-only setup, v4.6.1) #3092) (line 370–397)

The server.neighbor-info-position.test.ts update (lines 149–156) correctly mirrors the live-route filter inline. This test file is a standalone mock-app test — the filter copy is necessary to keep it honest against the real route behavior.

Minor Observations

1. neighborInfoRoutes.ts line 12: getLatestNeighborInfoPerNodeScoped is called without await. This is consistent with the pre-existing code, but if this method was ever made async it would silently break. Since this file appears to be the legacy endpoint (not per-source), this is an existing pre-PR issue, not introduced here.

2. maxNodeAgeHours settings key differs between routes: sourceRoutes.ts reads 'maxNodeAgeHours' (line 1130), while neighborInfoRoutes.ts reads 'maxNodeAge' (line 14). This discrepancy existed before this PR — both default to 24h when the setting is absent so behavior matches, but it's worth a follow-up to normalize the key name.

3. No change to the GET /api/analysis/neighbors analysis endpoint — correct; the PR says both views now use the same filtering field. No need to touch the analysis endpoint.

Verdict

Approve. The root cause is correctly identified and the fix is minimal, consistent, well-commented, and well-tested. No bugs or security issues introduced. The pre-existing key-name discrepancy (maxNodeAgeHours vs maxNodeAge) is out of scope for this fix but worth tracking.

Thanks for the review. All three observations are non-blocking and I'm leaving the code as-is:

1. Filter order (timestamp after enrichment): Agreed it's a no-op for perf — nodeMap is batch-fetched once and lookups are O(1), so reordering would only churn the pipeline. Left as-is.
2. getLatestNeighborInfoPerNodeScoped non-await / legacy route: Pre-existing, and that route (/api/neighbor-info) isn't called by the current frontend. Out of scope here.
3. Settings-key discrepancy (maxNodeAgeHours vs legacy maxNodeAge): Real but pre-existing and only affects the unused legacy route (both default to 24h). Flagging as a follow-up rather than widening this PR's scope.

Ready to merge from my side once you're happy.