[{"id":4376630837,"body":"๐Ÿ” **Coverage baseline may need a refresh.**\n\nThis PR touches `app/src/main/**`. If the changes affect what smoke covers, please refresh the baseline before merging:\n\n```bash\n./gradlew generateGmdCoverage\ngit add app/coverage-baselines/gmd_smoke.ec\ngit commit -m \"chore: refresh smoke coverage baseline\"\n```\n\n_Last checked: 2026-05-05 04:54 UTC ยท commit `083c11c`_\n\u003c!-- Sticky Pull Request Commentbaseline-reminder-bot --\u003e","html_url":"https://github.com/Mesya82/Service-Book/pull/55#issuecomment-4376630837","user":{"login":"github-actions[bot]","id":41898282,"profile_url":"https://github.com/apps/github-actions","avatar_url":"https://avatars.githubusercontent.com/in/15368?v=4"},"author_association":"NONE","reactions":{"total_count":0,"+1":0,"-1":0,"laugh":0,"confused":0,"heart":0,"hooray":0,"rocket":0,"eyes":0},"created_at":"2026-05-05T04:54:42Z","updated_at":"2026-05-05T04:54:42Z"},{"id":4376809245,"body":"## Code Review\n\n**Overview**\nImplements the 6-item action plan from #54: makes `DebugBillingOverride` nullable so R8 can strip it from release, surfaces `hasPendingPurchase` with a persistent dialog, makes `refresh()` reconnect when the client isn't ready, removes `simulatePurchaseCancel` and the stale TODO, and adds three billing tests + one VM test.\n\n### What looks good\n- `processPurchases()` rewrite correctly partitions purchases into purchased vs pending and only acknowledges PURCHASED ones โ€” matches Play Billing semantics.\n- `appGate.hasPendingPurchase` is wired symmetrically into both `VehicleListViewModel` and `SettingsViewModel` (state field + `launchIn`), consistent with the existing `isReadOnly`/`isPremium` handlers.\n- `FakeBillingManager` and `AppGateTest` are updated alongside the interface โ€” no stale fakes.\n- `setDebugOverride` and `debugBillingOverride.mode` now use safe-call, so release builds with `debugBillingOverride == null` are no-ops.\n\n### Issues\n\n**Blocker โ€” accidentally committed scratch file**\n`commit_msg.txt` at repo root is a draft of the commit message itself. Delete it before merging and consider adding `commit_msg*.txt` to `.gitignore`.\n\n**Medium โ€” `refresh()` reconnect path may not re-query purchases**\n```kotlin\noverride fun refresh() {\n if (!billingClient.isReady) {\n startConnection()\n } else {\n coroutineScope.launch { queryPurchases() }\n }\n}\n```\nCallers of `refresh()` expect fresh purchase state. `startConnection()` is async; unless its `onBillingSetupFinished` path chains into `queryPurchases()`, this branch returns without ever refreshing. Worth verifying that path actually queries โ€” if not, callers silently get stale data on the reconnect branch.\n\n**Medium โ€” pending dialog is unrecoverable if Play never resolves**\n`PurchasePendingDialog` has `onDismissRequest = {}` and no buttons. PENDING can sit for hours (bank transfer, parental approval) or transition to CANCELLED. If `onPurchasesUpdated` fires with `USER_CANCELED` for a previously pending purchase, it routes to the early-return branch (non-OK code), so `_hasPendingPurchase` stays `true` until the next cold start triggers `queryPurchases()`. Two suggestions:\n- Treat any non-OK update as a signal to clear pending state for that product.\n- Let `onDismissRequest` dismiss the dialog (keep the underlying flag, but let the user continue using read-only features).\n\n**Low โ€” R8 stripping isn't guaranteed by the nullable refactor alone**\n`open var debugBillingOverride: DebugBillingOverride? = null` keeps the type symbol reachable from the field declaration. R8 typically can strip the class when no instance is ever created (only `new` is inside `if (BuildConfig.DEBUG)`), but worth confirming by inspecting the release APK. The original action item said \"verify R8 strips it\" โ€” that verification step doesn't appear in the PR.\n\n**Low โ€” test name vs. coverage mismatch**\nThe PR description lists \"mid-call disconnects\" as one of the four new tests. `restorePurchases returns Error when queryPurchasesAsync returns SERVICE_DISCONNECTED` is a single-shot error-path test, not a mid-call disconnect (a connection dropping between connect and query). Consider adding a test where `onBillingServiceDisconnected` fires while a query is in flight.\n\n**Low โ€” empty product details branch**\n`launchPremiumPurchaseFlow returns BillingUnavailable when cachedProductDetails is null` stubs the query to return an empty list. Worth adding a complementary test where the query returns a non-OK response code (e.g., `SERVICE_UNAVAILABLE`).\n\n### Convention checks\n- `_state.value` reads inside `SettingsViewModel` โ€” convention preserved.\n- No new `_state` public setters; events still flow through `onEvent`.\n- Strings removed from `strings.xml` โ€” confirm no `values-xx/strings.xml` translations still reference `settings_simulate_purchase_cancel` / `settings_debug_purchase_cancel`.\n\n### Recommendation\n**Request changes**, primarily to drop `commit_msg.txt` and clarify the `refresh()` reconnect chain. The pending-dialog UX and R8 verification are worth addressing in this PR; the test naming/coverage points can be follow-ups.","html_url":"https://github.com/Mesya82/Service-Book/pull/55#issuecomment-4376809245","user":{"login":"Mesya82","id":32867735,"profile_url":"https://github.com/Mesya82","avatar_url":"https://avatars.githubusercontent.com/u/32867735?v=4"},"author_association":"OWNER","reactions":{"total_count":0,"+1":0,"-1":0,"laugh":0,"confused":0,"heart":0,"hooray":0,"rocket":0,"eyes":0},"created_at":"2026-05-05T05:46:15Z","updated_at":"2026-05-05T05:46:15Z"}]