From 2257c091e8acc70ed9f2e6bc7fc3a590053d1710 Mon Sep 17 00:00:00 2001 From: Jack Kingsman Date: Sat, 7 Mar 2026 20:55:58 -0800 Subject: [PATCH] Fix visualizer coercion for multibyte hops --- LICENSES.md | 2 +- app/models.py | 3 +- app/radio.py | 42 +++++++++++++-- .../components/AGENTS_packet_visualizer.md | 14 ++--- .../src/components/PacketVisualizer3D.tsx | 27 +++++----- frontend/src/test/visualizerUtils.test.ts | 51 +++++++++++++++++++ frontend/src/utils/visualizerUtils.ts | 48 +++++++++++++---- tests/test_contacts_router.py | 22 ++++++++ tests/test_repository.py | 13 +++++ 9 files changed, 187 insertions(+), 35 deletions(-) create mode 100644 frontend/src/test/visualizerUtils.test.ts diff --git a/LICENSES.md b/LICENSES.md index 77a7c2f..79975a6 100644 --- a/LICENSES.md +++ b/LICENSES.md @@ -144,7 +144,7 @@ THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND -### meshcore (2.2.5) — MIT +### meshcore (2.2.28) — MIT
Full license text diff --git a/app/models.py b/app/models.py index 1da3790..3561662 100644 --- a/app/models.py +++ b/app/models.py @@ -82,7 +82,8 @@ class ContactAdvertPath(BaseModel): path: str = Field(description="Hex-encoded routing path (empty string for direct)") path_len: int = Field(description="Number of hops in the path") next_hop: str | None = Field( - default=None, description="First hop toward us (2-char hex), or null for direct" + default=None, + description="First hop toward us as a full hop identifier, or null for direct", ) first_seen: int = Field(description="Unix timestamp of first observation") last_seen: int = Field(description="Unix timestamp of most recent observation") diff --git a/app/radio.py b/app/radio.py index 02837a1..a9e2fbf 100644 --- a/app/radio.py +++ b/app/radio.py @@ -274,21 +274,53 @@ class RadioManager: "set_flood_scope failed (firmware may not support it): %s", exc ) - # Query path hash mode support (best-effort; older firmware won't report it) + # Query path hash mode support (best-effort; older firmware won't report it). + # If the library's parsed payload is missing path_hash_mode (e.g. stale + # .pyc on WSL2 Windows mounts), fall back to raw-frame extraction. + reader = mc._reader + _original_handle_rx = reader.handle_rx + _captured_frame: list[bytes] = [] + + async def _capture_handle_rx(data: bytearray) -> None: + from meshcore.packets import PacketType + + if len(data) > 0 and data[0] == PacketType.DEVICE_INFO.value: + _captured_frame.append(bytes(data)) + return await _original_handle_rx(data) + + reader.handle_rx = _capture_handle_rx + self.path_hash_mode = 0 + self.path_hash_mode_supported = False try: device_query = await mc.commands.send_device_query() if device_query and "path_hash_mode" in device_query.payload: self.path_hash_mode = device_query.payload["path_hash_mode"] self.path_hash_mode_supported = True + elif _captured_frame: + # Raw-frame fallback: byte 1 = fw_ver, byte 81 = path_hash_mode + raw = _captured_frame[-1] + fw_ver = raw[1] if len(raw) > 1 else 0 + if fw_ver >= 10 and len(raw) >= 82: + self.path_hash_mode = raw[81] + self.path_hash_mode_supported = True + logger.warning( + "path_hash_mode=%d extracted from raw frame " + "(stale .pyc? try: rm %s)", + self.path_hash_mode, + getattr( + __import__("meshcore.reader", fromlist=["reader"]), + "__cached__", + "meshcore __pycache__/reader.*.pyc", + ), + ) + if self.path_hash_mode_supported: logger.info("Path hash mode: %d (supported)", self.path_hash_mode) else: - self.path_hash_mode = 0 - self.path_hash_mode_supported = False logger.debug("Firmware does not report path_hash_mode") except Exception as exc: - self.path_hash_mode = 0 - self.path_hash_mode_supported = False logger.debug("Failed to query path_hash_mode: %s", exc) + finally: + reader.handle_rx = _original_handle_rx # Sync contacts/channels from radio to DB and clear radio logger.info("Syncing and offloading radio data...") diff --git a/frontend/src/components/AGENTS_packet_visualizer.md b/frontend/src/components/AGENTS_packet_visualizer.md index 1f09167..fa4be89 100644 --- a/frontend/src/components/AGENTS_packet_visualizer.md +++ b/frontend/src/components/AGENTS_packet_visualizer.md @@ -172,15 +172,15 @@ function resolveNode(source, isRepeater, showAmbiguous): string | null { ### Ambiguous Nodes -When only a 1-byte prefix is known (from packet path bytes), the node is marked ambiguous and shown with a `?` prefix and gray styling. However, if the node is identified as a repeater (via advert or path hop), it shows blue regardless of ambiguity. +When only a partial hop token is known (for example a 1-byte hop from an older radio), the node is marked ambiguous and shown with a `?` prefix and gray styling. Full 2-byte and 3-byte hop tokens are preserved as distinct identities and are not collapsed back to their first byte. However, if the node is identified as a repeater (via advert or path hop), it shows blue regardless of ambiguity. ### Advert-Path Identity Hints -**Problem:** When multiple repeaters share a 1-byte prefix, the visualizer can't tell which physical repeater a path hop refers to. +**Problem:** During mixed-radio operation, some observations may only carry a 1-byte hop while others carry a full 2-byte or 3-byte hop token. The visualizer must not collapse the full token back to the first byte, but it also cannot over-resolve the short token. **Solution:** The backend tracks recent unique advertisement paths per contact in `contact_advert_paths` (see root `AGENTS.md` § "Contact Advert Path Memory"). On mount (and when new contacts appear), the visualizer fetches this data via `GET /api/contacts/repeaters/advert-paths` and builds an index keyed by 12-char prefix. -**Scoring:** `pickLikelyRepeaterByAdvertPath(candidates, nextPrefix)` scores each candidate repeater by how often its stored advert paths' `next_hop` matches the packet's actual next-hop prefix. It requires: +**Scoring:** `pickLikelyRepeaterByAdvertPath(candidates, nextPrefix)` scores each candidate repeater by how often its stored advert paths' `next_hop` matches the packet's actual next-hop token. It requires: - At least 2 matching observations (stronger-than-trivial evidence) - The top candidate's match score must be at least 2x the runner-up's @@ -193,7 +193,7 @@ When a winner is found, the ambiguous node gets a `probableIdentity` label (the ### Traffic Pattern Splitting (Experimental) -**Problem:** Multiple physical repeaters can share the same 1-byte prefix (collision). Since packet paths only contain 1-byte hashes, we can't directly distinguish them. However, traffic patterns provide a heuristic. +**Problem:** Multiple physical repeaters can share the same short hop token emitted by older radios. Since those packets only carry the short token, we can't directly distinguish them. However, traffic patterns provide a heuristic. **Key Insight:** A single physical repeater (even acting as a hub) will have the same sources routing through it regardless of next-hop. But if prefix `32` has completely disjoint sets of sources for different next-hops, those are likely different physical nodes sharing the same prefix. @@ -234,9 +234,9 @@ Here source `ae` routes through `32` to BOTH `ba` and `60`. This proves `32` is **Node ID format:** -- Without splitting (default): `?XX` (e.g., `?32`) -- With splitting (after evidence threshold met): `?XX:>YY` (e.g., `?32:>ba`) -- Final repeater: `?XX` (unchanged, no suffix) +- Without splitting (default): `?{hop}` (examples: `?32`, `?aa11`, `?bb22cc`) +- With splitting (after evidence threshold met): `?{hop}:>{nextHop}` (example: `?32:>ba`, `?aa11:>bb22`) +- Final repeater: `?{hop}` (unchanged, no suffix) ## Path Building diff --git a/frontend/src/components/PacketVisualizer3D.tsx b/frontend/src/components/PacketVisualizer3D.tsx index 99683b1..1025ce5 100644 --- a/frontend/src/components/PacketVisualizer3D.tsx +++ b/frontend/src/components/PacketVisualizer3D.tsx @@ -32,6 +32,8 @@ import { type Particle, type PendingPacket, type RepeaterTrafficData, + buildAmbiguousRepeaterLabel, + buildAmbiguousRepeaterNodeId, COLORS, PARTICLE_COLOR_MAP, PARTICLE_SPEED, @@ -538,7 +540,8 @@ function useVisualizerData3D({ } // type === 'prefix' - const matches = contactIndex.byPrefix.get(source.value.toLowerCase()) ?? []; + const lookupValue = source.value.toLowerCase(); + const matches = contactIndex.byPrefix.get(lookupValue) ?? []; const contact = matches.length === 1 ? matches[0] : null; if (contact) { const nodeId = contact.public_key.slice(0, 12).toLowerCase(); @@ -584,13 +587,14 @@ function useVisualizerData3D({ null as number | null ); - let nodeId = `?${source.value.toLowerCase()}`; - let displayName = source.value.toUpperCase(); + let nodeId = buildAmbiguousRepeaterNodeId(lookupValue); + let displayName = buildAmbiguousRepeaterLabel(lookupValue); let probableIdentity: string | null = null; let ambiguousNames = names.length > 0 ? names : undefined; if (useAdvertPathHints && isRepeater && trafficContext) { - const likely = pickLikelyRepeaterByAdvertPath(filtered, trafficContext.nextPrefix); + const normalizedNext = trafficContext.nextPrefix?.toLowerCase() ?? null; + const likely = pickLikelyRepeaterByAdvertPath(filtered, normalizedNext); if (likely) { const likelyName = likely.name || likely.public_key.slice(0, 12).toUpperCase(); probableIdentity = likelyName; @@ -602,25 +606,24 @@ function useVisualizerData3D({ } if (splitAmbiguousByTraffic && isRepeater && trafficContext) { - const prefix = source.value.toLowerCase(); + const normalizedNext = trafficContext.nextPrefix?.toLowerCase() ?? null; if (trafficContext.packetSource) { recordTrafficObservation( trafficPatternsRef.current, - prefix, + lookupValue, trafficContext.packetSource, - trafficContext.nextPrefix + normalizedNext ); } - const trafficData = trafficPatternsRef.current.get(prefix); + const trafficData = trafficPatternsRef.current.get(lookupValue); if (trafficData) { const analysis = analyzeRepeaterTraffic(trafficData); - if (analysis.shouldSplit && trafficContext.nextPrefix) { - const nextShort = trafficContext.nextPrefix.slice(0, 2).toLowerCase(); - nodeId = `?${prefix}:>${nextShort}`; + if (analysis.shouldSplit && normalizedNext) { + nodeId = buildAmbiguousRepeaterNodeId(lookupValue, normalizedNext); if (!probableIdentity) { - displayName = `${source.value.toUpperCase()}:>${nextShort}`; + displayName = buildAmbiguousRepeaterLabel(lookupValue, normalizedNext); } } } diff --git a/frontend/src/test/visualizerUtils.test.ts b/frontend/src/test/visualizerUtils.test.ts new file mode 100644 index 0000000..6bcd9bc --- /dev/null +++ b/frontend/src/test/visualizerUtils.test.ts @@ -0,0 +1,51 @@ +import { describe, expect, it } from 'vitest'; + +import { + analyzeRepeaterTraffic, + buildAmbiguousRepeaterLabel, + buildAmbiguousRepeaterNodeId, + recordTrafficObservation, + type RepeaterTrafficData, +} from '../utils/visualizerUtils'; + +describe('visualizer multibyte hop identity helpers', () => { + it('preserves the full hop token in ambiguous node ids', () => { + expect(buildAmbiguousRepeaterNodeId('aa11')).toBe('?aa11'); + expect(buildAmbiguousRepeaterNodeId('bb22cc')).toBe('?bb22cc'); + }); + + it('preserves the full current and next hop tokens in traffic split ids', () => { + expect(buildAmbiguousRepeaterNodeId('aa', 'bb22')).toBe('?aa:>bb22'); + expect(buildAmbiguousRepeaterNodeId('aa11', 'cc33dd')).toBe('?aa11:>cc33dd'); + }); + + it('formats labels from full hop tokens', () => { + expect(buildAmbiguousRepeaterLabel('aa11')).toBe('AA11'); + expect(buildAmbiguousRepeaterLabel('aa11', 'bb22')).toBe('AA11:>BB22'); + }); +}); + +describe('visualizer traffic pattern grouping', () => { + it('tracks traffic using full hop tokens instead of first-byte buckets', () => { + const traffic = new Map(); + + for (let i = 0; i < 20; i += 1) { + recordTrafficObservation(traffic, 'aa11', `src-a-${i}`, 'bb22'); + recordTrafficObservation(traffic, 'aa22', `src-b-${i}`, 'bb33'); + } + + expect(traffic.has('aa11')).toBe(true); + expect(traffic.has('aa22')).toBe(true); + expect(traffic.has('aa')).toBe(false); + + const firstTraffic = traffic.get('aa11'); + const secondTraffic = traffic.get('aa22'); + expect(firstTraffic).toBeDefined(); + expect(secondTraffic).toBeDefined(); + + const first = analyzeRepeaterTraffic(firstTraffic!); + const second = analyzeRepeaterTraffic(secondTraffic!); + expect(first.shouldSplit).toBe(false); + expect(second.shouldSplit).toBe(false); + }); +}); diff --git a/frontend/src/utils/visualizerUtils.ts b/frontend/src/utils/visualizerUtils.ts index 14e0df0..d0a8263 100644 --- a/frontend/src/utils/visualizerUtils.ts +++ b/frontend/src/utils/visualizerUtils.ts @@ -52,7 +52,7 @@ interface TrafficObservation { } export interface RepeaterTrafficData { - prefix: string; // The 1-byte hex prefix (e.g., "32") + hopKey: string; // The observed hop token (e.g. "32", "aa11", or "bbccdd") observations: TrafficObservation[]; } @@ -110,6 +110,31 @@ export const PACKET_LEGEND_ITEMS = [ { label: '?', color: COLORS.particleUnknown, description: 'Other' }, ] as const; +export function normalizeHopToken(hop: string | null | undefined): string | null { + const normalized = hop?.trim().toLowerCase() ?? ''; + return normalized.length > 0 ? normalized : null; +} + +export function buildAmbiguousRepeaterNodeId(hop: string, nextHop?: string | null): string { + const hopKey = normalizeHopToken(hop); + if (!hopKey) { + return '?'; + } + + const nextHopKey = normalizeHopToken(nextHop); + return nextHopKey ? `?${hopKey}:>${nextHopKey}` : `?${hopKey}`; +} + +export function buildAmbiguousRepeaterLabel(hop: string, nextHop?: string | null): string { + const hopKey = normalizeHopToken(hop)?.toUpperCase(); + if (!hopKey) { + return '?'; + } + + const nextHopKey = normalizeHopToken(nextHop)?.toUpperCase(); + return nextHopKey ? `${hopKey}:>${nextHopKey}` : hopKey; +} + // ============================================================================= // UTILITY FUNCTIONS (Data Layer) // ============================================================================= @@ -274,21 +299,26 @@ export function analyzeRepeaterTraffic(data: RepeaterTrafficData): RepeaterSplit */ export function recordTrafficObservation( trafficData: Map, - prefix: string, + hopKey: string, source: string, nextHop: string | null ): void { - const normalizedPrefix = prefix.toLowerCase(); - const now = Date.now(); - - if (!trafficData.has(normalizedPrefix)) { - trafficData.set(normalizedPrefix, { prefix: normalizedPrefix, observations: [] }); + const normalizedHopKey = normalizeHopToken(hopKey); + if (!normalizedHopKey) { + return; } - const data = trafficData.get(normalizedPrefix)!; + const normalizedNextHop = normalizeHopToken(nextHop); + const now = Date.now(); + + if (!trafficData.has(normalizedHopKey)) { + trafficData.set(normalizedHopKey, { hopKey: normalizedHopKey, observations: [] }); + } + + const data = trafficData.get(normalizedHopKey)!; // Add new observation - data.observations.push({ source, nextHop, timestamp: now }); + data.observations.push({ source, nextHop: normalizedNextHop, timestamp: now }); // Prune old observations data.observations = data.observations.filter( diff --git a/tests/test_contacts_router.py b/tests/test_contacts_router.py index c3b23d2..49d8b4a 100644 --- a/tests/test_contacts_router.py +++ b/tests/test_contacts_router.py @@ -326,6 +326,28 @@ class TestContactDetail: assert repeater["name"] == "Relay1" assert repeater["heard_count"] == 2 + @pytest.mark.asyncio + async def test_detail_nearest_repeaters_use_full_multibyte_next_hop(self, test_db, client): + """Nearest repeater resolution should distinguish multi-byte hops with the same first byte.""" + await _insert_contact(KEY_A, "Alice", type=1) + repeater_1 = "bb11" + "aa" * 30 + repeater_2 = "bb22" + "cc" * 30 + await _insert_contact(repeater_1, "Relay11", type=2) + await _insert_contact(repeater_2, "Relay22", type=2) + + await ContactAdvertPathRepository.record_observation(KEY_A, "bb221122", 1000, hop_count=2) + await ContactAdvertPathRepository.record_observation(KEY_A, "bb223344", 1010, hop_count=2) + + response = await client.get(f"/api/contacts/{KEY_A}/detail") + + assert response.status_code == 200 + data = response.json() + assert len(data["nearest_repeaters"]) == 1 + repeater = data["nearest_repeaters"][0] + assert repeater["public_key"] == repeater_2 + assert repeater["name"] == "Relay22" + assert repeater["heard_count"] == 2 + @pytest.mark.asyncio async def test_detail_advert_frequency_computed(self, test_db, client): """Advert frequency is computed from path observations over time span.""" diff --git a/tests/test_repository.py b/tests/test_repository.py index baaf882..9563388 100644 --- a/tests/test_repository.py +++ b/tests/test_repository.py @@ -273,6 +273,19 @@ class TestContactAdvertPathRepository: assert paths[0].last_seen == 1010 assert paths[0].heard_count == 2 + @pytest.mark.asyncio + async def test_record_observation_preserves_full_multibyte_next_hop(self, test_db): + repeater_key = "ab" * 32 + await ContactRepository.upsert({"public_key": repeater_key, "name": "Rmulti", "type": 2}) + + await ContactAdvertPathRepository.record_observation( + repeater_key, "aa11bb22", 1000, hop_count=2 + ) + + paths = await ContactAdvertPathRepository.get_recent_for_contact(repeater_key, limit=10) + assert len(paths) == 1 + assert paths[0].next_hop == "aa11" + @pytest.mark.asyncio async def test_prunes_to_most_recent_n_unique_paths(self, test_db): repeater_key = "bb" * 32