fix: use Android-owned Configuration.uiMode for SYSTEM dark mode (#677) - #691
Conversation
In SYSTEM mode, Theme.kt delegated the dark/light decision to the transient Compose isSystemInDarkTheme() signal with no app-owned reconciliation. A stale or false signal deterministically selected the light dynamic/static branch, producing light surfaces even when Android night mode was active. This fix introduces rememberPlatformSystemDark() — an Android-owned lifecycle-safe system appearance source that: - Seeds from Configuration.uiMode (the OS-owned source of truth) - Refreshes on ON_RESUME so lock/unlock and configuration changes are captured - Logs Compose-vs-Configuration mismatches for diagnostics - Follows the existing LifecycleEventObserver pattern from App.kt PR #678's manifest behavior (no uiMode in configChanges) is preserved. Permission-gated roots (BlePermissionHandler, OptionalPermissionsHandler) have their own isSystemInDarkTheme() calls and are outside this fix scope. Acceptance criteria: - SYSTEM + dynamic colors renders dark when Android night mode is dark - DARK/LIGHT remain stable across foreground/resume - Material You dark clamp contract preserved - Diagnostic mismatch logging active Fixes #677
![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: cc1d4f94de
ℹ️ 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".
| val composeSignal = try { | ||
| // Evaluate the Compose system-dark signal outside of composition | ||
| // so we can compare it against our authoritative source. | ||
| (context.resources.configuration.uiMode and Configuration.UI_MODE_NIGHT_MASK) == | ||
| Configuration.UI_MODE_NIGHT_YES |
There was a problem hiding this comment.
When the sporadic signal divergence recurs, this diagnostic can never report it: composeSignal rereads the same context.resources.configuration.uiMode expression used by readUiModeDark(), immediately after refreshed was obtained, rather than evaluating isSystemInDarkTheme(). The values are therefore necessarily identical unless the configuration changes between the two reads, defeating the mismatch telemetry this change explicitly introduces; capture the Compose signal during composition and compare that value, or remove the ineffective diagnostic.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in commit 705d0d0: the composeSignal now captures isSystemInDarkTheme() during composition (the Compose ambient) rather than re-reading Configuration.uiMode in the observer. The mismatch detector now actually works — if Compose and Configuration disagree on resume, a warning is logged.
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| isDark = refreshed | ||
|
|
||
| // Diagnostic: log mismatch between Android-owned value and Compose signal | ||
| val composeSignal = try { |
There was a problem hiding this comment.
🔥 The Roast: This composeSignal is the cockroach motel of telemetry — looks alive, does nothing. You literally read Configuration.uiMode & UI_MODE_NIGHT_MASK == UI_MODE_NIGHT_YES on line 53, which is byte-for-byte the same expression as readUiModeDark() already evaluated into refreshed on line 43. Then on line 58 you compare composeSignal != refreshed. That inequality is permanently false. The PR description proudly advertises "logs Compose-vs-Configuration mismatches" as a key piece of the fix — this logs them straight into /dev/null. The try { ... } catch (_: Throwable) is the cherry on top: it wraps a bitwise AND on a non-null Int that cannot throw, defending against ghosts.
🩹 The Fix: Delete lines 49–63 entirely (the composeSignal block and the surrounding mismatch log). Also delete the unused import androidx.compose.foundation.isSystemInDarkTheme on line 4. The refreshed != isDark log on line 44 already covers the operationally meaningful case (actual uiMode flip captured on resume). If you genuinely want a Compose-vs-Configuration drift detector, capture the Compose signal during composition (val composeSignal = isSystemInDarkTheme() in the composable body) and compare it against refreshed in the observer — but only do this if you have a concrete failure mode that needs it, not because the PR description promised it.
📏 Severity: critical
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
There was a problem hiding this comment.
Fixed in commit 705d0d0: replaced the dead composeSignal block with a val composeSignal = isSystemInDarkTheme() captured during composition. The observer now compares this Compose ambient against the refreshed Configuration.uiMode value. The try-catch is removed since isSystemInDarkTheme() is a pure read. The isSystemInDarkTheme import is now actually used.
Code Review Roast 🔥Verdict: No Issues Found | Recommendation: Merge Oh look, the followup actually finishes the job. I had my "stale closure on first composition" bit loaded and they went and bridged it with 🏆 Best part: 💀 Worst part: The mismatch diagnostic on line 58 is now technically correct (compares Compose's 📊 Overall: Like the rare third act that doesn't undo the good will of the second — Ponytail: Lean already. Ship. Files Reviewed (2 files, incremental)
🤖 Generated with Kilo Code Previous Review Summaries (2 snapshots, latest commit 705d0d0)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 705d0d0)Verdict: No Issues Found | Recommendation: Merge Oh wait, the fix actually fixes the previous critical issue. I need to sit down. I had my whole "tautology in a trench coat" bit prepared and they went and made 🏆 Best part: Closing over 💀 Worst part: Honestly nothing. The comment on lines 53–56 now correctly describes what the code does ( 📊 Overall: Like the sequel that actually listens to the critics — same bones, fixed the scene everyone walked out of the theater for. Ponytail: Lean already. Ship. Files Reviewed (1 file, incremental)
🤖 Generated with Kilo Code Previous review (commit cc1d4f9)Verdict: Request changes | Recommendation: Fix the dead diagnostic before merge Overview
Issue Details (click to expand)
VerdictRequest changes. The lifecycle reconciliation itself ( Correctness / Safety Findings
Scope Concern (Not Blocking, But Worth Naming)The PR description explicitly scopes out Ponytail Review
The Ponytail net: -16 lines. Suggested Minimal PatchSingle-file deletion in
That's it. No other files need to change. Final Merge GuidanceDo not merge until the dead mismatch diagnostic is removed or made real. The actual bug fix (lifecycle-safe 🏆 Best part: The 💀 Worst part: A diagnostic whose only two values are bitwise-equal, shipped with a test name that asserts it's there, and described in the PR body as the headline telemetry. It's the "we have logs at home" of telemetry. 📊 Overall: A correct fix wearing a costume of a feature it doesn't have. Strip the costume and it's shippable. Files Reviewed (5 files)
Reviewed by minimax-m3 · Input: 37K · Output: 5.9K · Cached: 133.8K Review guidance: REVIEW.md from base branch |
The previous composeSignal re-read Configuration.uiMode (same expression as readUiModeDark()), so the mismatch comparison was permanently false. Now capture isSystemInDarkTheme() during composition and compare against the refreshed Configuration value on ON_RESUME. This makes the drift detector actually functional — if Compose and Configuration disagree, it logs a warning instead of silently succeeding. Addresses review comments from Codex (P2) and Kilo (critical) on PR #691.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 705d0d07e6
ℹ️ 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".
Summary
Fixes the sporadic light-mode surface appearance when Android night mode is active and the user has selected
ThemeMode.SYSTEM. The root cause (per GPT-5.6 Terra RCA) is thatTheme.ktdelegated the SYSTEM-mode dark/light decision to the transient ComposeisSystemInDarkTheme()signal with no app-owned lifecycle reconciliation. A stale or false Compose signal deterministically selected the light dynamic/static branch.Root Cause
In
Theme.kt:110,ThemeMode.SYSTEM -> isSystemInDarkTheme()depends on a transient Compose signal. Phoenix had:Configuration.uiModestatePR #678 correctly removed
uiModefromconfigChangesin the manifest, but the underlying signal boundary was never addressed.Fix
Introduces
rememberPlatformSystemDark()— an Android-owned lifecycle-safe system appearance source:PlatformSystemDark.kt(commonMain) —expectdeclaration for the lifecycle-safe platform dark signalPlatformSystemDark.android.kt(androidMain) — ReadsConfiguration.uiMode & UI_MODE_NIGHT_MASK, registersLifecycleEventObserverto refresh onON_RESUME, logs Compose-vs-Configuration mismatchesPlatformSystemDark.ios.kt(iosMain) — Delegates toisSystemInDarkTheme()(iOS does not have the transient-signal issue)Theme.kt—ThemeMode.SYSTEM -> rememberPlatformSystemDark()replacesisSystemInDarkTheme()ThemeModeUiContractGuardTest.kt— Updated contract test to assert the new lifecycle-safe sourceNon-Goals (per RCA)
BlePermissionHandler,OptionalPermissionsHandler) have their ownisSystemInDarkTheme()calls — outside this fix scopeAcceptance Criteria
RCA Contract
gpt56_terra_rcaFixes #677