You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Bug: Theme "System" never persists — nil optional is omitted, so the route never clears the stored theme #79
Same class as #46 (a PATCH /api/user/update body that silently does nothing), found while
reviewing light/dark/system theme support.
Symptom
Settings → Appearance → Theme → System does not stick. Pick Dark, then pick System: the app
stays dark, and the picker snaps back to Dark the next time the sheet syncs from the user.
updateUserSettings puts that in a synthesized Encodable struct, and Swift encodes optionals with encodeIfPresent — so a nil field is omitted, not sent as null. Verified:
system-pick body: {}
The route applies ...(theme !== undefined && { theme }) (app/api/user/update/route.ts:102,157),
so an absent key is a no-op. The response then returns the unchanged user (theme: "dark"), authState.updateUser stores it, and syncFromUser resets the picker to Dark.
The comment justifying it is wrong
SettingsView.swift:85-86 says:
"system" means no explicit preference — send nil so the server clears it.
Sending the string "system" is rejected or treated as default (light) by the server.
The route performs no validation on theme — it stores whatever string arrives. And the web
stores the literal "system": ThemeProvider defaults to theme = 'system' and writes localStorage.setItem('theme', themeValue) unconditionally
(components/layout/ThemeProvider.tsx:11,17). "system" is the web's own persisted value.
Fix
Send "system" verbatim; delete the nil mapping and the incorrect comment.
Unit test asserting the body is {"theme":"system"} — it fails today, where the body is {}.
While here: RootView.preferredScheme already treats any non-light/dark value (including "system") as "follow the OS", so no change is needed there.
Acceptance criteria
Dark → System persists across a GET /api/user refresh and app relaunch.
The value round-trips with the web's Settings page.
Implemented in #97. Closing here — further work tracks in the PR.
Correction to this issue's own plan
The issue (and my instruction to the implementer) asked for a unit test that fails against the old
code. That turns out to be impossible at the API layer, and the reason is worth recording:
APIClient.updateUserSettings(theme: "system")already encoded {"theme":"system"} before the
fix. The nil was produced at the call site — inside a .onChange closure calling a private func save on a SwiftUI view, which XCTest cannot reach. So the regression test added in #97 pins the
wire format but would not have caught the original bug.
Extracting a testable seam for the mapping was considered and rejected: post-fix the mapping is newValue passed straight through, so the seam would be an identity function guarding nothing.
The diagnosis was instead confirmed by encoding the real Body struct standalone — legacy call-site
expression → {}, fixed → {"theme":"system"} — which matches this issue's analysis exactly.
The broader lesson is already captured in /ios-review from #64: the payload-key/nil-omission class
is caught by reading the route and the encoder, not by a mock-session test.
Backend re-confirmed
app/api/user/update/route.ts:24,102,157 destructures theme and stores it unvalidated; components/layout/ThemeProvider.tsx:11,17 persists the literal "system". The comment this PR
deletes was simply false.
Activity
Adron commentedon Sep 16, 2026
Implemented in #97. Closing here — further work tracks in the PR.
Correction to this issue's own plan
The issue (and my instruction to the implementer) asked for a unit test that fails against the old
code. That turns out to be impossible at the API layer, and the reason is worth recording:
APIClient.updateUserSettings(theme: "system")already encoded{"theme":"system"}before thefix. The
nilwas produced at the call site — inside a.onChangeclosure calling aprivate func saveon a SwiftUI view, which XCTest cannot reach. So the regression test added in #97 pins thewire format but would not have caught the original bug.
Extracting a testable seam for the mapping was considered and rejected: post-fix the mapping is
newValuepassed straight through, so the seam would be an identity function guarding nothing.The diagnosis was instead confirmed by encoding the real
Bodystruct standalone — legacy call-siteexpression →
{}, fixed →{"theme":"system"}— which matches this issue's analysis exactly.The broader lesson is already captured in
/ios-reviewfrom #64: the payload-key/nil-omission classis caught by reading the route and the encoder, not by a mock-session test.
Backend re-confirmed
app/api/user/update/route.ts:24,102,157destructuresthemeand stores it unvalidated;components/layout/ThemeProvider.tsx:11,17persists the literal"system". The comment this PRdeletes was simply false.