Repository navigation
Port upstream #2099 hop ambiguity UI with observer/cache correctness fixes #165
Description
Activity
Review notes (verified against upstream head
3532cedeand our master727efca0)The three defects are confirmed in the upstream code (read-only analysis):
observerPosition()turnslat: null/lon: nullinto[0, 0], becauseNumber(null)is0and therefore finite. Upstream/api/observersreturns these coordinates asinterface{}, so they come out as JSONnullwhen no node location is known.cacheResolvedPaths()only writes the bare hop keys.resolveHops()writeshop:observer, andrenderHop()prefers that key, so the client's heuristic overrides the server'sresolved_path.- The incremental path (
packets.js~1424–1434) only checks whether the bare key exists.
Upstream PR run
36783862462concludedfailureon the same head. I have not checked which individual tests failed.Our fork today:
renderHop()(public/packets.js~1028) already looks uphop:observer, butresolveHops(hops)never writes that key. None of the three defects exists here yet. A port would introduce them unless it fixes them too.Additions for the port
A. Test paths differ in this fork.
- The
#1153full-chain check lives intest-issue-1146-path-link-contrast-e2e.jsin the repo root, not undertests/e2e/. - The new unit test should follow our convention:
test-*.jsin the repo root, registered intest-all.shand in the relevantdeploy.ymltest step. The fork guards must stay unchanged (9 ×github.repository == 'Kpa-clawbot/CoreScope').
B. Bound the per-observer cache.
hopNameCachekeyed byhop:observergrows with prefixes × observers (~42 observers here).- It is reset in only one place today (
packets.js~1401). - AGENTS.md requires a size limit or eviction for every map. Add a cap, or a reset tied to the existing reset points, and test it.
C. Perf proof for the packets page.
- The initial load now resolves hops once per observer group instead of once overall. This is probably cheap, since there are at most ~42 groups and the work is local, but this is a hot path.
- Include before/after timing of
resolveHopsForPackets(or of the full initial load) on ~30K packets, as AGENTS.md rule 0 requires.
D. A possible shape for defect 2.
- When a packet has
observer_id,cacheResolvedPaths()can also writehop:observer. - Alternatively,
resolveHops()can skip hops that already have a server-resolved entry for that packet. - Either way, a test must show the server's candidate wins when the client heuristic would pick another.
Priority: low. This is a new feature, a UI for ambiguous hops, not a fix for a defect in our fork.
Relates to upstream
Kpa-clawbot/CoreScope#2099(referenced in code format on purpose, to avoid cross-references on the upstream PR).Deferred upstream assessment — 2026-10-04
Reusing this issue for the weekly upstream backlog rather than opening a duplicate. Upstream PR 2099 is now merged.
What it adds: observer-aware hop resolution and visible ambiguity information, so a short prefix does not look like a certain node identity merely because the UI chose one candidate. Useful for interpreting routes, but not proof of an individual sender, physical location or radio link.
What we already have: this issue's correctness requirements and open #185 provide the adaptation candidate. Backend observer-based last-hop resolution in #190 is now merged; it is complementary, not a substitute for correct UI presentation.
Remaining decision: #185 was explicitly parked pending #190. The prerequisite has landed, but that does not automatically unpark or approve it. Its review still records the grouped-row problem (missing
resolved_pathcauses misleading client warnings), cache/test gaps, popup count issues and increased client CPU cost. Prefer using persisted server evidence, with heuristics only where necessary, if this is resumed.Later checklist: recheck the grouped API shape and server precedence; reproduce review findings on the latest branch; measure JSON size and real-browser resolution/rendering cost; cover null coordinates, different observers, cache pressure and grouped/flat navigation; coordinate packet layout/popup work from upstream Kpa-clawbot#2090.
Decision remains deferred: improve the existing PR, adopt only the useful UI subset, or retain current behavior. No new port or implementation is started by this update.
- added 7 commits that reference this issue
on Oct 7, 2026 - added a commit that references this issue
on Oct 7, 2026
Track the proposed port of upstream
Kpa-clawbot/CoreScope#2099to this fork. Review was performed on upstream head3532cede0e3ad242dee8d348f9ee80c1a78b8d9f. These are findings in the upstream PR, not claims that the same code or defects already exist in our fork. Compare against our currentmasterbefore implementing; carry over the useful ambiguity UI only with the cases below addressed and tested against our code.1. Missing observer coordinates become a real
(0, 0)anchorobserverPosition()acceptsNumber(null)as finite, so an observer returned withlat: null, lon: nullproduces[0, 0], not[null, null]. The default/api/observersresponse uses nullable coordinates when no node location is known (response construction).HopResolver.resolve()then treats(0, 0)as a geographic anchor and can select the candidate closest to that false location. The PR description says only 27 of 42 observers report a location, so this is a normal input, not just a malformed response.Reproduction: with
observerMapcontaining{id: 'obs', lat: null, lon: null},observerPosition('obs')returns[0, 0]. A test should verify that this returns[null, null]and does not influence an ambiguous hop's candidate selection.2. Client resolution can replace a server-resolved path
The initial packet load calls
cacheResolvedPaths(packets)followed byresolveHopsForPackets(packets). The first writes serverresolved_pathentries under bare hop keys; the second looks for observer-specific keys, finds them absent, recomputes the same hops on the client, and writes those keys.renderHop()prefers the observer-specific entry. This can replace the server-confirmed pubkey with a different heuristic candidate, contrary tocacheResolvedPaths()'s documented precedence. The same sequence occurs when expanding grouped packets.Reproduction: provide a packet whose
resolved_pathnames candidate A and whose ambiguous prefix makesHopResolver.resolve()select candidate B for its observer. The rendered hop should keep A; the current cache ordering renders B.3. Incremental packets can skip observer-specific resolution
The incremental update tests only whether a bare hop key is cached before deciding to call
resolveHopsForPackets()(packets.jslines 1424–1434). After observer A has populated that bare key, a packet from observer B with the same prefix yieldsnewHops.size === 0, even though B's observer-specific key is absent. Rendering then falls back to A's cached result. This defeats the per-observer cache introduced by this PR on the live update path.Reproduction: process the same ambiguous prefix first for observer A, then for B via the incremental packet path. Verify that B receives its own resolved entry and does not inherit A's name.
Upstream CI gate
PR run 36783862462 failed in Playwright on the same head: four
#1153full-chain checks report “Expected all 18 hops in the rendered chain.” The new.hop-groupwrapper changes the child structure, while the existing test calculateschain.children.length - chain.querySelectorAll('button').length(test line 202). Count the actual hop links and verify visually that the full chain remains present. The new unit test passes 30/30 locally, but it does not cover the cases above.Acceptance for a fork port
resolved_pathprecedence while showing uncertainty when it is genuinely unresolved.(0, 0).