Claude finished @Yeraze's task —— View job

PR Review: fix(meshcore): honor packet-monitor Max count setting in the live list

Tasks:

• Gather context and understand the PR
• Review meshcoreRoutes.ts changes
• Review MeshCorePacketMonitorView.tsx changes
• Review meshcoreRoutes.test.ts changes
• Post final review feedback

Summary

Clean, targeted fix that resolves the inconsistency between the list and export endpoints. The approach is sound and the test coverage is appropriate. A few observations below.

Code Quality

meshcoreRoutes.ts (lines 3281–3284)

The logic is correct, but there's a subtle edge case worth noting: if a client passes ?limit=900 when getMaxCount() returns 500, the explicit client limit (900) wins and the user would see more rows than their configured max. The comment says "An explicit, smaller client-supplied limit still wins" — but the implementation doesn't enforce smaller; any positive finite value wins. This means:

const effectiveLimit = Number.isFinite(requestedLimit) && requestedLimit > 0 ? requestedLimit : maxCount;

...could return up to MESHCORE_PACKET_MAX_LIMIT (1000) even when the user set maxCount to 100. Since the frontend now doesn't send an explicit limit, this isn't an issue in practice, but it's a behavior gap between the comment and the code. Suggest either:

• Updating the comment to reflect the actual behavior, or
• Using Math.min(requestedLimit, maxCount) as the effective limit when a client limit is supplied.

meshcoreRoutes.ts — getMaxCount() deduplication

The PR description mentions deduplication of the maxCount fetch. Confirmed: the new code reads maxCount once at the top and uses it in both the query and the response envelope (line 3307). No duplicate await needed.

MeshCorePacketMonitorView.tsx

The load callback (lines 87–108) is clean. The URLSearchParams approach is correct. No concerns.

The removal of PAGE_LIMIT is the right call — it was previously causing the server setting to be silently ignored.

Test Coverage

The three new tests in meshcoreRoutes.test.ts (lines 2104–2140) cover:

1. Server-configured max as default limit ✓
2. Ceiling clamp (5000 → 1000) ✓
3. Explicit smaller client limit honored ✓

Missing test case: There's no test covering an explicit client-supplied limit larger than maxCount (e.g., ?limit=900 with getMaxCount() = 500). Based on the current code, this resolves to 900 (clamped to 1000 ceiling), not 500. This behavior may or may not be intentional — a test would clarify the expected contract.

Minor Observations

• The since query parameter input handling (req.query.since) uses the same seconds/milliseconds heuristic as the Meshtastic route (lines 3287–3288). This is fine and consistent.
• No security concerns identified — the existing requirePermission middleware is in place.
• No performance concerns — getMaxCount() reads a single settings row (cached or fast DB lookup).

Verdict

Approve with suggestions. The core fix is correct and the tests cover the primary behavior. The edge case around client limits larger than maxCount is a minor documentation/behavior gap, not a regression.