fix(node): disambiguate signal-log packet keys

[jamesarich]

The Signal Metrics screen crashed for real users on 2.8.0: its LazyColumn keyed packet rows off MeshPacket.id alone, but that id is unique only per originating node and a retransmitted packet is stored once per reception, so genuine duplicates reach the list and Compose throws a fatal IllegalArgumentException. Observed on several 2.8.0 builds, on real devices with normal interaction.

🐛 Bug Fixes

• SignalLogEntry.PacketEntry now carries its source-list index and keys off "packet_<id>_<index>". The sibling LocalStatsEntry already disambiguated this way — packets were simply missed.

🛠️ Refactoring & Architecture

• Extracted the list building out of SignalMetricsScreen into internal fun buildSignalLog(...), and widened SignalLogEntry from private to internal. The keys had no test coverage because the type was file-private, and a duplicate key here is a crash rather than a cosmetic glitch — this makes the invariant testable.

Testing Performed

New feature/node/src/commonTest/.../SignalLogKeyTest.kt (commonMain logic, so it runs on every KMP target):

• repeated packet ids still produce distinct keys — the shape that actually crashed
• a negative packet id is not conflated with its positive twin — packet ids are signed in the proto. Asserts the exact key with each id at index 0, so the index cannot be what separates them (strengthened after review feedback)
• local stats and packets sharing a timestamp do not collide — cross-type key safety
• entries are ordered newest first across both types — guards the ordering the extraction preserves

Full local baseline green: spotlessApply spotlessCheck detekt assembleDebug test allTests.

No UI appearance change — this only affects item identity, so there is nothing visual to capture.

Summary by CodeRabbit

• Bug Fixes

  • Improved signal history merging so packet receptions and local statistics are combined reliably.
  • Fixed potential duplicate/overlapping signal-log entries when packet identifiers repeat or share timestamps.
  • Ensured stable, newest-first ordering across both signal sources.

• Tests

  • Added signal-log key coverage for repeated identifiers, negative vs. positive IDs, timestamp collisions, and newest-first sorting across sources.

[coderabbitai]

📝 Walkthrough

Walkthrough

Signal metrics now use a shared helper to merge telemetry and packet samples, sort entries newest-first, and generate collision-free list keys. Tests cover repeated IDs, signed IDs, shared timestamps, and ordering.

Changes

Signal log handling

Layer / File(s)	Summary
Signal log construction and stable identity feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/SignalMetrics.kt	buildSignalLog combines and sorts telemetry and packet entries, while indexed packet keys prevent duplicate list identities. The composable now uses the helper.
Signal log validation feature/node/src/commonTest/kotlin/org/meshtastic/feature/node/metrics/SignalLogKeyTest.kt	Tests verify key uniqueness and newest-first ordering across packet and local-stat entries.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: making signal-log packet keys unique to avoid collisions.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

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
`@feature/node/src/commonTest/kotlin/org/meshtastic/feature/node/metrics/SignalLogKeyTest.kt`:
- Around line 39-46: Update the test `a negative packet id is not conflated with
its positive twin` so it verifies packet ID encoding independently of packet
index: build each signed-ID packet in a separate single-item `buildSignalLog`
call, or assert the exact expected signed keys. Ensure the assertion fails if
key generation ignores or normalizes the packet ID.

🪄 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: 8557c8e1-3ef5-4b44-9c51-4d7dfae6aa2b

📥 Commits

Reviewing files that changed from the base of the PR and between 92e3300 and 1193653.

📒 Files selected for processing (2)

• feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/SignalMetrics.kt
• feature/node/src/commonTest/kotlin/org/meshtastic/feature/node/metrics/SignalLogKeyTest.kt