Skip to content

follow-ups from #203, #204 and #206 reviews (P3: test gaps, inactive-card wording, history defaults, Hash Stats audit) #208

Description

@dborup

Summary

Low-priority (P3) follow-ups from the reviews of three merged PRs:

None of them blocked the merges.

Relates to #199, #180, #205.

From #203 (observer node retention)

  1. The lookup error is silent. When lookupMissingNode fails in cmd/server/node_not_found.go (writeNodeNotFound), the handler falls back to the bare 404 and drops the error. A broken lookup, for example schema drift on inactive_nodes, then looks like an ordinary not-found. Log it once, with a rate limit.
  2. Test gap. Mutant M8, which drops the Last advert row from missingNodeView, survives the unit test and the E2E, because the same date also appears in the explanation sentence. Assert the row itself.
  3. The card wording can contradict reality. An observer can be uploading right now while its node row is still in inactive_nodes: it was retired before fix(nodes): keep active observers as nodes and explain inactive ones (#199) #203, or while the observer was quiet. The card then says "this device is inactive". Reword it, for example "No advert heard since ; this node is listed as inactive", or show the observer's current status next to it.

From #204 (packets URL and modal)

  1. Test gap. No test covers Escape with focus inside the View Path modal. Mutant M4, which removes overlay.contains(el) in public/packet-path-map.js (around line 100), survives every unit and E2E test, but a probe shows that it breaks real behaviour. Add a test that focuses an element inside the modal and asserts that Escape closes it.

From #206 (analytics deep links)

  1. Back/forward to a history entry in a default state shows the session value, not that entry's view.

    • restoreViewParams (public/analytics.js around line 395) leaves defaults out of the URL, and a missing key falls back to sessionStorage.
    • So navigating back to an entry whose view was the default can restore the last session value instead.
    • Either treat "key absent" as "default" during history navigation, or write the default explicitly on history entries.
  2. The audit misses two Hash Stats items:

    • the multi-byte adopters filter (All / Confirmed / Suspected / Unknown, data-mb-filter, around line 1630);
    • its column sort (around line 1657).

    Both are local-only. Deep-link them, or record why they stay local.

  3. Pre-existing: the Hash Issues keys bytes and section are not cleared on a tab switch, because they are missing from TAB_URL_PARAMS (around line 354), although the comment says the map holds "the hash keys each tab owns".

Acceptance criteria

  • Each item is fixed with a test that fails before and passes after, or it is closed with a recorded reason.
  • No behaviour change beyond the items listed.

Activity

  1. added a commit that references this issue on Oct 4, 2026
  2. dborup commented on Oct 4, 2026

    @dborup
    OwnerAuthor

    Fixed by #214 (merged as 376d51c8).

    All 7 items are handled:

    Leftovers from the #214 review:

  3. dborup commented on Oct 4, 2026

    @dborup
    OwnerAuthor

    Correction: the TestIssue1008_HandlerReturns503WhileSubpathIndexLoading flake is tracked in #227, not #226. #226 is the Hash Stats sort.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions