Skip to content

fix(ui): prevent duplicate LazyColumn keys in node metrics logs#5890

Merged
jamesarich merged 1 commit into
mainfrom
fix/metrics-lazycolumn-duplicate-key-crash
Jun 21, 2026
Merged

fix(ui): prevent duplicate LazyColumn keys in node metrics logs#5890
jamesarich merged 1 commit into
mainfrom
fix/metrics-lazycolumn-duplicate-key-crash

Conversation

@jamesarich

Copy link
Copy Markdown
Collaborator

The node-detail metrics screens crash with IllegalArgumentException: Key "<timestamp>" was already used when a node reports two telemetry or position samples in the same wall-clock second. Several LazyColumns keyed their items on a bare second-resolution timestamp, so same-second samples collide and Compose tears the screen down. Observed on the 2.8.0 internal track (versionCode 29321164); the crash recurs across versions.

🐛 Bug Fixes

  • Fixed the duplicate-key crash in the node metrics log lists by giving each LazyColumn item a guaranteed-unique key. The Device, Environment, Power, Air Quality, and Position logs now key on "${time}_$index" (the pattern HostMetricsLog already used) instead of the bare telemetry.time / position.time.
  • Fixed the same latent defect in SignalMetrics.LocalStatsEntry (its key was "local_stats_${time}"); it now threads the source-list index so two local-stats samples in the same second can't collide. The sibling PacketEntry keeps its real packet-id key ("packet_${meshPacket.id}").

🛠️ Refactoring & Architecture

  • Standardized item keys across the whole feature/node metrics module on two patterns: a real unique id (uuid / packet id) where the list element carries one, and "${time}_$index" for the id-less Telemetry / Position protos. No bare-timestamp keys remain.

Why not a real id everywhere?

The data layer does have a unique id for every one of these lists (each Telemetry is parsed from a MeshLog with a uuid; positions come from a MeshPacket with .id), but those ids are discarded when the rows are mapped to bare protos before reaching the UI state. Threading them through to a single key = { it.uuid } idiom would be a data-model refactor (changing MetricsState's list types and rippling through the use case, charts, and CSV exporters), so it's out of scope for this crash fix and noted as a possible follow-up.

Testing Performed

  • ./gradlew spotlessApply spotlessCheck detekt :feature:node:allTests kmpSmokeCompile — all green (722 tests pass; Spotless and detekt clean).
  • No test changes: this is a Compose key-stability fix with no behavioral change to the rendered cards or the selection/highlight logic.

Several node-detail metrics log lists keyed their LazyColumn items on a
bare second-resolution timestamp (telemetry.time / position.time). When a
node delivers two telemetry or position packets within the same second
(bursts, resends), two items share a key and Compose throws
IllegalArgumentException: "Key ... was already used", crashing the
metrics screens (seen on the 2.8.0 internal track, versionCode 29321164).

Give every item a guaranteed-unique key, matching the existing
HostMetricsLog pattern:

- Device, Environment, Power, Air Quality, and Position logs now key on
  "${time}_$index" instead of the bare timestamp.
- SignalMetrics.LocalStatsEntry had the same bare-timestamp defect; it now
  threads the source-list index ("local_stats_${time}_$index"), keeping
  the sibling PacketEntry's real packet-id key intact.

This standardizes the whole node-metrics module on two key patterns: a
real unique id (uuid / packet id) where the element carries one, and
"${time}_$index" for the id-less Telemetry/Position protos.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions github-actions Bot added the bugfix PR tag label Jun 21, 2026
@jamesarich jamesarich added this pull request to the merge queue Jun 21, 2026
Merged via the queue into main with commit 0ce0c28 Jun 21, 2026
22 checks passed
@jamesarich jamesarich deleted the fix/metrics-lazycolumn-duplicate-key-crash branch June 21, 2026 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix PR tag

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant