fix: stabilize traceroute map node identity & fix log metric alignment - #6270
Conversation
…nment Fixes two bugs in the traceroute UI/rendering path. #6197 — Traceroute map node IDs changing every reload: the google-flavor traceroute marker loop emitted MarkerComposables positionally with no composition key and empty MarkerComposable keys. When displayNodes reorders between reloads, marker slots (and their cached icon bitmaps) get reused positionally, so node chips/positions appear swapped and unstable. Wrap each marker in key(node.num) and pass node.num + short_name as MarkerComposable keys, matching the existing NodeTrack marker pattern. #5743 — Traceroute log metric alignment: the three metric labels (forward hops / return hops / round trip) were laid out in a plain Row that can't fit three items on the narrow card, crushing the last item to ~zero width so it wrapped one character per line (worse with long translated strings). Use FlowRow so items wrap onto a new line cleanly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughTraceroute map markers now use stable Compose keys tied to node identity and receive node metadata inputs. Traceroute log metrics now use a wrapping ChangesTraceroute UI
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@androidApp/src/google/kotlin/org/meshtastic/app/map/MapView.kt`:
- Around line 1265-1272: Update the marker key arguments around NodeChip and
MarkerComposable to include node.colors and node.isIgnored in addition to
node.num and node.user.short_name, ensuring the marker recomposes when any
NodeChip input changes while preserving the stable node identity.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 27daed07-24b0-4cdb-bb7e-1d75c92e7bb9
📒 Files selected for processing (2)
androidApp/src/google/kotlin/org/meshtastic/app/map/MapView.ktfeature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/TracerouteLog.kt
Add node.colors and node.isIgnored to the MarkerComposable keys so the cached marker icon bitmap refreshes when those NodeChip inputs change (not just node.num / short_name). Prevents stale colors or strike-through styling on a marker whose num and short name are unchanged. Addresses CodeRabbit review. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Why
Two related bugs in the traceroute UI/rendering path, grouped because they live in the same view and are cheap to fix together.
🐛 #6197 — Traceroute map node IDs change on every reload
On the traceroute map, node markers show the wrong IDs, and re-opening the same traceroute shuffles them. The google-flavor marker loop in
TracerouteMapContentemittedMarkerComposables positionally with no composition key and noMarkerComposablekeys:displayNodes.forEach { node -> val markerState = rememberUpdatedMarkerState(position = node.position.toLatLng()) MarkerComposable(state = markerState, zIndex = 4f) { NodeChip(node = node) } }displayNodesis derived from a live, time-sorted node flow, so its order is not stable between reloads.MarkerComposablecaches its rendered icon bitmap keyed by itskeysvararg (empty here), and Compose reuses marker slots positionally — so when the list reorders, a marker keeps a stale cached chip while getting a different node's position, making node IDs look wrong and unstable. This is the same unstable-key class of bug seen elsewhere in the app (lists keyed on index/timestamp instead ofnode.num).Fix: wrap each marker in
key(node.num)and passnode.num+short_nameasMarkerComposablekeys, so each marker's composition state and cached bitmap stay bound to its stable node identity. This mirrors the existingNodeTrackMapmarker pattern, which already wraps markers inkey(position.time).🐛 #5743 — Traceroute log metric alignment
In the Traceroute Logs list, the third metric label (round-trip time) was crushed and wrapped one character per line vertically. The metrics sat in a plain
Rowwith three items that can't fit the narrow card — worse with longer translated strings (reported in Russian, also reproduces on desktop and in English).Fix: use
FlowRowso the metrics wrap onto a new line cleanly instead of the last item being squeezed to zero width.Testing Performed
./gradlew spotlessApply spotlessCheck detekt— clean, no violations./gradlew assembleDebug— passes (all flavors)./gradlew :feature:node:allTests— passes (~130 tests)Marker keying is a Compose-recomposition fix that isn't covered by the existing headless tests; the node-selection resolution (
tracerouteNodeSelection, already num-keyed) remains covered byTracerouteNodeSelectionTest.Closes #6197
Closes #5743
🤖 Generated with Claude Code
Summary by CodeRabbit