Fix CursorWindow crash on large Steam libraries - #961
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughReplaced the single-query Room-backed getAllOwnedApps Flow with a count-driven reactive pipeline and transactional paged loading; added adaptive page-size retry on SQLiteBlobTooBigException and centralized the owned-app WHERE clause into OWNED_APPS_WHERE. (≤50 words) Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant DAO as DAO:getAllOwnedApps
participant Flow as FlowPipeline
participant DB as Database
Caller->>DAO: subscribe/getAllOwnedApps()
DAO->>Flow: start observing (_observeOwnedAppCount)
Flow->>DB: SELECT COUNT(*) WHERE OWNED_APPS_WHERE
DB-->>Flow: emit count
Flow->>Flow: distinctUntilChanged()
alt count changed
Flow->>DB: fetch page 0 (LIMIT/OFFSET)
DB-->>Flow: return page data
loop fetch remaining pages
Flow->>DB: fetch next page (LIMIT/OFFSET)
DB-->>Flow: return page data
end
else SQLiteBlobTooBigException during page fetch
Flow->>Flow: halve pageSize (>=1) and retry
Flow->>DB: fetch page with smaller LIMIT
DB-->>Flow: return page data or error
end
Flow-->>Caller: emit accumulated List<SteamApp>
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/src/main/java/app/gamenative/db/dao/SteamAppDao.kt">
<violation number="1" location="app/src/main/java/app/gamenative/db/dao/SteamAppDao.kt:53">
P1: Offset pagination with non-unique sort key causes unstable ordering</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/main/java/app/gamenative/db/dao/SteamAppDao.kt (1)
82-90: Correct implementation with documented trade-off.The reactive pipeline correctly uses
flatMapLatestto cancel stale reloads during rapid PICS inserts. ThedistinctUntilChanged()on count is a documented trade-off that aligns withLibraryViewModel's existing behavior (which already only re-filters on size changes per the relevant snippet at lines 116-128).One minor observation: the inner flow construction could alternatively use
flow { emit(...) }from the existing import scope rather than the fully qualifiedkotlinx.coroutines.flow.flow:🔧 Optional simplification
- .flatMapLatest { // cancel stale reloads during rapid PICS inserts - kotlinx.coroutines.flow.flow { emit(_getAllOwnedAppsPaged(invalidPkgId)) } - } + .flatMapLatest { // cancel stale reloads during rapid PICS inserts + flow { emit(_getAllOwnedAppsPaged(invalidPkgId)) } + }Add to imports:
import kotlinx.coroutines.flow.flow🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/db/dao/SteamAppDao.kt` around lines 82 - 90, The code uses a fully qualified call kotlinx.coroutines.flow.flow inside getAllOwnedApps to emit the paged results; replace it with the unqualified flow by adding import kotlinx.coroutines.flow.flow and changing the call to flow { emit(_getAllOwnedAppsPaged(invalidPkgId)) } in the getAllOwnedApps pipeline (referencing getAllOwnedApps and _getAllOwnedAppsPaged) to simplify the code and use the existing import scope.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/src/main/java/app/gamenative/db/dao/SteamAppDao.kt`:
- Around line 82-90: The code uses a fully qualified call
kotlinx.coroutines.flow.flow inside getAllOwnedApps to emit the paged results;
replace it with the unqualified flow by adding import
kotlinx.coroutines.flow.flow and changing the call to flow {
emit(_getAllOwnedAppsPaged(invalidPkgId)) } in the getAllOwnedApps pipeline
(referencing getAllOwnedApps and _getAllOwnedAppsPaged) to simplify the code and
use the existing import scope.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: adf51f8f-33f1-46ba-b56c-7218e018d940
📒 Files selected for processing (1)
app/src/main/java/app/gamenative/db/dao/SteamAppDao.kt
|
The query change in #945 (adding |
6f01948 to
7c2937a
Compare
|
Nice, need to review |
Here's the report: https://discord.com/channels/1378308569287622737/1482852295245959219/1484895355630911559 |
7c2937a to
2d598d0
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/app/gamenative/db/dao/SteamAppDao.kt`:
- Around line 12-14: The use of flatMapLatest in getAllOwnedApps is experimental
and requires opt-in; add the `@OptIn`(ExperimentalCoroutinesApi::class) annotation
either at the top of the file (file-level) or directly on the getAllOwnedApps
function to satisfy the compiler, and ensure
kotlinx.coroutines.ExperimentalCoroutinesApi is imported if you add the
annotation; update SteamAppDao.kt accordingly so flatMapLatest usage is properly
opted-in.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4b7f275f-73d3-4861-bb1d-d53248153eac
📒 Files selected for processing (1)
app/src/main/java/app/gamenative/db/dao/SteamAppDao.kt
2d598d0 to
bb3a7a8
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/app/gamenative/db/dao/SteamAppDao.kt`:
- Around line 44-50: _replace the COUNT(*) invalidation key with a lightweight
mutation-sensitive projection:_ change the query used by _observeOwnedAppCount
(and the similar query at lines 84-93) to select a small sentinel projection of
the library rows (e.g. app primary key and/or a last_modified/updated_at or
rowid) instead of COUNT(*), and update the return type from Flow<Int> to a Flow
of that projection (e.g. Flow<List<Int>> or Flow<List<IdAndTs>>), so
getAllOwnedApps() sees ordering/content/name/icon/PICS changes and will
invalidate correctly via distinctUntilChanged().
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 26457d6e-d952-438a-ae7d-7783bcfb4b4d
📒 Files selected for processing (1)
app/src/main/java/app/gamenative/db/dao/SteamAppDao.kt
bb3a7a8 to
a90a579
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/app/gamenative/db/dao/SteamAppDao.kt`:
- Around line 60-70: The current loop lowers the shared pageSize when catching
SQLiteBlobTooBigException, which then throttles all subsequent pages; instead
scope the reduced size to the current fetch only by introducing a
temporary/currentPageSize passed to _getOwnedAppsPage (or reset pageSize back to
Int.MAX_VALUE after a successful fetch), so that when a page overflow occurs you
retry the same offset with smaller sizes (PAGE_SIZE, pageSize/2,...), but once a
page succeeds you restore the default/full pageSize for the next offset; update
usages of pageSize and the catch path around _getOwnedAppsPage, referencing
variables pageSize, offset, PAGE_SIZE and the exception
SQLiteBlobTooBigException to locate and implement the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f24c11b4-96df-4620-ba53-d666f47c9950
📒 Files selected for processing (1)
app/src/main/java/app/gamenative/db/dao/SteamAppDao.kt
a90a579 to
b6a2b62
Compare
getAllOwnedApps loaded all rows in one cursor, which can overflow the CursorWindow when rows contain large blobs. Try loading all at once; on SQLiteBlobTooBigException, retry with progressively smaller pages (50 → 25 → ... → 1). Rethrows if a single row exceeds the window. Flow reactivity preserved via COUNT(*) observer that triggers a reload on every table change.
b6a2b62 to
af8849b
Compare
3 participants
Summary
SQLiteBlobTooBigException, retry with progressively smaller pages (50 → 25 → ... → 1)COUNT(*)observer withdistinctUntilChanged+flatMapLatestTest plan
Fixes #960
Summary by cubic
Prevents crashes on large Steam libraries by paging the owned-apps query and handling
SQLiteBlobTooBigException. Small libraries still load in one shot; the app list reloads only when the owned-app count changes. Fixes #960.CursorWindow.COUNT(*)changes withdistinctUntilChanged+flatMapLatestto cancel stale reloads.@Transactionand sort byLOWER(name), idwithLIMIT/OFFSETfor stable, consistent results.Written for commit af8849b. Summary will update on new commits.
Summary by CodeRabbit
Performance Improvements
Stability