Fix Analytics History crash from duplicate LazyColumn keys.#485
Conversation
Prefix single vs grouped history item keys so synced standalone session IDs cannot collide with grouped routineSessionId values when scrolling the History tab. Co-authored-by: Cursor <cursoragent@cursor.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a helper function historyItemLazyColumnKey to prefix LazyColumn keys by item type, preventing ID collisions between standalone sessions and grouped routine sessions. It also adds a corresponding unit test and updates HistoryTab to use this helper. The reviewer suggested refactoring this helper function into an extension property HistoryItem.lazyColumnKey to make the code more idiomatic in Kotlin, providing suggestions to update the implementation, imports, usage, and tests accordingly.
| items(filteredHistory.size, key = { index -> | ||
| when (val item = filteredHistory[index]) { | ||
| is com.devil.phoenixproject.presentation.manager.SingleSessionHistoryItem -> item.session.id | ||
| is com.devil.phoenixproject.presentation.manager.GroupedRoutineHistoryItem -> item.routineSessionId | ||
| } | ||
| historyItemLazyColumnKey(filteredHistory[index]) |
There was a problem hiding this comment.
Use the more idiomatic lazyColumnKey extension property on the HistoryItem instance.
| items(filteredHistory.size, key = { index -> | |
| when (val item = filteredHistory[index]) { | |
| is com.devil.phoenixproject.presentation.manager.SingleSessionHistoryItem -> item.session.id | |
| is com.devil.phoenixproject.presentation.manager.GroupedRoutineHistoryItem -> item.routineSessionId | |
| } | |
| historyItemLazyColumnKey(filteredHistory[index]) | |
| items(filteredHistory.size, key = { index -> | |
| filteredHistory[index].lazyColumnKey |
…tion/manager/HistoryManager.kt Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
…tion/screen/HistoryTab.kt Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
…sentation/manager/HistoryManagerTest.kt Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
9thLevelSoftware
deleted the
cursor/fix-analytics-history-duplicate-lazy-keys
branch
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4eb3b7b2c6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| is com.devil.phoenixproject.presentation.manager.SingleSessionHistoryItem -> item.session.id | ||
| is com.devil.phoenixproject.presentation.manager.GroupedRoutineHistoryItem -> item.routineSessionId | ||
| } | ||
| historyItemLazyColumnKey(filteredHistory[index]) |
There was a problem hiding this comment.
The commit adds and imports the HistoryItem.lazyColumnKey extension property, but this call invokes historyItemLazyColumnKey, which is not defined anywhere in the repo (rg only finds this call and the new property). Any build that compiles HistoryTab will fail with an unresolved reference; call filteredHistory[index].lazyColumnKey here or add the intended helper.
Useful? React with 👍 / 👎.
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| } | ||
|
|
||
| @Test | ||
| fun `history lazy column keys stay unique when standalone session id matches grouped routineSessionId`() = runTest { |
There was a problem hiding this comment.
🔥 The Roast: This test name is longer than my attention span after a double espresso. It's a mouthful that would make a German compound word blush.
🩹 The Fix: Consider shortening to something like lazyColumnKeysAreUniqueWhenIdsCollide - still descriptive but won't require a scroll bar to read.
📏 Severity: nitpick
| val keys = historyItems.map { it.lazyColumnKey } | ||
| val duplicateKeys = keys.groupingBy { it }.eachCount().filter { it.value > 1 } | ||
|
|
||
| assertEquals(emptyList(), duplicateKeys.keys.toList()) |
There was a problem hiding this comment.
🔥 The Roast: Wrapping emptyList() in assertEquals is like using a sledgehammer to crack a nut - technically correct but utterly overkill when assertTrue(duplicateKeys.isEmpty()) exists.
🩹 The Fix: Replace with assertTrue(duplicateKeys.isEmpty()) for clearer intent and less visual noise.
📏 Severity: suggestion
Code Review Roast 🔥Verdict: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)
🏆 Best part: The lazyColumnKey extension is a clean extraction that eliminates duplication - finally some sanity in this key generation logic. 💀 Worst part: That painfully long test name that commits terminally verbose syndrome. 📊 Overall: Like a well-meaning but overly verbose grandpa - the heart's in the right place, but could use some editing. Files Reviewed (3 files)
Reviewed by nemotron-3-super-120b-a12b-20230311:free · 75,155 tokens |
PR #485 refactored the History LazyColumn key into the extension property HistoryItem.lazyColumnKey and added the correct import, but left the call site calling a non-existent function historyItemLazyColumnKey(...). This never compiled in commonMain, breaking compileAndroidMain and compileKotlinIosArm64 — taking down all four release workflows (Android AAB, Android APK, iOS IPA, iOS TestFlight). Use the extension property at the call site. Verified with ./gradlew :shared:compileAndroidMain. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Prefix single vs grouped history item keys so synced standalone session IDs cannot collide with grouped routineSessionId values when scrolling the History tab.