Skip to content

feat(#1784): wire display consumers to path trust threshold - #1841

Open
Bjorkan wants to merge 6 commits into
Kpa-clawbot:masterfrom
Bjorkan:feat/path-trust-display-consumers-1784
Open

feat(#1784): wire display consumers to path trust threshold#1841
Bjorkan wants to merge 6 commits into
Kpa-clawbot:masterfrom
Bjorkan:feat/path-trust-display-consumers-1784

Conversation

@Bjorkan

@Bjorkan Bjorkan commented Jul 10, 2026

Copy link
Copy Markdown

Fixes #1784

Summary

Step 4 of the #1784 multi-PR project — wires all frontend display consumers to respect the configured pathTrust.minHashBytesForMapping value.

Depends on: #1840 (must be merged first — provides MC_meetsPathTrust / MC_pathBelowTrust / MC_getPathTrustThreshold helpers and PATH_TRUST global).

Changes

map.js — trust-gated route display

  • drawPacketRoute checks MC_pathBelowTrust before drawing polylines
  • When all path hops are below the configured threshold, shows a "Route not displayed" message with config guidance instead of speculative polylines
  • Cleans up the trust message control when a new route is drawn

analytics.js — subpath trust filtering

  • renderTable in renderSubpaths filters route patterns whose hops are below trust threshold
  • Combined filter logic: both the 1-byte hide toggle AND trust threshold are applied together
  • Info line shows which filters are active ("1-byte hide + 2-byte trust", etc.)
  • No-data message explains which filters caused exclusion

route-view.js — speculative path annotation

  • Path picker groups tagged with belowTrust flag when any hop doesn't meet the configured threshold
  • Speculative paths show (speculative, <N-byte hops) annotation with tooltip explaining the trust threshold
  • Tooltip includes guidance on changing pathTrust.minHashBytesForMapping in config.json

live.js — Paths Through widget

  • _pathHopsBelowTrust() helper checks whether all hops in a path are below trust threshold
  • Widget fallback message distinguishes between "1-byte filtered" (display toggle) and "N-byte trust threshold" (server config)
  • Message includes the config key for operators to adjust

nodes.js — confidence weight adjustment

Testing

test-issue-1633-hide-1byte-hops.js:

  • 5 new source-grep guards verifying each consumer references the trust threshold helpers
  • All 26 tests passing (21 existing + 5 new)

Files changed (6 files, +136/-6)

public/analytics.js                | 28 ++++++++++++++++++---
public/live.js                     | 19 ++++++++++++++-
public/map.js                      | 21 ++++++++++++++++
public/nodes.js                    |  8 ++++++
public/route-view.js               | 16 +++++++++++-
test-issue-1633-hide-1byte-hops.js | 50 ++++++++++++++++++++++++++++++

Depends on: #1840
Written by: DeepSeek V4 Pro in Max Mode

…pping config

Step 3 of the Kpa-clawbot#1784 multi-PR project — wires the shared
packetpath.MeetsPathTrust helper (from PR Kpa-clawbot#1824) into all consumer sites
so the configurable trust threshold actually gates mapping/topology evidence.

Server-side:
- neighbor_graph.go: BuildOptions gains PathTrust field; edge building skips
  observations whose prefix length is below the configured threshold
- neighbor_api.go: passes s.cfg.PathTrust through to BuildOptions
- path_inspect.go: hopEvidence gains Trusted field; candidates marked
  speculative when any hop is below threshold; stats include threshold
- /api/config/client: exposes pathTrust to the frontend

Frontend:
- roles.js: PATH_TRUST global populated from config client
- hop-filter.js: MC_getPathTrustThreshold(), MC_meetsPathTrust(),
  MC_pathBelowTrust() — shared decision points for render-time consumers
- customize-v2.js: trust threshold info shown below the existing
  hide-1byte-hops toggle in Path Display section

Default: 1 (backward-compatible, trust-all). Operators wanting the
conservative default (exclude 1-byte) can set minHashBytesForMapping: 2
in config.json; strictest mode uses 3.

Testing:
- trust_test.go: 10 tests covering default=1, threshold=1/2/3, clamping,
  zero-value fallback, nil safety
- test-issue-1633-hide-1byte-hops.js: 10 additional tests for the new
  MC_* functions (all passing: 31/31 total)

Depends on: PR Kpa-clawbot#1824 (SaarMesh-Bot) — this PR includes the foundation
files from that branch for compilation; should be merged after Kpa-clawbot#1824.

Written by: DeepSeek V4 Pro in Max Mode
@Kpa-clawbot

Copy link
Copy Markdown
Owner

Independent adversarial review — pr-polish parallel fan-out (round 1)

Reviewed at HEAD 9ae4203. Diff: +136/−6, 6 files, pure frontend, tests present. Verdict: NEEDS-WORK — 2 MAJOR, 2 minor. Chain (#1824#1840#1841) not a review blocker (code is typeof-guarded) but IS a merge-order blocker.

MAJOR

  1. Inconsistent default when MC_getPathTrustThreshold is undefined (dijkstra). analytics.js:2020 and nodes.js:262 fall back to 1 (no-op). map.js:646, live.js:2632, route-view.js:397 fall back to 2 (filter/annotate). If feat(#1784): wire consumers to pathTrust.minHashBytesForMapping config #1840 fails to expose the global, map + live + route-view aggressively filter/annotate while analytics + nodes do nothing → user sees inconsistent UI state across widgets on the same page. Pick one default (recommend 1 — silent no-op is safer than false-negative route hiding) and apply uniformly. Add a single MC_pathTrustDefault helper to eliminate the drift.

  2. Source-grep tests are tautological (kent-beck). All 5 new tests in test-issue-1633-hide-1byte-hops.js:451-493 are src.indexOf('X') !== -1 checks. A stub function MC_meetsPathTrust(){return true} that ignores its argument would pass every one. Kent Beck Q(c) "could a wrong implementation pass this test?" — yes. At least one JSDOM behavior test needed: e.g., set window.MC_getPathTrustThreshold=()=>2, window.MC_meetsPathTrust=(h)=>h.length>=4, render, assert Route not displayed appears and speculative annotation is present.

Minor

  1. nodes.js:266-269 redundant (carmack). for(_m<_tt) modeWeight[_m]=0 already zeroes bucket 0; the subsequent if(_tt>=2) modeWeight[0]=0 is dead code inside a nested if(_tt>=2). Also: bucket-0 (legacy/snapshot edges) silent mass-zero when threshold≥2 needs one line in operator docs — it can materially drop confidence scores for pre-existing edges.

  2. map.js:652 theme drift (tufte). Control uses hardcoded #1e293b/#e2e8f0 fallbacks inline — bypasses CSS-var theming on non-dark themes. Use pure var(--input-bg)/var(--text) without literal fallbacks, or reuse an existing themed class.

Not blocking: chain reviewable in isolation due to defensive guards, but merge in order #1824#1840#1841.

Bjorkan added 5 commits July 10, 2026 11:06
…estor surface, nil-safe, tests

- BLOCKER (munger): Sync DefaultMinHashBytesForMapping to 2 per Kpa-clawbot#1824.
  Wire-format minimum is no longer the default threshold — operator-
  confirmed. Update trust_test.go, hop-filter.js fallback, and
  config.example.json to match.
- MAJOR (dijkstra): Remove ingestor-side PathTrust surface since no
  consumer reads it. Field, type alias, method and import removed
  from cmd/ingestor/config.go.
- MINOR (carmack): Use s.cfg.GetPathTrust() consistently in
  path_inspect.go so nil s.cfg doesn't panic. Precompute
  statsMinBytes for the struct literal.
- MAJOR (kent-beck): Add TestBuildNeighborGraph_PathTrustExcludes1ByteHops
  — consumer wiring test that proves edges are skipped at threshold=2
  but built at threshold=1.
- MINOR (tufte): Fix customize-v2.js trust-info: remove misleading
  'or hidden', collapse two adjacent hint paragraphs into one.
… GetPathTrust, hoist loop var

- roles.js: PATH_TRUST fallback now 2 (was 1), matching DefaultMinHashBytesForMapping
- routes.go: handleConfigClient uses GetPathTrust() so nil s.cfg.PathTrust
  doesn't drop the field — default is always serialized
- customize-v2.js: inline fallback 1→2 for trust threshold
- neighbor_graph_test.go: remove mangled/stale comment block
- path_inspect.go: hoist GetPathTrust() out of per-hop evidence loop,
  fix extra indentation
Step 4 of the Kpa-clawbot#1784 multi-PR project — wires all frontend display
consumers to respect the configured pathTrust.minHashBytesForMapping
value. Depends on PR Kpa-clawbot#1840 (server-side foundation + config exposure).

Display consumers wired:
- map.js: drawPacketRoute checks MC_pathBelowTrust before drawing
  polylines; shows "Route not displayed" message with config guidance
  when all path hops are below the configured trust threshold
- analytics.js: subpath renderTable filters route patterns whose
  hops are below trust, with clear messaging in the info line
  explaining which filters are active
- route-view.js: path picker groups tagged with belowTrust flag;
  speculative paths show "(speculative, <N-byte hops)" annotation
  with tooltip explaining the trust threshold
- live.js: Paths Through widget distinguishes between 1-byte-filtered
  and trust-threshold-filtered paths in its fallback message
- nodes.js: confidence mode weights zeroed for hash modes below the
  configured trust threshold (bucket-0 excluded at threshold >= 2)

Testing:
- test-issue-1633-hide-1byte-hops.js: 5 new source-grep guards
  verifying each consumer references the trust threshold helpers
  (all 26 tests passing)

Depends on: PR Kpa-clawbot#1840 (must be merged first — provides the
MC_meetsPathTrust / MC_pathBelowTrust / MC_getPathTrustThreshold
helpers and the PATH_TRUST global)

Written by: DeepSeek V4 Pro in Max Mode
…e, CSS vars, behavioral test

- MAJOR 1 (dijkstra): Unify inline fallback for MC_getPathTrustThreshold
  to 1 across all consumers (map.js, live.js, route-view.js). Was
  inconsistent: analytics/nodes fell back to 1, map/live/route-view
  fell back to 2.
- MAJOR 2 (kent-beck): Add anti-tautology behavioral test that verifies
  MC_pathBelowTrust responds to actual hop content (1-byte vs 2-byte),
  not just a source-grep reference check.
- MINOR 3 (carmack): Remove dead modeWeight[0]=0 assignment in nodes.js
  (loop already zeroes bucket 0 when _tt >= 2).
- MINOR 4 (tufte): Replace hardcoded #1e293b / #e2e8f0 fallbacks with
  pure CSS vars (--input-bg / --text) in map.js trust-message control.
…age, fix stale comment

- map.js: compute _hb as max across all hopKeys, not just first hop, so the
  trust-not-met message correctly reports the largest byte length seen
- analytics.js: update stale comment claiming default=1
@Bjorkan
Bjorkan force-pushed the feat/path-trust-display-consumers-1784 branch from 9ae4203 to 368211a Compare July 10, 2026 09:28
@Bjorkan

Bjorkan commented Jul 10, 2026

Copy link
Copy Markdown
Author

Review feedback addressed (commits de71d2b, 368211a):

MAJOR 1 (dijkstra): Unified all inline fallbacks for MC_getPathTrustThreshold to 1 across map.js, live.js, route-view.js (was inconsistent: 3 consumers used 2, 2 used 1). The production initialization path (roles.js) sets the real default (2); inline fallbacks are dead-code safety nets.

MAJOR 2 (kent-beck): Added anti-tautology behavioral test (#1784: anti-tautology — MC_pathBelowTrust returns correct boolean for given hops). Sets PATH_TRUST=2, verifies 2-byte hop is NOT below trust, 1-byte hop IS below trust, mixed path works, and threshold 1 returns false. A stub returning hardcoded true would fail this test.

MINOR 3 (carmack): Removed dead code in nodes.js — duplicate modeWeight[0]=0 assignment (loop already zeroes it).

MINOR 4 (tufte): Replaced hardcoded #1e293b / #e2e8f0 fallbacks with pure CSS vars (--input-bg / --text) in map.js trust-message control.

Additional fixes from subagent review:

  • map.js: _hb now computes max byte length across ALL hopKeys (not just hopKeys[0]), so trust-message correctly reports worst-case
  • analytics.js: updated stale comment claiming default=1

Chain: Rebased on updated feat/path-trust-consumers-1784 (includes DefaultMinHashBytesForMapping=2, roles.js fallback fix, routes.go GetPathTrust fix). All 37 JS tests pass.

@Kpa-clawbot

Copy link
Copy Markdown
Owner

Round-2 verification (parent grep, no re-spawn)

Prior round-1 findings — verified at HEAD 368211a:

# Reviewer Item Landed Evidence
1 dijkstra (MAJOR) Unify MC_getPathTrustThreshold inline fallback across all 5 consumers All 5 sites (map.js, live.js, route-view.js, analytics.js, nodes.js) now use ... : 1 (safe no-op fallback); real default is set by roles.js from server config
2 kent-beck (MAJOR) Add anti-tautology behavioral test (source-grep tests were stubbable) New test #1784: MC_getPathTrustThreshold reads window.PATH_TRUST sets PATH_TRUST=2/3, asserts round-trip; MC_pathBelowTrust behavioral test with mixed hop lengths (a stub returning constant true/false would fail)
3 carmack (MINOR) Dead code in nodes.js bucket-0 zeroing Duplicate modeWeight[0]=0 after the loop removed
4 tufte (MINOR) Hardcoded #1e293b/#e2e8f0 fallbacks bypass CSS-var theming Replaced with pure var(--input-bg) / var(--text) in map.js:652 trust-message control

Self-driven additions noted (map.js _hb computes max byte length across all hopKeys not just [0], analytics.js stale-comment fix) — present in diff.

Zero BLOCKER / MAJOR remain from grep re-pass. No new findings.

Merge blocker (structural)

mergeStateStatus: BLOCKED — chain dependency. Merge order: #1824#1840#1841. #1824 (foundation helper) still has 0 reviews and no CI checks; #1840 verified merge-clean but blocked on #1824. This PR (#1841) is defensively guarded (all consumers typeof-check the global), so it's independently reviewable, but must not merge before its dependencies.

Auto-merge not enabled — no CI checks reported on this branch (workflow triggers on master push per repo config).

Recommend operator: shepherd #1824 through review first.

@Kpa-clawbot

Copy link
Copy Markdown
Owner

Bot polish review — round 1 (parallel)

Verdict: needs-changes
3-axis: mergeable=MERGEABLE · CI=not-run (fork-approval-gated) · reviews=1 BLOCKER / 2 MAJOR / 2 nit

[dijkstra] BLOCKER — default drift, same as #1840. public/roles.js:609-611 sets window.PATH_TRUST = cfg.pathTrust?.minHashBytesForMapping || 2 and public/hop-filter.js:55 also falls back to 2. Combined with #1840's Go Default=2, upgrading operators without a pathTrust config block silently lose 1-byte polylines (map.js), subpath rows (analytics.js), and bucket-1 confidence (nodes.js). PR body claims "backward-compatible" — pick 1 or 2 and align everywhere (Go default, JS fallback in roles.js + hop-filter.js, PR bodies, config.example.json).

[kent-beck] MAJOR — nodes.js:263-265 silently zeros bucket-0 at default=2. for (var _m = 0; _m < _tt; _m++) modeWeight[_m] = 0 — with _tt=2, bucket-0 (pre-#1638 persisted edges, comment at :258-260 explicitly weights 0.5) collapses to zero. Confidence for every snapshot-restored edge drops to 0 until fresh observations arrive. internal/packetpath/trust.go:41-50 docs bucket-0 policy differently (bucket-0 is "unknown", not "1-byte"). No test guards this. Preserve bucket-0's 0.5 independently, or add a failing test.

[tufte] MAJOR — inconsistent gating copy + map.js:653 theme drift. map.js "Route not displayed" leaflet-bar uses inline cssText w/ hard-coded rgba(0,0,0,0.3) box-shadow (light-mode drift) and omits the pathTrust.minHashBytesForMapping config-key hint that live.js:2626, route-view.js:394, analytics.js:2039 all include. Move cssText to a class, align copy.

[carmack] nitanalytics.js:2019 reads window.MC_getPathTrustThreshold on every renderTable; hoist per render pass.

[kent-beck] nittest-issue-1633-hide-1byte-hops.js:601 consumer-wiring guards are token-grep only; don't fail on behaviour revert. Add a DOM-level test for the map.js "Route not displayed" branch.

Blocked on: (1) resolve default drift w/ #1840; (2) fix bucket-0 collapse in nodes.js; (3) map.js copy + theming.

@Kpa-clawbot

Copy link
Copy Markdown
Owner

PR #1841 Review — path trust display consumers

Dependency note: This PR depends on #1840#1824. Reviewed in isolation; merge ordering is the author's concern.


[MAJOR] Stale trust banner on map (Carmack)map.js: the __mc_routeTrustControl Leaflet control is added on below-trust early return, but the normal route-drawing path never removes it. Navigating from a below-trust packet to an above-trust packet leaves "Route not displayed" stuck on the map. Add if (window.__mc_routeTrustControl) { try { map.removeControl(window.__mc_routeTrustControl); } catch(_e){} window.__mc_routeTrustControl = null; } at the top of drawRoute before the trust gate.

[MAJOR] Speculative flag broadened without API doc (Dijkstra)path_inspect.go:261: Speculative now includes !allHopsTrusted, changing semantics for API consumers who keyed on score < speculativeThreshold. Any external tooling parsing /api/path-inspect will see paths marked speculative that previously weren't. The trusted per-hop field is good — but the aggregate flag change needs at minimum a comment or changelog note.

[MINOR] Unrelated comment deletions (Beck)path_inspect.go removes two clarifying comments ("Score this alternative…", "Sort alts by score desc, cap at 5") unrelated to trust wiring. Keep the PR scoped; restore or split.

[MINOR] Redundant GetPathTrust() call (Munger)path_inspect.go:264: statsMinBytes := s.cfg.GetPathTrust() duplicates the pt variable from line 221. Use pt directly: "minHashBytesForMapping": pt.MinHashBytesOrDefault().

[MINOR] nodes.js zeroes bucket-0 weight (Carmack)modeWeight[0] = 0 when _tt >= 2 changes legacy/unknown-bucket weight from 0.5→0. Intentional? Legacy edges predating per-mode tracking lose all confidence contribution. Worth a comment if deliberate.

Tests: Thorough — unit tests for MeetsPathTrust, integration test for BuildFromStoreWithOptions with trust gate, JS consumer wiring guards, anti-tautology checks. Well done. (Beck)

Config: config.example.json updated with field + _comment_pathTrust. Meets config documentation rule. (Munger)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add configurable trust threshold for 1-byte/2-byte path-hash observations

2 participants