Add synced navigation and card customization - #165
Open
Quick104 wants to merge 1 commit into
Open
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:
📝 WalkthroughWalkthroughThis PR adds revision-5 UI customization settings, client-family scopes, credential-owner-based account isolation, durable customization synchronization, mobile and TV navigation controls, card presentation settings, and reactive audiobook visibility. ChangesUI customization and identity integration
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant SettingsScreen
participant SettingsViewModel
participant UiCustomizationStore
participant SettingsRepository
participant SettingsApi
SettingsScreen->>SettingsViewModel: request customization mutation
SettingsViewModel->>UiCustomizationStore: validate and enqueue mutation
UiCustomizationStore->>SettingsRepository: write scoped setting or shortcut
SettingsRepository->>SettingsApi: send pinned identity and mutation ID
SettingsApi-->>UiCustomizationStore: return stored value or error
UiCustomizationStore-->>SettingsViewModel: publish effective customization state
SettingsViewModel-->>SettingsScreen: update UI with new state
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![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: 72bd9fa673
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (20)
android-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/settings/ServerSettingsFlusherTest.kt (1)
529-563: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRecord
authScopeinCallso tests can assert scope forwarding.The fake now accepts
authScope, butCalldiscards it. This PR binds durable outbox operations to credential ownership. Capturing the value lets a test assert that the flusher sends the owning scope, and that a stale scope is not reused after an identity transition.♻️ Proposed change to record the scope
data class Call( val kind: Kind, val key: String, val value: JsonElement?, val profileId: String?, val mutationId: String?, val scope: SettingScopeIdentity, + val authScope: AuthScopeSnapshot? = null, ) {- val call = Call(Call.Kind.PUT, key, value, profileId, mutationId, scope) + val call = Call(Call.Kind.PUT, key, value, profileId, mutationId, scope, authScope)- val call = Call(Call.Kind.DELETE, key, null, profileId, null, scope) + val call = Call(Call.Kind.DELETE, key, null, profileId, null, scope, authScope)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/settings/ServerSettingsFlusherTest.kt` around lines 529 - 563, Update the test fake’s Call recording flow used by putValue and deleteValue to retain each operation’s authScope, and extend the Call data structure accordingly so tests can assert forwarded credential ownership and verify stale scopes are not reused after identity changes.androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/shell/TvTopMenuBar.kt (1)
183-188: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
tabFocusRequestersgrows without bound and is mutated during composition.The map keeps an entry for every library destination ever present. Reorder and visibility edits add keys but never remove them. Lines 186-187 also mutate remembered state directly in the composition body.
The retained requesters are unused, so behavior stays correct. Consider deriving the map from
destinationswhile reusing existing requesters.♻️ Proposed derivation
- val tabFocusRequesters = remember { - mutableMapOf<TvRootDestination.LibraryType, FocusRequester>() - } - destinations.filterIsInstance<TvRootDestination.LibraryType>().forEach { destination -> - tabFocusRequesters.getOrPut(destination) { FocusRequester() } - } + val requesterCache = remember { + mutableMapOf<TvRootDestination.LibraryType, FocusRequester>() + } + val tabFocusRequesters = remember(destinations) { + val live = destinations.filterIsInstance<TvRootDestination.LibraryType>() + .associateWith { requesterCache.getOrPut(it) { FocusRequester() } } + requesterCache.keys.retainAll(live.keys) + live + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/shell/TvTopMenuBar.kt` around lines 183 - 188, Update the tabFocusRequesters logic in the TvTopMenuBar composition to derive entries only from the current LibraryType destinations, removing requesters for destinations no longer present while reusing existing requesters where possible. Avoid mutating the remembered map during composition; use a derived map or equivalent state transformation that preserves requester identity for retained destinations.androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/data/preferences/TvLibraryScopeStore.kt (1)
200-223: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider handling DataStore read failures in the suspend accessors.
storeFor(identity).data.first()propagatesIOExceptionwhen the preferences file is corrupt or unreadable.getSelectedLibraryId,getShowAudiobooksTab, andresolvedLibraryare called fromLaunchedEffectblocks inTvMainShell, where an exception cancels the effect and can surface as a crash. The reactive flow already defaults safely throughcatch; the one-shot reads do not.♻️ Suggested guard
private suspend fun getSelectedLibraryId( identity: StorageIdentity, type: TvLibraryTabType, - ): Int? = storeFor(identity).data.first()[scopeKey(identity.serverId, type)] + ): Int? = runCatching { + storeFor(identity).data.first()[scopeKey(identity.serverId, type)] + }.getOrNull() private suspend fun getShowAudiobooksTab(identity: StorageIdentity): Boolean = - storeFor(identity).data.first()[showAudiobooksKey(identity.serverId)] ?: false + runCatching { + storeFor(identity).data.first()[showAudiobooksKey(identity.serverId)] + }.getOrNull() ?: false🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/data/preferences/TvLibraryScopeStore.kt` around lines 200 - 223, Handle DataStore read failures in the one-shot accessors getSelectedLibraryId, getShowAudiobooksTab, and resolvedLibrary by catching IOException from data.first() and returning their existing safe fallback values. Preserve normal preference reads and the reactive flow’s current catch/default behavior, while ensuring LaunchedEffect callers are not cancelled by unreadable or corrupt preference data.androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/navigation/MobileMediaTabs.kt (1)
65-72: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
capabilitiesis now dead, and its producer performs a network call.
visibleMobileTabsno longer readscapabilities;projectedMobileTabssupplies the whole order. The parameter remains only to satisfy callers, and@Suppress("UNUSED_PARAMETER")hides that fact.The cost lands in
MainScreen. It still buildsmediaCapabilitiesthroughproduceStatewithpersonalDataRepository.listUserLibraries(), and it still keysrememberon that value at Lines 180-186. So each caller pays a server round trip and an extra recomputation for a value that no longer changes the result.If capability filtering is intentionally superseded by the server-authored menu, remove the parameter and its call sites.
♻️ Proposed change
fun visibleMobileTabs( - `@Suppress`("UNUSED_PARAMETER") capabilities: MediaModeCapabilities, showDownloads: Boolean, primaryMenu: PrimaryMenu? = null, ): List<Tab> = buildList { addAll(projectedMobileTabs(primaryMenu)) if (showDownloads) add(Tab.Downloads) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/navigation/MobileMediaTabs.kt` around lines 65 - 72, Remove the unused capabilities parameter from visibleMobileTabs and delete its `@Suppress` annotation. Update all callers, especially MainScreen, to stop producing mediaCapabilities via listUserLibraries(), remove the related remember key and recomputation, and invoke visibleMobileTabs using only the parameters it still consumes.android-shared/src/androidMain/kotlin/org/siloserver/silo/common/ui/components/CardPresentationLocals.kt (1)
9-9: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider
compositionLocalOffor this runtime-mutable value.
LocalCardPresentationchanges at runtime when the user edits presets or when the store hydrates.AppNavigationprovides it above the wholeNavHost, so each change invalidates the entire subtree instead of only the readers.staticCompositionLocalOfis intended for values that effectively never change.compositionLocalOftracks reads and limits invalidation to the composables that read the value.♻️ Proposed change
-import androidx.compose.runtime.staticCompositionLocalOf +import androidx.compose.runtime.compositionLocalOf import androidx.compose.ui.unit.Dp import org.siloserver.silo.model.settings.CardPresentation import org.siloserver.silo.model.settings.PosterSizePreset /** Current family-synchronized card presentation, with safe native defaults. */ -val LocalCardPresentation = staticCompositionLocalOf { CardPresentation.DEFAULT } +val LocalCardPresentation = compositionLocalOf { CardPresentation.DEFAULT }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android-shared/src/androidMain/kotlin/org/siloserver/silo/common/ui/components/CardPresentationLocals.kt` at line 9, Change LocalCardPresentation from staticCompositionLocalOf to compositionLocalOf so runtime preset and hydration updates invalidate only composables that read the local, while preserving CardPresentation.DEFAULT as the initial value.androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/navigation/AppNavigation.kt (1)
254-257: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRename
overlaySessionKeytohydrationIdentity.ProvideCardOverlays.sessionKeyacceptsAny?, so the current call compiles. The value identifies the server/profile hydration scope, not only card overlays.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/navigation/AppNavigation.kt` around lines 254 - 257, Rename the local variable overlaySessionKey to hydrationIdentity in the AppNavigation flow, and update its usage when passing the value to ProvideCardOverlays.sessionKey. Preserve the existing uiCustomizationHydrationIdentity arguments and behavior.androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/settings/TvSettingsViewModel.kt (1)
1171-1192: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
menuCandidatesduplicates the builtin availability rules fromstandardTvMenu.Both functions test the same library types for Movies, Series, Music, and Audiobooks. A new media type must be added in two places. Extract one helper that returns the available builtin items, then build the standard menu and the candidate list from it. Keep the Audiobooks difference explicit:
standardTvMenugates it onshowAudiobooks, andmenuCandidatesdoes not.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/settings/TvSettingsViewModel.kt` around lines 1171 - 1192, Extract the shared Movies, Series, Music, and Audiobooks builtin-selection logic from standardTvMenu and menuCandidates into one helper returning the available builtin items. Update both callers to build from that helper, while keeping the Audiobooks distinction explicit: standardTvMenu must still apply showAudiobooks, whereas menuCandidates must include Audiobooks based only on library availability.androidTvApp/src/androidUnitTest/kotlin/org/siloserver/silo/tv/ui/screens/settings/TvUiCustomizationCapabilityTest.kt (1)
103-120: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winState the missing capability flags explicitly.
These two cases omit
supportsIdempotentWritesandsupportsBatchedEffectiveand rely on their declared defaults beingfalse. If a default changes totrue, both assertions still pass while testing a different input. Pass every flag explicitly so the intent stays fixed.♻️ Proposed change
assertFalse( available( SettingsContractCapabilities( revision = 5, supportsBatchedEffective = true, + supportsIdempotentWrites = false, supportsAtomicShortcuts = true, ), ), ) assertFalse( available( SettingsContractCapabilities( revision = 5, + supportsBatchedEffective = false, supportsIdempotentWrites = true, supportsAtomicShortcuts = true, ), ), )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@androidTvApp/src/androidUnitTest/kotlin/org/siloserver/silo/tv/ui/screens/settings/TvUiCustomizationCapabilityTest.kt` around lines 103 - 120, Update the two SettingsContractCapabilities instances in the available tests to explicitly set the omitted capability flags: set supportsIdempotentWrites to false in the first case and supportsBatchedEffective to false in the second, while preserving the other flags and assertions.androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/settings/TvSettingsScreen.kt (1)
1049-1060: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace the duplicated
"builtin:home"literal with one shared constant. Three guards compare a codec identity against the hard-coded string"builtin:home". The identity format is produced byUiCustomizationCodec.identity, so a format change would break every guard silently and allow the Home item to be reordered or hidden.
androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/settings/TvSettingsScreen.kt#L1049-L1060: replace the literal at Line 1053, and the matching literal in the row-enable rule at Line 907, with the shared constant.androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/settings/TvSettingsViewModel.kt#L604-L608: declare the constant once, for exampleinternal val TvHomeMenuIdentity = UiCustomizationCodec.identity(PrimaryMenuItem.Builtin(PrimaryMenuBuiltin.HOME)), and use it in theremoveMenuItemguard.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/settings/TvSettingsScreen.kt` around lines 1049 - 1060, The hard-coded Home identity is duplicated across menu guards; define one shared identity constant and reuse it everywhere. In androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/settings/TvSettingsViewModel.kt:604-608, declare the constant from UiCustomizationCodec.identity(PrimaryMenuItem.Builtin(PrimaryMenuBuiltin.HOME)) and use it in removeMenuItem. In androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/settings/TvSettingsScreen.kt:1049-1060, replace the literals in the isHome check and row-enable rule with that constant.shared/src/commonMain/kotlin/org/siloserver/silo/network/api/SettingsApi.kt (1)
265-289: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueBuild the shortcut path from
SettingKeys.NAV_SHORTCUTS.The path hardcodes
nav.shortcuts.SettingKeys.NAV_SHORTCUTSholds the same string and is already the single source of truth for remote keys. Referencing the constant keeps the endpoint and the key list from drifting.♻️ Proposed refactor
- client.put("/api/v1/settings/values/nav.shortcuts/item") { + client.put("/api/v1/settings/values/${SettingKeys.NAV_SHORTCUTS}/item") {Add the import:
+import org.siloserver.silo.model.settings.SettingKeys🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@shared/src/commonMain/kotlin/org/siloserver/silo/network/api/SettingsApi.kt` around lines 265 - 289, Update putNavigationShortcutItem to build its endpoint path using SettingKeys.NAV_SHORTCUTS instead of the hardcoded “nav.shortcuts” key, adding the required import and preserving the existing request behavior.shared/src/commonTest/kotlin/org/siloserver/silo/model/settings/UiCustomizationTest.kt (1)
127-164: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider covering the collection-size caps and the builtin shortcut encode.
The bounds tests cover field types, blank strings, and the 128-character target-id cap. Three guards in the codec have no assertion: the 1..64 primary-menu item cap, the 256-item shortcut cap, and
encodeShortcutItemreturning null forPrimaryMenuItem.Builtin. A regression in any of those fails silently, because a rejected document degrades to the native default instead of raising.As per path instructions: "Add focused tests for shared logic only when behavior is critical or high risk".💚 Proposed additional test
`@Test` fun collectionSizeCapsAndBuiltinShortcutEncodingAreEnforced() { val home = """{"type":"builtin","destination":"home"}""" val overflow = (1..64).joinToString(",") { """{"type":"library","library_id":$it,"label":"L$it"}""" } assertNull(UiCustomizationCodec.parsePrimaryMenu(json("""{"items":[]}"""))) assertNull(UiCustomizationCodec.parsePrimaryMenu(json("""{"items":[$home,$overflow]}"""))) val shortcuts = (1..257).joinToString(",") { """{"type":"collection","collection_id":"c$it","label":"C$it"}""" } assertNull(UiCustomizationCodec.parseShortcuts(json("""{"items":[$shortcuts]}"""))) assertNull( UiCustomizationCodec.encodeShortcutItem( PrimaryMenuItem.Builtin(PrimaryMenuBuiltin.HOME), ), ) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@shared/src/commonTest/kotlin/org/siloserver/silo/model/settings/UiCustomizationTest.kt` around lines 127 - 164, Extend UiCustomizationTest with focused assertions for the untested codec guards: verify parsePrimaryMenu rejects empty items and more than 64 items, parseShortcuts rejects more than 256 items, and encodeShortcutItem returns null for PrimaryMenuItem.Builtin. Keep the tests centered on collection-size boundaries and builtin shortcut encoding, using the existing test helpers and symbols.Source: Path instructions
android-shared/src/androidMain/kotlin/org/siloserver/silo/common/settings/UiCustomizationStore.kt (2)
173-185: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the ownership guard into one named helper.
The same predicate appears four times in
refreshandrefreshBinding, and narrower two-term variants appear at lines 613, 688, 706, 731, and 741. Each site must combine the epoch check, theactiveIdentitycheck, andcurrentOwnerMatches. One omitted term reintroduces a cross-identity repaint. A single helper makes the invariant reviewable and keeps the sites consistent.♻️ Proposed helper
+ private suspend fun stillOwns(binding: RequestBinding, requestedEpoch: Long): Boolean = + synchronized(authoringStateLock) { requestedEpoch == authoringEpoch } && + activeIdentity == binding.identity && + currentOwnerMatches(binding)Then replace each occurrence:
- if (synchronized(authoringStateLock) { requestedEpoch != authoringEpoch } || - activeIdentity != binding.identity || !currentOwnerMatches(binding) - ) return@withLock + if (!stillOwns(binding, requestedEpoch)) return@withLockAlso applies to: 200-202, 223-225
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android-shared/src/androidMain/kotlin/org/siloserver/silo/common/settings/UiCustomizationStore.kt` around lines 173 - 185, Extract the repeated authoring ownership predicate into one named helper in UiCustomizationStore, combining the authoring-epoch check, activeIdentity check, and currentOwnerMatches(binding). Replace all four guards in refresh and refreshBinding, plus the narrower variants around the other listed sites, with this helper so every early-return and repaint path enforces all three conditions consistently.
607-616: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winCache the credential-owner digest instead of recomputing it per check.
currentOwnerMatchesruns once per loop iteration influshPending,flushPendingDeviceDeletes, and the unbounded drain loop influshPendingShortcutOps. Each call reads the token manager, andcredentialOwnerKeythen computes a SHA-256 and allocates oneString.formatresult per byte. On Android the token read hits EncryptedSharedPreferences. Keep the ownership re-read, because a mid-drain identity switch must be detected, but memoize the digest for a givencredentialOwnerId.♻️ Proposed change
+ private var cachedOwnerDigestInput: String? = null + private var cachedOwnerDigest: String? = null + + private fun ownerDigest(value: String): String = synchronized(generationLock) { + if (cachedOwnerDigestInput != value) { + cachedOwnerDigestInput = value + cachedOwnerDigest = sha256(value) + } + checkNotNull(cachedOwnerDigest) + }?: snapshot.credentialOwnerId ?.takeIf { it.isNotBlank() } - ?.let { "persistent:${sha256(it)}" } + ?.let { "persistent:${ownerDigest(it)}" }Also build the hex string with a
StringBuilderrather thanString.formatper byte:- .joinToString(separator = "") { byte -> "%02x".format(byte) } + .joinToString(separator = "") { byte -> (byte.toInt() and 0xff).toString(16).padStart(2, '0') }Also applies to: 681-691, 725-734, 797-812
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android-shared/src/androidMain/kotlin/org/siloserver/silo/common/settings/UiCustomizationStore.kt` around lines 607 - 616, Update credentialOwnerKey and the currentOwnerMatches call paths in flushPending, flushPendingDeviceDeletes, and flushPendingShortcutOps to memoize the computed digest per credentialOwnerId while still re-reading ownership on each check to detect identity changes. Replace per-byte String.format allocation with a StringBuilder-based hexadecimal conversion, reusing the cached digest for repeated checks during each drain operation.shared/src/commonTest/kotlin/org/siloserver/silo/network/SiloAuthPluginPinTest.kt (1)
243-265: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCapture all
X-Silo-Client-Familyvalues.
header(name, value)appends values, whilerequest.headers[name]returns only the first value. Capturerequest.headers.getAll("X-Silo-Client-Family")and assertlistOf("mobile"); otherwise an appended"tablet"value can go undetected.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@shared/src/commonTest/kotlin/org/siloserver/silo/network/SiloAuthPluginPinTest.kt` around lines 243 - 265, The test’s Captured helper must retain every X-Silo-Client-Family header value instead of reading only the first value. Update the request capture logic used by explicitClientFamilyHeaderIsNotOverwrittenByLiveDeviceMetadata to call getAll("X-Silo-Client-Family"), then assert the captured values equal listOf("mobile") so appended metadata such as "tablet" is detected.android-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/network/AndroidClientFamilyTest.kt (1)
8-19: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueCover the unknown-platform fallback.
When
platformis not"android-tv"andsmallestScreenWidthDpis below 600,androidClientFamilyreturns"mobile". Add an assertion for this case to document the fallback.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/network/AndroidClientFamilyTest.kt` around lines 8 - 19, Add an assertion in mapsTvPhoneAndTabletToCanonicalFamilies covering an unrecognized platform with smallestScreenWidthDp below 600, such as the expected "mobile" result from androidClientFamily. Keep the existing Android TV, mobile, and tablet assertions unchanged.androidApp/src/androidMain/kotlin/org/siloserver/silo/android/di/AndroidModule.kt (1)
191-202: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winBind and cancel the store scope.
DefaultUiCustomizationStorestarts a transition collector and an authoring-command loop. The inlineCoroutineScopehas no owner, so these jobs survive Koin shutdown or module reload. Provide a namedCoroutineScopebinding and cancel it withonClose, then inject it into the store.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@androidApp/src/androidMain/kotlin/org/siloserver/silo/android/di/AndroidModule.kt` around lines 191 - 202, Update the AndroidModule dependency setup around DefaultUiCustomizationStore to define a named CoroutineScope binding, register an onClose handler that cancels it, and inject that binding into the store instead of constructing an inline scope. Ensure the scope uses the existing SupervisorJob and Dispatchers.IO configuration.android-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/pairing/RegistryPairingAuthPortTest.kt (1)
81-81: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the new owner id exists before you compare it.
snapshotCurrentScope()?.credentialOwnerIdis nullable. If the snapshot returns null,assertNotEqualspasses and the test no longer proves that ownership rotated. UsecheckNotNull, as line 63 does.As per coding guidelines: "Use Kotlin test/JUnit for Android tests".💚 Proposed test fix
- assertNotEquals(oldOwner, tokens.snapshotCurrentScope()?.credentialOwnerId) + val newOwner = checkNotNull(tokens.snapshotCurrentScope()?.credentialOwnerId) + assertNotEquals(oldOwner, newOwner)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/pairing/RegistryPairingAuthPortTest.kt` at line 81, Update the ownership-rotation assertion in RegistryPairingAuthPortTest to unwrap the nullable credentialOwnerId with checkNotNull before comparing it to oldOwner, matching the existing pattern near line 63 and ensuring the test verifies a non-null new owner.Source: Coding guidelines
android-shared/src/androidMain/kotlin/org/siloserver/silo/common/pairing/PairingAuthPort.kt (1)
64-72: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueDocument that rollback cannot restore replaced credentials for the same server.
For a same-URL pairing, the commit stages the replacement tokens into the existing server slot before it publishes the registry state. If a later step in the commit throws,
switchTo(previousServerId)returns the user to that server, but its previous access and refresh tokens are already overwritten. The user must sign in again. State this limitation in the comment so the rollback contract is not read as a full restore.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android-shared/src/androidMain/kotlin/org/siloserver/silo/common/pairing/PairingAuthPort.kt` around lines 64 - 72, Update the rollback comment in the catch block around PairingAuthPort’s commit flow to explicitly state that same-server pairing may overwrite the existing access and refresh tokens before a later failure, so switching back cannot restore them and the user must sign in again. Keep the rollback behavior unchanged.shared/src/commonMain/kotlin/org/siloserver/silo/network/TokenManager.kt (1)
52-62: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueThe default returns a server URL where callers expect a server id.
The KDoc states "Returns the committed server id". When
getCurrentServerId()returns null, the default returnsserverUrl.TokenManagerImpl.getCurrentServerId()reads onlytemporaryScope?.serverId, so a single-scope commit always returns the URL. Any caller that later passes this value toswitchActiveServeror a registry lookup gets a value that matches no entry. Document the fallback explicitly, or returnnull-safe typed information instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@shared/src/commonMain/kotlin/org/siloserver/silo/network/TokenManager.kt` around lines 52 - 62, The fallback in commitPairedPersistentSession returns serverUrl even though callers interpret the result as a server id. Update commitPairedPersistentSession and its contract to avoid presenting the URL as an id: either document that fallback explicitly for callers or return nullable/typed information that distinguishes an absent server id from serverUrl, while preserving the committed-session behavior.shared/src/androidMain/kotlin/org/siloserver/silo/network/AndroidServerRegistry.kt (1)
185-214: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument that
stageCredentialsmust not suspend or re-enter the registry.The callback runs while this function holds
mutex. A callback that calls back into anyServerRegistrymethod deadlocks, becausemutexis not reentrant. The current caller satisfies this, but the KDoc does not state the constraint. Add it so future callers do not break the invariant.📝 Proposed KDoc addition
* Pairing-only combined registration seam. [stageCredentials] runs before * the new entry or active id is persisted/published, while the caller owns * the surrounding identity transition. Existing display-name overrides are * preserved, but profile selection is cleared for the replacement account. + * + * [stageCredentials] runs under this registry's non-reentrant lock. It must + * not call back into [ServerRegistry]. It must throw to abort the + * registration, in which case no entry and no active id are published. */🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@shared/src/androidMain/kotlin/org/siloserver/silo/network/AndroidServerRegistry.kt` around lines 185 - 214, Add KDoc for upsertPairedServerInsideIdentityTransition documenting that stageCredentials executes while mutex is held and therefore must not suspend or re-enter any ServerRegistry method. Keep the existing callback behavior unchanged and state the non-reentrancy constraint clearly for future callers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@android-shared/src/androidMain/kotlin/org/siloserver/silo/common/pairing/PairingAuthPort.kt`:
- Around line 47-48: Update commitPairedPersistentSession to run its
NonCancellable commitMutex.withLock block in withContext(Dispatchers.IO +
NonCancellable), and add the required Dispatchers import. Preserve the existing
locking and commit behavior while moving the synchronous SharedPreferences
commit off the caller’s dispatcher.
In
`@androidApp/src/androidMain/kotlin/org/siloserver/silo/android/di/AndroidModule.kt`:
- Around line 184-190: The Android DI binding in AndroidModule.kt must resolve
unknown wire values safely: replace the throwing SiloClientFamily.entries.first
lookup with a nullable lookup that falls back to SiloClientFamily.MOBILE. In
AndroidClientFamilyTest.kt, add coverage for an unexpected platform string and
verify its returned wire resolves to a SiloClientFamily member.
In
`@androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/navigation/MobileMediaTabs.kt`:
- Around line 42-50: Update projectedMobileTabs in
androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/navigation/MobileMediaTabs.kt
lines 42-50 to return standardMobileTabs when the projected result is empty, or
ensure Tab.Home is always included so the returned tab list is never empty. The
observable site in
androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/MainScreen.kt
lines 180-198 requires no direct change because the projection floor resolves
its behavior.
- Around line 107-111: Update MobileMediaTabs.kt lines 107-111 in hideMobileTab
to record the hidden aggregate separately instead of filtering Library, Section,
and Collection wire items, allowing showMobileTab to restore the original
authored pins; update lines 179-187 in rebuildMobileMenuForPreset to append
buckets omitted by the preset order and mark them hidden rather than discarding
them, preserving pins when switching presets.
In
`@androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/search/TvSearchViewModel.kt`:
- Around line 84-88: Update loadAvailableMediaTypes so a failed
listUserLibraries fetch retries or triggers a search-screen refresh, rather than
returning without updating visibility state. Ensure the resolved library
collection is assigned to resolvedLibraries and use resolvedLibraries throughout
the remaining method body. Preserve the Audiobooks visibility and filtering
behavior after a successful retry.
In
`@androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/settings/TvSettingsScreen.kt`:
- Around line 165-179: Bound the unresolved uiCustomizationSupport state so
tvGeneralInitialFocusTarget resolves to the non-supported/read-only focus target
after a short deadline instead of remaining DEFER indefinitely. Use that same
bounded target in both the LaunchedEffect detail-focus logic and SettingsRail,
allowing D-pad navigation and clearing detailFocusPending while preserving
deferred behavior during the initial window.
In
`@androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/settings/TvSettingsViewModel.kt`:
- Around line 52-53: Move the 64-item primary-menu and 256-item
navigation-shortcut limits from TvSettingsViewModel into shared code, then
update TvSettingsViewModel to reference the shared constants instead of defining
local values. Reuse the existing UiCustomizationCodec limit symbols where
available to keep both modules aligned.
- Around line 630-646: Update the transform and write flow around
prepareTvMenuItemAddition so updatePrimaryMenuAndShortcut is called only when
that function returns a changed PrimaryMenu; when it returns null, skip the
shortcut update entirely. Preserve the existing updatePrimaryMenu behavior for
builtin items and avoid persisting present = true without a corresponding menu
entry.
In
`@androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/shell/TvMainShell.kt`:
- Around line 346-362: Update the stale-identity cleanup guard in the
LaunchedEffect keyed by selectedLibraryDestinationIdentity and related state to
also require customization resolution, using the existing
customization-readiness state. Keep the identity intact until the primary menu
and uiCustomizationSupported values have emitted, while preserving the current
cleanup behavior afterward.
In
`@androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/shell/TvMediaDestinations.kt`:
- Around line 36-62: Update the customized-menu branch in the root-destination
builder around PrimaryMenuItem handling so it returns the projected custom roots
only when that projection is non-empty; otherwise continue into the existing
legacy construction path. Ensure menus containing only unsupported items or
unresolved/non-TV-renderable libraries still produce the legacy Home entry and a
focusable bar.
In
`@androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/shell/TvTopMenuBar.kt`:
- Around line 212-233: Update selectedEntryRequester() and the menu-group
focusProperties.enter fallback to resolve the selected root only from the
currently composed destinations collection. If Home, ForYou, or Calendar is
omitted from the customized primaryMenu (or otherwise has no matching
destination), return the always-composed searchFocusRequester instead of an
unattached requester; preserve valid requester selection for available
destinations.
- Around line 639-644: Add the Compose Foundation ExperimentalFoundationApi
opt-in to the declaration containing bringIntoViewRequester and its related
Modifier.bringIntoViewRequester usage, while preserving the existing
ExperimentalTvMaterial3Api opt-in and behavior.
In
`@shared/src/androidMain/kotlin/org/siloserver/silo/network/EncryptedTokenManagerImpl.kt`:
- Around line 179-184: In the method containing the SIGN_IN transition, validate
and capture the registry as AndroidServerRegistry before calling
identityTransitions.changing(IdentityTransitionKind.SIGN_IN). Return or
otherwise structure the flow so the existing transition and remaining body
execute only after this precondition succeeds, then close the added block while
leaving the body unchanged.
In `@shared/src/commonMain/kotlin/org/siloserver/silo/network/TokenManager.kt`:
- Around line 31-38: Update the KDoc contract near the persistent-session
installation method to separate “rotate durable ownership” and “end any
temporary overlay” with the required punctuation and line breaks, using the
exact wording provided while leaving the surrounding contract unchanged.
---
Nitpick comments:
In
`@android-shared/src/androidMain/kotlin/org/siloserver/silo/common/pairing/PairingAuthPort.kt`:
- Around line 64-72: Update the rollback comment in the catch block around
PairingAuthPort’s commit flow to explicitly state that same-server pairing may
overwrite the existing access and refresh tokens before a later failure, so
switching back cannot restore them and the user must sign in again. Keep the
rollback behavior unchanged.
In
`@android-shared/src/androidMain/kotlin/org/siloserver/silo/common/settings/UiCustomizationStore.kt`:
- Around line 173-185: Extract the repeated authoring ownership predicate into
one named helper in UiCustomizationStore, combining the authoring-epoch check,
activeIdentity check, and currentOwnerMatches(binding). Replace all four guards
in refresh and refreshBinding, plus the narrower variants around the other
listed sites, with this helper so every early-return and repaint path enforces
all three conditions consistently.
- Around line 607-616: Update credentialOwnerKey and the currentOwnerMatches
call paths in flushPending, flushPendingDeviceDeletes, and
flushPendingShortcutOps to memoize the computed digest per credentialOwnerId
while still re-reading ownership on each check to detect identity changes.
Replace per-byte String.format allocation with a StringBuilder-based hexadecimal
conversion, reusing the cached digest for repeated checks during each drain
operation.
In
`@android-shared/src/androidMain/kotlin/org/siloserver/silo/common/ui/components/CardPresentationLocals.kt`:
- Line 9: Change LocalCardPresentation from staticCompositionLocalOf to
compositionLocalOf so runtime preset and hydration updates invalidate only
composables that read the local, while preserving CardPresentation.DEFAULT as
the initial value.
In
`@android-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/network/AndroidClientFamilyTest.kt`:
- Around line 8-19: Add an assertion in mapsTvPhoneAndTabletToCanonicalFamilies
covering an unrecognized platform with smallestScreenWidthDp below 600, such as
the expected "mobile" result from androidClientFamily. Keep the existing Android
TV, mobile, and tablet assertions unchanged.
In
`@android-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/pairing/RegistryPairingAuthPortTest.kt`:
- Line 81: Update the ownership-rotation assertion in
RegistryPairingAuthPortTest to unwrap the nullable credentialOwnerId with
checkNotNull before comparing it to oldOwner, matching the existing pattern near
line 63 and ensuring the test verifies a non-null new owner.
In
`@android-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/settings/ServerSettingsFlusherTest.kt`:
- Around line 529-563: Update the test fake’s Call recording flow used by
putValue and deleteValue to retain each operation’s authScope, and extend the
Call data structure accordingly so tests can assert forwarded credential
ownership and verify stale scopes are not reused after identity changes.
In
`@androidApp/src/androidMain/kotlin/org/siloserver/silo/android/di/AndroidModule.kt`:
- Around line 191-202: Update the AndroidModule dependency setup around
DefaultUiCustomizationStore to define a named CoroutineScope binding, register
an onClose handler that cancels it, and inject that binding into the store
instead of constructing an inline scope. Ensure the scope uses the existing
SupervisorJob and Dispatchers.IO configuration.
In
`@androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/navigation/AppNavigation.kt`:
- Around line 254-257: Rename the local variable overlaySessionKey to
hydrationIdentity in the AppNavigation flow, and update its usage when passing
the value to ProvideCardOverlays.sessionKey. Preserve the existing
uiCustomizationHydrationIdentity arguments and behavior.
In
`@androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/navigation/MobileMediaTabs.kt`:
- Around line 65-72: Remove the unused capabilities parameter from
visibleMobileTabs and delete its `@Suppress` annotation. Update all callers,
especially MainScreen, to stop producing mediaCapabilities via
listUserLibraries(), remove the related remember key and recomputation, and
invoke visibleMobileTabs using only the parameters it still consumes.
In
`@androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/data/preferences/TvLibraryScopeStore.kt`:
- Around line 200-223: Handle DataStore read failures in the one-shot accessors
getSelectedLibraryId, getShowAudiobooksTab, and resolvedLibrary by catching
IOException from data.first() and returning their existing safe fallback values.
Preserve normal preference reads and the reactive flow’s current catch/default
behavior, while ensuring LaunchedEffect callers are not cancelled by unreadable
or corrupt preference data.
In
`@androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/settings/TvSettingsScreen.kt`:
- Around line 1049-1060: The hard-coded Home identity is duplicated across menu
guards; define one shared identity constant and reuse it everywhere. In
androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/settings/TvSettingsViewModel.kt:604-608,
declare the constant from
UiCustomizationCodec.identity(PrimaryMenuItem.Builtin(PrimaryMenuBuiltin.HOME))
and use it in removeMenuItem. In
androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/settings/TvSettingsScreen.kt:1049-1060,
replace the literals in the isHome check and row-enable rule with that constant.
In
`@androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/settings/TvSettingsViewModel.kt`:
- Around line 1171-1192: Extract the shared Movies, Series, Music, and
Audiobooks builtin-selection logic from standardTvMenu and menuCandidates into
one helper returning the available builtin items. Update both callers to build
from that helper, while keeping the Audiobooks distinction explicit:
standardTvMenu must still apply showAudiobooks, whereas menuCandidates must
include Audiobooks based only on library availability.
In
`@androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/shell/TvTopMenuBar.kt`:
- Around line 183-188: Update the tabFocusRequesters logic in the TvTopMenuBar
composition to derive entries only from the current LibraryType destinations,
removing requesters for destinations no longer present while reusing existing
requesters where possible. Avoid mutating the remembered map during composition;
use a derived map or equivalent state transformation that preserves requester
identity for retained destinations.
In
`@androidTvApp/src/androidUnitTest/kotlin/org/siloserver/silo/tv/ui/screens/settings/TvUiCustomizationCapabilityTest.kt`:
- Around line 103-120: Update the two SettingsContractCapabilities instances in
the available tests to explicitly set the omitted capability flags: set
supportsIdempotentWrites to false in the first case and supportsBatchedEffective
to false in the second, while preserving the other flags and assertions.
In
`@shared/src/androidMain/kotlin/org/siloserver/silo/network/AndroidServerRegistry.kt`:
- Around line 185-214: Add KDoc for upsertPairedServerInsideIdentityTransition
documenting that stageCredentials executes while mutex is held and therefore
must not suspend or re-enter any ServerRegistry method. Keep the existing
callback behavior unchanged and state the non-reentrancy constraint clearly for
future callers.
In `@shared/src/commonMain/kotlin/org/siloserver/silo/network/api/SettingsApi.kt`:
- Around line 265-289: Update putNavigationShortcutItem to build its endpoint
path using SettingKeys.NAV_SHORTCUTS instead of the hardcoded “nav.shortcuts”
key, adding the required import and preserving the existing request behavior.
In `@shared/src/commonMain/kotlin/org/siloserver/silo/network/TokenManager.kt`:
- Around line 52-62: The fallback in commitPairedPersistentSession returns
serverUrl even though callers interpret the result as a server id. Update
commitPairedPersistentSession and its contract to avoid presenting the URL as an
id: either document that fallback explicitly for callers or return
nullable/typed information that distinguishes an absent server id from
serverUrl, while preserving the committed-session behavior.
In
`@shared/src/commonTest/kotlin/org/siloserver/silo/model/settings/UiCustomizationTest.kt`:
- Around line 127-164: Extend UiCustomizationTest with focused assertions for
the untested codec guards: verify parsePrimaryMenu rejects empty items and more
than 64 items, parseShortcuts rejects more than 256 items, and
encodeShortcutItem returns null for PrimaryMenuItem.Builtin. Keep the tests
centered on collection-size boundaries and builtin shortcut encoding, using the
existing test helpers and symbols.
In
`@shared/src/commonTest/kotlin/org/siloserver/silo/network/SiloAuthPluginPinTest.kt`:
- Around line 243-265: The test’s Captured helper must retain every
X-Silo-Client-Family header value instead of reading only the first value.
Update the request capture logic used by
explicitClientFamilyHeaderIsNotOverwrittenByLiveDeviceMetadata to call
getAll("X-Silo-Client-Family"), then assert the captured values equal
listOf("mobile") so appended metadata such as "tablet" is detected.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 60b63226-d039-4914-919c-d2f187bc1f9b
📒 Files selected for processing (78)
android-shared/src/androidMain/kotlin/org/siloserver/silo/common/network/AndroidDeviceMetadataProvider.ktandroid-shared/src/androidMain/kotlin/org/siloserver/silo/common/pairing/PairingAuthPort.ktandroid-shared/src/androidMain/kotlin/org/siloserver/silo/common/settings/UiCustomizationStore.ktandroid-shared/src/androidMain/kotlin/org/siloserver/silo/common/ui/components/CardPresentationLocals.ktandroid-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/network/AndroidClientFamilyTest.ktandroid-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/pairing/RegistryPairingAuthPortTest.ktandroid-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/settings/AndroidPlayerSettingsStoreTest.ktandroid-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/settings/OverlayPrefsStoreTest.ktandroid-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/settings/ServerSettingsFlusherTest.ktandroid-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/settings/UiCustomizationStoreTest.ktandroid-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/ui/components/CardPresentationSizingTest.ktandroidApp/src/androidMain/kotlin/org/siloserver/silo/android/di/AndroidModule.ktandroidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/components/MediaCard.ktandroidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/navigation/AppNavigation.ktandroidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/navigation/MobileMediaTabs.ktandroidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/MainScreen.ktandroidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/browse/CatalogGrid.ktandroidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/collections/CollectionDetailScreen.ktandroidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/collections/LibraryCollectionsScreen.ktandroidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/libraries/LibrariesScreen.ktandroidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/people/PersonDetailScreen.ktandroidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/personal/PersonalMediaGridContent.ktandroidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/search/SearchResults.ktandroidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/settings/PlaybackSettings.ktandroidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/settings/SettingsScreen.ktandroidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/screens/settings/SettingsViewModel.ktandroidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/ui/navigation/MobileMediaTabsTest.ktandroidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/ui/navigation/UiCustomizationHydrationIdentityTest.ktandroidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/ui/screens/settings/UiCustomizationCapabilityTest.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/data/preferences/TvLibraryScopeStore.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/di/AndroidTvModule.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/components/TvCatalogGrid.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/components/TvMediaCard.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/auth/TvLoginViewModel.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/library/TvLibraryDetailScreen.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/people/TvPersonDetailViewModel.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/search/TvSearchViewModel.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/settings/TvSettingsScreen.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/settings/TvSettingsViewModel.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/settings/TvVisiblePrimaryMenu.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/shell/TvMainShell.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/shell/TvMediaDestinations.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/shell/TvRootDestination.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/shell/TvTopMenuBar.ktandroidTvApp/src/androidUnitTest/kotlin/org/siloserver/silo/tv/data/preferences/TvLibraryScopeStoreTest.ktandroidTvApp/src/androidUnitTest/kotlin/org/siloserver/silo/tv/ui/screens/detail/TvItemDetailSubtitlePreferenceTest.ktandroidTvApp/src/androidUnitTest/kotlin/org/siloserver/silo/tv/ui/screens/people/TvPersonDetailViewModelTest.ktandroidTvApp/src/androidUnitTest/kotlin/org/siloserver/silo/tv/ui/screens/settings/TvCustomizationLimitsTest.ktandroidTvApp/src/androidUnitTest/kotlin/org/siloserver/silo/tv/ui/screens/settings/TvUiCustomizationCapabilityTest.ktandroidTvApp/src/androidUnitTest/kotlin/org/siloserver/silo/tv/ui/shell/TvMediaDestinationsTest.ktandroidTvApp/src/androidUnitTest/kotlin/org/siloserver/silo/tv/ui/shell/TvUiCustomizationLifecycleTest.ktshared/src/androidMain/kotlin/org/siloserver/silo/network/AndroidServerRegistry.ktshared/src/androidMain/kotlin/org/siloserver/silo/network/EncryptedTokenManagerImpl.ktshared/src/androidUnitTest/kotlin/org/siloserver/silo/model/settings/SettingsConformanceTest.ktshared/src/androidUnitTest/kotlin/org/siloserver/silo/network/EncryptedTokenManagerScopeGenerationTest.ktshared/src/commonMain/kotlin/org/siloserver/silo/domain/settings/ProfileSettingsController.ktshared/src/commonMain/kotlin/org/siloserver/silo/model/settings/SettingKeys.ktshared/src/commonMain/kotlin/org/siloserver/silo/model/settings/SettingValueModels.ktshared/src/commonMain/kotlin/org/siloserver/silo/model/settings/UiCustomization.ktshared/src/commonMain/kotlin/org/siloserver/silo/network/AuthInterceptorImpl.ktshared/src/commonMain/kotlin/org/siloserver/silo/network/AuthScopeSnapshot.ktshared/src/commonMain/kotlin/org/siloserver/silo/network/DeviceMetadataProvider.ktshared/src/commonMain/kotlin/org/siloserver/silo/network/IdentityTransitionBarrier.ktshared/src/commonMain/kotlin/org/siloserver/silo/network/TokenManager.ktshared/src/commonMain/kotlin/org/siloserver/silo/network/TokenManagerImpl.ktshared/src/commonMain/kotlin/org/siloserver/silo/network/api/SettingsApi.ktshared/src/commonMain/kotlin/org/siloserver/silo/repository/AuthRepository.ktshared/src/commonMain/kotlin/org/siloserver/silo/repository/SettingsRepository.ktshared/src/commonTest/kotlin/org/siloserver/silo/domain/settings/ProfileSettingsControllerTest.ktshared/src/commonTest/kotlin/org/siloserver/silo/model/settings/SettingsResolve.ktshared/src/commonTest/kotlin/org/siloserver/silo/model/settings/UiCustomizationTest.ktshared/src/commonTest/kotlin/org/siloserver/silo/network/IdentityTransitionBarrierTest.ktshared/src/commonTest/kotlin/org/siloserver/silo/network/SiloAuthPluginPinTest.ktshared/src/commonTest/kotlin/org/siloserver/silo/network/api/SettingsApiValuesTest.ktshared/src/commonTest/kotlin/org/siloserver/silo/repository/SettingsRepositoryShortcutTest.ktshared/src/commonTest/resources/settings/v1/SOURCEshared/src/commonTest/resources/settings/v1/conformance.jsonshared/src/commonTest/resources/settings/v1/manifest.json
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
androidTvApp/src/androidUnitTest/kotlin/org/siloserver/silo/tv/ui/screens/people/TvPersonDetailViewModelTest.kt (1)
172-182: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRequire a successful reload after hiding audiobooks.
If the unfiltered reload fails,
TvPersonDetailViewModel.loadItems(reset = true)clearsitemsand setsisLoadingItemsto false while storing the failure inpagingError(androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/people/TvPersonDetailViewModel.kt, Lines 164-187). The predicate in Lines 172-176 can then pass without loading any result. Also requirepagingError == nulland assert an expected movie item.Proposed fix
awaitState(viewModel) { !it.isLoadingItems && + it.pagingError == null && TvPersonMediaFilter.Audiobooks !in it.availableFilters && it.selectedFilter == TvPersonMediaFilter.All && - it.items.none { item -> item.type == "audiobook" } + it.items.none { item -> item.type == "audiobook" } && + it.items.any { item -> item.type == "movie" } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@androidTvApp/src/androidUnitTest/kotlin/org/siloserver/silo/tv/ui/screens/people/TvPersonDetailViewModelTest.kt` around lines 172 - 182, Strengthen the post-hide reload assertion in TvPersonDetailViewModelTest by requiring pagingError == null and verifying that items contains an expected movie item, while retaining the existing loading, filter, and audiobook-exclusion checks. This ensures the TvPersonDetailViewModel.loadItems(reset = true) reload succeeded rather than merely finishing with cleared items.
🧹 Nitpick comments (1)
androidTvApp/src/androidUnitTest/kotlin/org/siloserver/silo/tv/ui/screens/people/TvPersonDetailViewModelTest.kt (1)
142-183: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winMake the request recorder safe for concurrent probes.
TvPersonDetailViewModel.probeFiltersWithContent()starts several requests withasyncandawaitAll(androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/people/TvPersonDetailViewModel.kt, Lines 104-128). This test writes to and readsqueriesthrough a plainMutableList. If MockEngine dispatches handlers concurrently, these accesses can race. UseCopyOnWriteArrayListor synchronize writes and snapshots.Proposed fix
+import java.util.concurrent.CopyOnWriteArrayList - val queries = mutableListOf<Map<String, String?>>() + val queries = CopyOnWriteArrayList<Map<String, String?>>()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@androidTvApp/src/androidUnitTest/kotlin/org/siloserver/silo/tv/ui/screens/people/TvPersonDetailViewModelTest.kt` around lines 142 - 183, Make the queries recorder in syncedAudiobookVisibilityUpdatesFiltersAndLoadedResults thread-safe because probeFiltersWithContent issues concurrent requests. Replace the plain mutable list with CopyOnWriteArrayList, or synchronize both request-handler writes and reads/snapshots, while preserving the existing query assertions and ordering-independent checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/search/TvSearchViewModel.kt`:
- Around line 85-97: Rename the Kotlin retry properties LibraryLoadMaxAttempts
and LibraryLoadRetryDelayMillis to camelCase names, then update every reference
in loadAvailableMediaTypes, including the retry loop bound and delay call.
In
`@androidTvApp/src/androidUnitTest/kotlin/org/siloserver/silo/tv/ui/screens/people/TvPersonDetailViewModelTest.kt`:
- Around line 102-140: The test cleanup in
slowLibraryLookupCannotExposeAudiobooksBeforeHiddenPreferenceIsObserved should
wait until MockEngine request processing has fully completed before closing
personalDataClient. Add a request-completion signal after the handler returns
its response, await that signal in the finally block after releasing
releaseLibraryResponse, then close the client.
---
Outside diff comments:
In
`@androidTvApp/src/androidUnitTest/kotlin/org/siloserver/silo/tv/ui/screens/people/TvPersonDetailViewModelTest.kt`:
- Around line 172-182: Strengthen the post-hide reload assertion in
TvPersonDetailViewModelTest by requiring pagingError == null and verifying that
items contains an expected movie item, while retaining the existing loading,
filter, and audiobook-exclusion checks. This ensures the
TvPersonDetailViewModel.loadItems(reset = true) reload succeeded rather than
merely finishing with cleared items.
---
Nitpick comments:
In
`@androidTvApp/src/androidUnitTest/kotlin/org/siloserver/silo/tv/ui/screens/people/TvPersonDetailViewModelTest.kt`:
- Around line 142-183: Make the queries recorder in
syncedAudiobookVisibilityUpdatesFiltersAndLoadedResults thread-safe because
probeFiltersWithContent issues concurrent requests. Replace the plain mutable
list with CopyOnWriteArrayList, or synchronize both request-handler writes and
reads/snapshots, while preserving the existing query assertions and
ordering-independent checks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0903b238-96b8-4930-8845-5dd9d67ed1d3
📒 Files selected for processing (22)
android-shared/src/androidMain/kotlin/org/siloserver/silo/common/pairing/PairingAuthPort.ktandroid-shared/src/androidMain/kotlin/org/siloserver/silo/common/settings/UiCustomizationStore.ktandroid-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/network/AndroidClientFamilyTest.ktandroid-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/settings/UiCustomizationStoreTest.ktandroidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/navigation/MobileMediaTabs.ktandroidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/ui/navigation/MobileMediaTabsTest.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/people/TvPersonDetailViewModel.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/search/TvSearchViewModel.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/settings/TvSettingsScreen.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/settings/TvSettingsViewModel.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/shell/TvMainShell.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/shell/TvMediaDestinations.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/shell/TvTopMenuBar.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/shell/TvTopMenuFocusRequest.ktandroidTvApp/src/androidUnitTest/kotlin/org/siloserver/silo/tv/ui/screens/people/TvPersonDetailViewModelTest.ktandroidTvApp/src/androidUnitTest/kotlin/org/siloserver/silo/tv/ui/screens/settings/TvCustomizationLimitsTest.ktandroidTvApp/src/androidUnitTest/kotlin/org/siloserver/silo/tv/ui/shell/TvMediaDestinationsTest.ktandroidTvApp/src/androidUnitTest/kotlin/org/siloserver/silo/tv/ui/shell/TvTopMenuFocusRequestTest.ktshared/src/androidMain/kotlin/org/siloserver/silo/network/EncryptedTokenManagerImpl.ktshared/src/androidUnitTest/kotlin/org/siloserver/silo/network/EncryptedTokenManagerScopeGenerationTest.ktshared/src/commonMain/kotlin/org/siloserver/silo/model/settings/UiCustomization.ktshared/src/commonMain/kotlin/org/siloserver/silo/network/TokenManager.kt
🚧 Files skipped from review as they are similar to previous changes (17)
- android-shared/src/androidMain/kotlin/org/siloserver/silo/common/pairing/PairingAuthPort.kt
- android-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/network/AndroidClientFamilyTest.kt
- shared/src/commonMain/kotlin/org/siloserver/silo/network/TokenManager.kt
- androidTvApp/src/androidUnitTest/kotlin/org/siloserver/silo/tv/ui/screens/settings/TvCustomizationLimitsTest.kt
- androidTvApp/src/androidUnitTest/kotlin/org/siloserver/silo/tv/ui/shell/TvMediaDestinationsTest.kt
- androidApp/src/androidMain/kotlin/org/siloserver/silo/android/ui/navigation/MobileMediaTabs.kt
- shared/src/commonMain/kotlin/org/siloserver/silo/model/settings/UiCustomization.kt
- shared/src/androidMain/kotlin/org/siloserver/silo/network/EncryptedTokenManagerImpl.kt
- shared/src/androidUnitTest/kotlin/org/siloserver/silo/network/EncryptedTokenManagerScopeGenerationTest.kt
- androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/shell/TvMainShell.kt
- android-shared/src/androidMain/kotlin/org/siloserver/silo/common/settings/UiCustomizationStore.kt
- androidApp/src/androidUnitTest/kotlin/org/siloserver/silo/android/ui/navigation/MobileMediaTabsTest.kt
- androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/settings/TvSettingsScreen.kt
- androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/people/TvPersonDetailViewModel.kt
- android-shared/src/androidUnitTest/kotlin/org/siloserver/silo/common/settings/UiCustomizationStoreTest.kt
- androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/settings/TvSettingsViewModel.kt
- androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/shell/TvMediaDestinations.kt
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a37c0132ed
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 76f4b9a53a
ℹ️ 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".
This was referenced Aug 6, 2026
Quick104
added a commit
that referenced
this pull request
Aug 18, 2026
Rebases PR #165 onto current main (180 commits of drift, 47 of the PR's 84 files also changed upstream) and resolves the review findings that rebase surfaced. Rebase: - Squash-applied onto main rather than replaying six commits. TvTopMenuBar and TvMainShell were reset to main and the customization work re-authored onto main's post-#228 structure, so the sign-in flow, For You with its Watchlist/Favorites dropdown, the player HUD, subtitle handling and the whole top-menu focus-handoff contract are preserved unchanged. - Settings contract taken from main at manifest revision 7. The PR's revision-5 fixtures and SettingKeys are dropped as stale; revisions 6 and 7 touched only playback keys, so the nav./ui.card_* surface this feature targets is byte-identical from 5 through 7. - The PR's replacePersistentSession / commitPairedPersistentSession identity API is dropped in favour of main's replaceAccountSession, which solves the same atomicity problem. The durable credentialOwnerId is kept and rewired onto main's commit points, since main's identityGeneration and credentialEpoch are process-scoped and cannot own a cache across restarts. - LocalCardPresentation moved to TvAppNavigation, so card presentation also reaches item detail, library-collection detail and person detail. Fixes: - Hiding a mobile tab no longer deletes library, section or collection pins from the shared profile_client document. Hide now removes only builtins, and Libraries refuses to hide while pins exist, so the loss can no longer propagate to iPhone and web clients on the same profile. - A preset that omits a bucket still holding pins keeps that bucket's builtins, which would otherwise be unrecoverable from the Android editor. - TvLibraryScopeStore imports the pre-namespace DataStore file, so existing TV installs keep their Show Audiobooks preference and per-type library scope selections across the upgrade. - A definitively rejected shortcut operation now rolls back only itself and the outbox keeps draining, instead of wedging at the head and permanently suppressing nav.shortcuts reconciliation. - Phone settings explain that customization needs a newer server instead of rendering nothing, the TV menu editor filters ebook library pins, selecting a pinned library before the library list resolves keeps its identity, and MainActivity and the DI provider read the same Configuration. Adds coverage for the credential-owner lifecycle: rotation inside the account replacement transaction, clearing on both sign-out paths, lazy backfill for pre-existing installs, stability across process restart, guest isolation, and the full DID_CHANGE payload TvLibraryScopeStore consumes. Full unit matrix and both debug APK assemblies pass. Not yet exercised on a device: D-pad traversal of the scrolling top menu, and two-device sync. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Quick104
force-pushed
the
codex/ui-customization-android
branch
from
August 18, 2026 14:47
40c472d to
f387cf9
Compare
Rebases PR #165 onto current main (180 commits of drift, 47 of the PR's 84 files also changed upstream) and resolves the review findings that rebase surfaced. Rebase: - Squash-applied onto main rather than replaying six commits. TvTopMenuBar and TvMainShell were reset to main and the customization work re-authored onto main's post-#228 structure, so the sign-in flow, For You with its Watchlist/Favorites dropdown, the player HUD, subtitle handling and the whole top-menu focus-handoff contract are preserved unchanged. - Settings contract taken from main at manifest revision 7. The PR's revision-5 fixtures and SettingKeys are dropped as stale; revisions 6 and 7 touched only playback keys, so the nav./ui.card_* surface this feature targets is byte-identical from 5 through 7. - The PR's replacePersistentSession / commitPairedPersistentSession identity API is dropped in favour of main's replaceAccountSession, which solves the same atomicity problem. The durable credentialOwnerId is kept and rewired onto main's commit points, since main's identityGeneration and credentialEpoch are process-scoped and cannot own a cache across restarts. - LocalCardPresentation moved to TvAppNavigation, so card presentation also reaches item detail, library-collection detail and person detail. Fixes: - Hiding a mobile tab no longer deletes library, section or collection pins from the shared profile_client document. Hide now removes only builtins, and Libraries refuses to hide while pins exist, so the loss can no longer propagate to iPhone and web clients on the same profile. - A preset that omits a bucket still holding pins keeps that bucket's builtins, which would otherwise be unrecoverable from the Android editor. - TvLibraryScopeStore imports the pre-namespace DataStore file, so existing TV installs keep their Show Audiobooks preference and per-type library scope selections across the upgrade. - A definitively rejected shortcut operation now rolls back only itself and the outbox keeps draining, instead of wedging at the head and permanently suppressing nav.shortcuts reconciliation. - Phone settings explain that customization needs a newer server instead of rendering nothing, the TV menu editor filters ebook library pins, selecting a pinned library before the library list resolves keeps its identity, and MainActivity and the DI provider read the same Configuration. Adds coverage for the credential-owner lifecycle: rotation inside the account replacement transaction, clearing on both sign-out paths, lazy backfill for pre-existing installs, stability across process restart, guest isolation, and the full DID_CHANGE payload TvLibraryScopeStore consumes. Full unit matrix and both debug APK assemblies pass. Not yet exercised on a device: D-pad traversal of the scrolling top menu, and two-device sync. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Quick104
force-pushed
the
codex/ui-customization-android
branch
from
August 18, 2026 14:50
f387cf9 to
dfbc3a5
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dfbc3a5cef
ℹ️ 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".
| destinations = visibleRoots, | ||
| ) | ||
| } | ||
| val uiCustomizationResolved = uiCustomizationSupported != null || primaryMenu != null |
There was a problem hiding this comment.
On a first launch or replacement account with no cached menu, a restored main/audiobooks route can be redirected to Home as soon as libraries and the legacy flag resolve, while the revision-5 primary-menu request is still pending and visibleRoots therefore uses the legacy menu with Audiobooks hidden. The redirect at line 1010 does not consult this gate, and this definition would not be sufficient anyway because capability support becomes non-null before the effective menu GET completes; track actual menu hydration (including an authoritative null) and wait for it before rejecting the route, otherwise the later authored Audiobooks menu cannot restore the user to the route.
Useful? React with 👍 / 👎.
Comment on lines
+73
to
+75
| TvSettingsViewModel.NavigationPreset.MEDIA_FIRST -> | ||
| TvNavigationPresetMutation.SetPrimaryMenu( | ||
| mediaFirstTvMenu(libraries, showAudiobooks), |
There was a problem hiding this comment.
When the synced TV-family menu contains a section or collection placement authored by another client, selecting Media First enters this branch and replaces the complete document with a newly generated builtin-only menu; Minimal does the same below. visibleTvMenuItems deliberately hides those unsupported placements from this editor, so the user receives no indication that the preset will permanently delete them and cannot recreate them on Android TV. Apply the preset to the editor-visible projection while weaving hidden section/collection entries back into the stored document.
Useful? React with 👍 / 👎.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Part of Silo-Server/silo-server#376.
Android phone and TV had fixed navigation/card presentation and could not synchronize these preferences across like devices.
Depends on Silo-Server/silo-server#538.
Approach
Validation
5e185e6e592613fb84529e192ce13c7551374ff1.git diff --checkpasses.APK assembly is not emulator or physical Android TV D-pad proof; no compatible AVD/device runtime session was available for this PR.
AI Disclosure
Checklist