Repository navigation
port(upstream#1968): clearly mark the selected node in path chains - #46
Merged
dborup merged 3 commits intoSep 20, 2026
Merged
Conversation
…t#1968) Red commit: `0988bc0` (local browser: 3 passed, 8 behavior assertion failures before the fix). The selected node now has a compact outline in long “Paths Through This Node” chains, in the side panel and full detail page. Matching uses complete public keys without case sensitivity; same-prefix siblings and unresolved hops stay unmarked. Existing links, escaped names, warnings and ambiguity underlines are preserved. Fixes Kpa-clawbot#1153. Its prerequisite Kpa-clawbot#1144 is already merged. - E2E assertion added: `test-issue-1146-path-link-contrast-e2e.js:220`. The existing CI-selected harness passes 11 checks across 18-hop paths, both themes, desktop/mobile, and the renderer fallback. Review follow-up `bd8118f` verifies the marked ambiguous hop's dashed underline. - Browser verified: `http://127.0.0.1:55635`; desktop/mobile path screenshots were inspected. The broader smoke runner exited successfully with fixture-dependent skips. - Required frontend checks pass: 99 filter, 18 aging, 666 helpers. CSS variables, seven CSS self-tests, 31 XSS sink checks, 17 XSS gate self-tests and XSS diff preflight pass. - Three independent reviews found no blocking issues. Traversal remains linear with no new requests, settings, dependencies or cache invalidation; styling uses the existing customizer token. ## Preflight overrides - The external preflight runner is absent; corresponding scoped gates passed. Red browser evidence is local, with upstream CI approval tracked separately under the process in Kpa-clawbot#1922. - Existing rapid-navigation map resize timer errors remain visible in browser logs and are outside this change. (cherry picked from commit a938176) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…tline Independent review found that the new marker uses outline, which is this app's focus affordance (style.css "Focus Indicators": a:focus-visible gets outline: 2px solid var(--accent)). Measured on the rendered chain, an unfocused marked hop drew a 1px blue outline and a focused one 2px in a near-identical blue — so the marker read as focus, and real focus was almost indistinguishable from it while tabbing an 18-hop chain. box-shadow: 0 0 0 1px draws the same ring, keeps the property the original comment was reaching for (it does not disturb the shared renderer's dashed ambiguity border, verified by the existing dashed-bottom assertion), and leaves outline exclusively to :focus-visible. The test's ring detection only recognised outline/border, so it now also accepts a shadow ring, and asserts the marker does NOT use outline. Both mutations fail: reverting to outline → 8 failures, removing the ring → 8. Not changed, and reported instead: the review traced that the case-sensitive pubkey compare this PR also fixes was never actually reachable — both sides come from the same lowercase nodes.public_key column (the uppercase domain is the observers table, which never feeds path hops). The lowercasing is kept as harmless normalisation, but it is hardening, not a fix for an observed bug. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Split out of #25 (commit
6ca5964dthere). This branch holds exactly one upstream change so it can be reviewed, tested and reverted on its own.Upstream
a938176f831f979ef0442594ab33286e2585724fgit cherry-pick -xonto masterfda24ca5; upstream authorship kept, and the commit message carries the(cherry picked from commit …)line.public/nodes.js,public/style.css,test-issue-1146-path-link-contrast-e2e.js.Problem
In “Paths Through This Node” the selected node was matched with a case-sensitive pubkey comparison and highlighted only by inline bold styling, so it was easy to miss or not marked at all.
Change
Case-insensitive comparison on full public keys; the matching hop gets the
.hop-currentclass, which now draws a compact outline usingvar(--link-color)(no hardcoded colour). Same-prefix siblings and unresolved hops stay unmarked.Adaptation to this fork
None. The cherry-pick applied without conflicts and the changed lines are identical to upstream.
Dependencies and merge order
fda24ca5and needs no other PR from this split.TestPruneOldNeighborMetricsdeterministic). If test(ingestor): make neighbor metrics pruning deterministic #33 lands first, the expected CI failure named below disappears; nothing in this PR depends on it.Verification
Local run of the same commands as CI's “Go Build & Test” job (server tests with
-race), on this branch and on masterfda24ca5under the same conditions (same machine, run one after another):fda24ca5xss-gate-diffchannel-lib-testdecrypt-cli-build-testdockerfile-copy-invariantsdeclare -A), macOS has 3.2; identical on masterstaging-disk-monitorcss-vars-linttest-issue-1375-scope-stats-fetch.jsExactly oneapi('/scope-stats'call exists (the fixed loader) — found 2test-issue-1648-m4-emoji-scan.jsmap.js has 1 emoji/misc-icon hit(s):test-a11y-axe-routes-coverage.jsaxe ROUTES missing analytics tabs (issue #1706): areas, foreign-traffic, wardrivingtest-frontend-helpers.jsfavStar returns empty star for non-favorite: The expression evaluated to a falsy value:,favStar returns filled star for favorite: The expression evaluated to a falsy value:Baseline failures (fail identically on master; not introduced or changed here): see rows marked baseline failure, unchanged.
Browser validation (local, fixture DB, no staging/production): Local Go server (built from master) on the committed E2E fixture DB (freshened, migrated and seeded exactly like CI), serving this branch's
public/, compared side by side with the same server serving master'spublic/. Full node detail for a fixture repeater that appears in its own "Paths Through This Node" chain. master: the matching hop has.hop-currentwith bold text only (outline: none). this branch: the same hop carries.hop-currentwith bold text andoutline: solid 1pxin--link-color(rgb(37, 99, 235) in the light theme); screenshot shows the outlined node name at the start of the chain, other hops unmarked.Not run:
test-issue-1146-path-link-contrast-e2e.js(Playwright) have not run anywhere for this fork.eslint(not installed locally; CI installs it on the fly).-race/tests for modules this PR does not touch (unchanged code, identical to master).Expected GitHub CI: “Go Build & Test” is expected to fail on
TestPruneOldNeighborMetrics, which already fails on master (see #25's run). Downstream jobs (Playwright, image build) are therefore skipped. “Deploy Staging” and all GHCR publish steps only run onpushtomasterand cannot run for this PR.Two further ingestor tests have failed intermittently in this split's CI on branches whose
cmd/ingestortree is byte-identical to master (#27, #28), so they can also appear here without being caused by this change:TestBackfillTxLastSeen_ResolvesFromMaxObservationTimestamp: also reproduced locally on unmodified master.TestMQTTStallWatchdog_DisconnectedEscalationThrottled_1749: the suite flake that upstream test(ingestor): join the watchdog loop goroutine instead of only asking it to stop Kpa-clawbot/CoreScope#2003 (also split out of port(upstream): 26 clean upstream fixes — prune batching, /ws limits, observer liveness, watchdog race #25) addresses.GitHub CI result: run 34752478311 on
50045bbd. Go Build & Test: failure; all downstream jobs incl. Deploy Staging skipped. Failed tests:TestPruneOldNeighborMetrics: fails on master, documented baseline🤖 Generated with Claude Code