Fix TV startup navigation and complete title pages #39

Merged

justinzeus merged 7 commits from fm/tv-walk-fix1 into main 2026-11-06 13:16:43 +01:00

Conversation 1 Commits 7 Files changed 18 +829 -33

justinzeus commented 2026-11-06 11:30:25 +01:00

Owner

Copy link

Outcome

Deliver the corrected TV walk build B6b. AI-assisted (fork-internal).

Final pushed head: 6e0f9952cf13225635a70767812c03674de5a652.
Four final debug APKs plus verified SHA256SUMS are in /root/.treehouse/firstmate-7bab20/2/firstmate/data/tv-build-b/apk-b6b/.

Findings, causes and regressions

Finding	Root cause / change	Regression
F1: first-entry Movies/Shows retain Home	Cached NavDisplay entries retained the temporary launch ShellState, even after household data was ready. Entries now consume current shell/Discover composition locals; only the current entry may set the shared route or enable the outer host Back handler.	FirstSignInNavigationTest.firstContinueThenMoviesRendersMindelsGridWithoutReenteringThePicker — RED before the cached-entry correction; GREEN after. Fresh host, immediate signed-in Mindel replies, one Continue, rendered Movies (6), Shows (29), then Home.
F2: picker skeleton outlives usable identity	The cached entry retained its skeleton; live-shell publication also formerly waited for saved-login I/O. Publish the current Jellyfin person before I/O, and let cached entries adopt that shell.	First-entry regression above; ShellStartupTimingTest.firstFramePickerAndEveryTabWorkBeforeSavedPeopleArrive keeps saved-user and real-wire activity reads pending while using the picker/tabs.
F3: held-film cast/related missing	Missing ProviderIds prevented catalogue enrichment. Explicitly read missing ProviderIds/People without changing Jellyfin playback identity.	TitleReadFacadeTest.heldMovieResolvesCatalogueIdentityWhenTheDetailOmitsProviderIds
F3: Discover details have no rows	Details opened a header-only preview route. Navigate to the shared title page and remove the obsolete parallel detail read.	DiscoverTitleTimingTest.discoverDetailsFillCastAndRelatedAfterAsyncReplyAndBackRestoresDiscover
F4: scrolled rows bleed into nav	The scrolled brush still faded to transparent. Draw the whole scrolled band opaque; leave the unscrolled treatment/geometry unchanged.	NavBandOcclusionTest.scrolledRowsNeverShowThroughTheNavBand
F6: empty Anime audience creates an empty tab	is_anime identifies a grouping, not owned-title counts. Before enabling Anime, issue a one-item Jellyfin existence read using the same user/library/series criteria as the Anime grid. No owners/library names or count fields are invented. Empty Home Anime shelves were already omitted and remain covered.	AnimeAvailabilityTest.configuredAnimeAudienceNeedsActualTitlesBeforeItCreatesATabOrHomeShelf — signed-in Shared/Mindel, zero and nonzero titles, owner/user audience membership, empty Home shelf exclusion.
F6 safety: tab removal leaves invalid focus	Removing Anime while the avatar was focused retained the old index 5 for a new 0..4 nav. Remap focus to the avatar or a surviving destination.	Same Anime regression: RED at expected index 4 / actual 5; GREEN for Anime appearing and disappearing.

Active-root review correction (B6b)

The review found that removing Anime while it was the current root left route/rootRoute and the actual singleton navigation entry pointing to Anime. Back then emitted Pop forever against that singleton.

ShellState.updatePeople now moves a removed root to Home and emits ShellAction.Destination("home", preservePushed = true). The real integration replaces the singleton root normally; when pages are pushed, it changes only entry zero, retaining the current title/person pages and order. Avatar and surviving-tab focus remapping remain unchanged.

Regression: AnimeRootRemovalTest.removedAnimeRootReturnsHomeWithoutDiscardingPushedPagesOrAvatarFocus. RED on the reviewed head: expected [Home(id=0)], observed [Wolfskin(route=anime)]. GREEN after the correction for all four scenarios: active root, avatar-focused root, pushed title, pushed title plus person. Back enters Home's nav ladder or pops only the current pushed page.

Touched-area validation passed: 16 tests across AnimeRootRemovalTest, AnimeAvailabilityTest, FirstSignInNavigationTest, ShellStartupTimingTest, DiscoverTitleTimingTest, ShellKeysTest, and ShellSnapshotReadTest. No full suite was run for this review cycle. The optional nested-handler note is bounded explicitly in the documentation; no broader handler propagation or player change was added.

Verification

• ONE final-head make apk-debug passed on 6e0f9952cf13225635a70767812c03674de5a652, including the packaged branding check for all four APKs. All four delivered B6b SHA256 checksums verified.
• F6/focus changes passed targeted Anime availability, fresh-sign-in rendering, async person-switch/startup, audience recovery and snapshot-read tests. The Anime test uses the existing signed-in audience wire shape and derives its library names from that response, not hardcoded owner/library guesses.
• Firstmate's real Xiaomi B5 walk confirmed F1 fixed for Shared/Justin/Mindel and accepted F2 (fully drawn picker about eight seconds after sign-in). Earlier B4 walks confirmed cast/related rows on held and not-held title pages; nav bleed was not seen again.
• Full-suite history: FirstSignInNavigationTest.firstContinueThenMoviesRendersMindelsGridWithoutReenteringThePicker previously timed out on Shows only in full runs: 671/672 app tests passed, while its targeted run passed. The one authorized isolation correction added Compose idle waits, a 15-second wait on the actual Shows grid, and @After restoration of endpoint/fixture/screen preference globals. The subsequent authorized serial make test passed on 5a846660, before F6 was added. No full-suite/native retry loop was run after the F6 instruction; final F6 changes were verified with targeted regressions.
• Earlier native Robolectric java/nio/ShortBuffer SIGABRT runs were avoided by serial Gradle workers/test forks. Overrides were external to the repository; no tracked build settings changed.
• SnapshotFlow blocks/getter dependencies were re-grepped, and snapshot-read smoke passed; no new state writes were introduced into read-only getter blocks.
• Current main and the already-merged snapshot-read hotfix are included. Documentation is updated; the worktree is clean.

Evidence, including the original first-entry RED output, historical 671/672 logs/XML, the passing isolation-cycle suite, native title captures and final APK output, is under /root/.treehouse/firstmate-7bab20/2/firstmate/data/tv-walk-fix1/.

Device-unverified and risk

F5 video-player Back remains device-unverified because playback was not allowed on the box. Existing playback handoff/Back regression remains; upstream player code still uses NavigationManager.goBack and was not modified. Corrected shell ownership closes the real session picker, and outgoing outer-host Back handlers are disabled. Nested title handlers retain their existing lifecycle-based behaviour (pre-existing review note); no universal nested-handler or real-player claim is made.

The F6 existence read runs after the live picker is published. A failed availability read retains the signed-in user's last confirmed value; a successful empty read hides Anime. F6's final device re-walk remains with firstmate. No ADB, release/signing, version edits, service mutations or authority widening. This request is ready for review; nothing was merged.

## Outcome Deliver the corrected TV walk build B6b. AI-assisted (fork-internal). Final pushed head: `6e0f9952cf13225635a70767812c03674de5a652`. Four final debug APKs plus verified `SHA256SUMS` are in `/root/.treehouse/firstmate-7bab20/2/firstmate/data/tv-build-b/apk-b6b/`. ## Findings, causes and regressions | Finding | Root cause / change | Regression | |---|---|---| | F1: first-entry Movies/Shows retain Home | Cached NavDisplay entries retained the temporary launch ShellState, even after household data was ready. Entries now consume current shell/Discover composition locals; only the current entry may set the shared route or enable the outer host Back handler. | `FirstSignInNavigationTest.firstContinueThenMoviesRendersMindelsGridWithoutReenteringThePicker` — RED before the cached-entry correction; GREEN after. Fresh host, immediate signed-in Mindel replies, one Continue, rendered Movies (6), Shows (29), then Home. | | F2: picker skeleton outlives usable identity | The cached entry retained its skeleton; live-shell publication also formerly waited for saved-login I/O. Publish the current Jellyfin person before I/O, and let cached entries adopt that shell. | First-entry regression above; `ShellStartupTimingTest.firstFramePickerAndEveryTabWorkBeforeSavedPeopleArrive` keeps saved-user and real-wire activity reads pending while using the picker/tabs. | | F3: held-film cast/related missing | Missing ProviderIds prevented catalogue enrichment. Explicitly read missing ProviderIds/People without changing Jellyfin playback identity. | `TitleReadFacadeTest.heldMovieResolvesCatalogueIdentityWhenTheDetailOmitsProviderIds` | | F3: Discover details have no rows | Details opened a header-only preview route. Navigate to the shared title page and remove the obsolete parallel detail read. | `DiscoverTitleTimingTest.discoverDetailsFillCastAndRelatedAfterAsyncReplyAndBackRestoresDiscover` | | F4: scrolled rows bleed into nav | The scrolled brush still faded to transparent. Draw the whole scrolled band opaque; leave the unscrolled treatment/geometry unchanged. | `NavBandOcclusionTest.scrolledRowsNeverShowThroughTheNavBand` | | F6: empty Anime audience creates an empty tab | `is_anime` identifies a grouping, not owned-title counts. Before enabling Anime, issue a one-item Jellyfin existence read using the same user/library/series criteria as the Anime grid. No owners/library names or count fields are invented. Empty Home Anime shelves were already omitted and remain covered. | `AnimeAvailabilityTest.configuredAnimeAudienceNeedsActualTitlesBeforeItCreatesATabOrHomeShelf` — signed-in Shared/Mindel, zero and nonzero titles, owner/user audience membership, empty Home shelf exclusion. | | F6 safety: tab removal leaves invalid focus | Removing Anime while the avatar was focused retained the old index 5 for a new 0..4 nav. Remap focus to the avatar or a surviving destination. | Same Anime regression: RED at expected index 4 / actual 5; GREEN for Anime appearing and disappearing. | ## Active-root review correction (B6b) The review found that removing Anime while it was the current root left `route`/`rootRoute` and the actual singleton navigation entry pointing to Anime. Back then emitted Pop forever against that singleton. `ShellState.updatePeople` now moves a removed root to Home and emits `ShellAction.Destination("home", preservePushed = true)`. The real integration replaces the singleton root normally; when pages are pushed, it changes only entry zero, retaining the current title/person pages and order. Avatar and surviving-tab focus remapping remain unchanged. Regression: `AnimeRootRemovalTest.removedAnimeRootReturnsHomeWithoutDiscardingPushedPagesOrAvatarFocus`. RED on the reviewed head: expected `[Home(id=0)]`, observed `[Wolfskin(route=anime)]`. GREEN after the correction for all four scenarios: active root, avatar-focused root, pushed title, pushed title plus person. Back enters Home's nav ladder or pops only the current pushed page. Touched-area validation passed: **16 tests** across `AnimeRootRemovalTest`, `AnimeAvailabilityTest`, `FirstSignInNavigationTest`, `ShellStartupTimingTest`, `DiscoverTitleTimingTest`, `ShellKeysTest`, and `ShellSnapshotReadTest`. No full suite was run for this review cycle. The optional nested-handler note is bounded explicitly in the documentation; no broader handler propagation or player change was added. ## Verification - ONE final-head `make apk-debug` passed on `6e0f9952cf13225635a70767812c03674de5a652`, including the packaged branding check for all four APKs. All four delivered B6b SHA256 checksums verified. - F6/focus changes passed targeted Anime availability, fresh-sign-in rendering, async person-switch/startup, audience recovery and snapshot-read tests. The Anime test uses the existing signed-in audience wire shape and derives its library names from that response, not hardcoded owner/library guesses. - Firstmate's real Xiaomi B5 walk confirmed F1 fixed for Shared/Justin/Mindel and accepted F2 (fully drawn picker about eight seconds after sign-in). Earlier B4 walks confirmed cast/related rows on held and not-held title pages; nav bleed was not seen again. - **Full-suite history:** `FirstSignInNavigationTest.firstContinueThenMoviesRendersMindelsGridWithoutReenteringThePicker` previously timed out on Shows only in full runs: **671/672 app tests passed**, while its targeted run passed. The one authorized isolation correction added Compose idle waits, a 15-second wait on the actual Shows grid, and @After restoration of endpoint/fixture/screen preference globals. The subsequent authorized serial **`make test` passed on `5a846660`**, before F6 was added. No full-suite/native retry loop was run after the F6 instruction; final F6 changes were verified with targeted regressions. - Earlier native Robolectric `java/nio/ShortBuffer` SIGABRT runs were avoided by serial Gradle workers/test forks. Overrides were external to the repository; no tracked build settings changed. - SnapshotFlow blocks/getter dependencies were re-grepped, and snapshot-read smoke passed; no new state writes were introduced into read-only getter blocks. - Current main and the already-merged snapshot-read hotfix are included. Documentation is updated; the worktree is clean. Evidence, including the original first-entry RED output, historical 671/672 logs/XML, the passing isolation-cycle suite, native title captures and final APK output, is under `/root/.treehouse/firstmate-7bab20/2/firstmate/data/tv-walk-fix1/`. ## Device-unverified and risk **F5 video-player Back remains device-unverified** because playback was not allowed on the box. Existing playback handoff/Back regression remains; upstream player code still uses NavigationManager.goBack and was not modified. Corrected shell ownership closes the real session picker, and outgoing outer-host Back handlers are disabled. Nested title handlers retain their existing lifecycle-based behaviour (pre-existing review note); no universal nested-handler or real-player claim is made. The F6 existence read runs after the live picker is published. A failed availability read retains the signed-in user's last confirmed value; a successful empty read hides Anime. F6's final device re-walk remains with firstmate. No ADB, release/signing, version edits, service mutations or authority widening. This request is ready for review; nothing was merged.

justinzeus added 1 commit 2026-11-06 11:30:26 +01:00

Fix TV startup navigation and complete title pages d401ebc355

justinzeus added 1 commit 2026-11-06 11:52:51 +01:00

Read live shell state inside cached navigation entries afff383b75

justinzeus added 1 commit 2026-11-06 12:03:22 +01:00

Restrict shell route and Back handling to the current entry 22850632ec

justinzeus added 1 commit 2026-11-06 12:11:43 +01:00

Synchronize fresh-sign-in navigation and restore test globals 5a84666074

justinzeus added 1 commit 2026-11-06 12:25:53 +01:00

Hide Anime destinations without visible library titles 53b8358317

justinzeus added 1 commit 2026-11-06 12:33:12 +01:00

Preserve navigation focus when Anime availability changes 9e15dbee8c

justinzeus commented 2026-11-06 12:51:43 +01:00

Author

Owner

Copy link

Behaviour/safety verdict: request changes

Reviewed TV startup navigation and title-page fixes: #39

Reviewed head: 9e15dbee8c97dede2fba370693f09eaba736f403, against origin/main (e794dec25344d2f5ae05d959a9d3ceaff285505a). Read-only code review; no Gradle, make, device operations, or test reruns.

Blocking: B1 — Removing Anime can strand the current root destination and break Back

wolfskin-design/src/main/java/org/justinvisser/wolfskin/design/ShellState.kt:54–63 remaps navIndex when hasAnime changes, but leaves route and rootRoute equal to anime. The new successful-empty Jellyfin read can trigger this during onForeground (WolfskinApp.kt:126–148).

[INFERENCE: deterministic source trace, not device-reproduced] Open Anime as the root tab, background the app, remove the last visible anime series (or its access), and resume. After the successful empty existence read, tabs no longer contains Anime, but the current navigation entry is still Destination.Wolfskin("anime"). Every Back now enters route !in tabs and emits ShellAction.Pop (ShellState.kt:293–310). Root tabs use NavigationManager.replace, leaving a one-entry stack; NavigationManager.goBack deliberately does nothing on that stack (NavigationManager.kt:39–44,74–83). Back therefore cannot restore nav/Home; the page can display Anime content with Home highlighted. The avatar-index fix prevents the bounds crash but does not repair the invalid active destination.

Recommendation: when the removed tab is the active root, reconcile both shell route/root memory and the real navigation entry to a surviving root (Home is the conservative choice). Preserve the existing avatar/surviving-tab focus rule. Add coverage for removal while Anime itself is current, including Back; the new AnimeAvailabilityTest:84–100 only changes availability while Home/avatar is current.

Non-blocking notes

1. Nested Back callbacks are not covered by the new explicit active-entry guard. The host callback is guarded at WolfskinApp.kt:302, but TitleDestination.kt:316–318 still enables its own callback solely on title != null. Navigation 3 1.1.7 caps popped/replaced entries at CREATED, so those callbacks are lifecycle-disabled; an entry covered by a push remains STARTED during the transition. This is pre-existing, not a demonstrated new misroute. Propagate active-entry ownership if strengthening that guarantee; do not describe the outer guard as covering every nested handler.

2. The historical Shows timeout is consistent with suite isolation/timing, but its exact cause is not established by source alone. The regression creates fresh session/navigation/read objects, explicitly sets the endpoint, fixtures and screen preference, waits for the actual Shows grid, and restores those globals in @After (FirstSignInNavigationTest.kt:50–64,69–94,126–148). Other tests mutate process-wide preference objects and Dispatchers.Main; ShellStartupTimingTest does not restore the prior fixture flag and restores its URL only on the normal path (:35–42,96,101–104,161). These are concrete suite-isolation surfaces, not proof that a particular one caused Shows to time out. The PR body reports a later serial full-suite pass before F6; I did not rerun or independently verify that history. No separate product blocker follows from the reported full-suite-only timeout.

Checks and limits

• Cached entry content now reads live shell/Discover composition locals; only the current host entry writes the route. Effects are keyed/cancelled; no new unbounded collector was added. Person switching, title push/pop, Settings return, picker ownership and rollout-off routing were traced.
• The usable shell is published before saved-login I/O. Conductarr authentication/profile/audience/activity reads do not gate picker publication or the independent Home library read; failures publish usable/recovery state rather than the discarded launch skeleton.
• Discover details now use the existing shared title page. Held film/series/episode reads enrich missing ProviderIds/People without replacing playback/watch identity. Missing/unparseable TMDB IDs skip Conductarr enrichment; empty cast/related rows stay omitted, not error rows.
• Anime existence is one user-views request plus at most one limit=1, Series-only request per matching accessible library, with no pagination/polling or recomposition-triggered read. Jellyfin SDK 1.7.1 defaults enforce 6s connect and 30s total/read timeouts; its OkHttp factory applies them. Errors retain the current user's last confirmed availability; identity changes discard late results. Empty Home Anime shelves remain omitted.
• The merged snapshot-read hotfix and both audit/runtime regression sources remain intact; rememberedView does not insert into snapshot state, and all three owned snapshotFlow bodies remain read-only.
• No typography/token/geometry files changed. The only drawing change is the intended opaque scrolled nav band; the unscrolled brush is unchanged. The named locked-frame/spec paths are absent from this disposable worktree, so this is not a screenshot/frame-comparison sign-off. Actual player Back remains device-unverified as disclosed in the PR body.

Commit status: review/code-gpt = failure for B1 on the reviewed SHA. No source edits, pushes, approvals or merges.

## Behaviour/safety verdict: request changes Reviewed TV startup navigation and title-page fixes: https://git.justinvisser.org/justinzeus/wolfskin/pulls/39 Reviewed head: `9e15dbee8c97dede2fba370693f09eaba736f403`, against `origin/main` (`e794dec25344d2f5ae05d959a9d3ceaff285505a`). Read-only code review; no Gradle, make, device operations, or test reruns. ### Blocking: B1 — Removing Anime can strand the current root destination and break Back `wolfskin-design/src/main/java/org/justinvisser/wolfskin/design/ShellState.kt:54–63` remaps `navIndex` when `hasAnime` changes, but leaves `route` and `rootRoute` equal to `anime`. The new successful-empty Jellyfin read can trigger this during `onForeground` (`WolfskinApp.kt:126–148`). [INFERENCE: deterministic source trace, not device-reproduced] Open Anime as the root tab, background the app, remove the last visible anime series (or its access), and resume. After the successful empty existence read, `tabs` no longer contains Anime, but the current navigation entry is still `Destination.Wolfskin("anime")`. Every Back now enters `route !in tabs` and emits `ShellAction.Pop` (`ShellState.kt:293–310`). Root tabs use `NavigationManager.replace`, leaving a one-entry stack; `NavigationManager.goBack` deliberately does nothing on that stack (`NavigationManager.kt:39–44,74–83`). Back therefore cannot restore nav/Home; the page can display Anime content with Home highlighted. The avatar-index fix prevents the bounds crash but does not repair the invalid active destination. Recommendation: when the removed tab is the active root, reconcile both shell route/root memory and the real navigation entry to a surviving root (Home is the conservative choice). Preserve the existing avatar/surviving-tab focus rule. Add coverage for removal while Anime itself is current, including Back; the new `AnimeAvailabilityTest:84–100` only changes availability while Home/avatar is current. ### Non-blocking notes 1. **Nested Back callbacks are not covered by the new explicit active-entry guard.** The host callback is guarded at `WolfskinApp.kt:302`, but `TitleDestination.kt:316–318` still enables its own callback solely on `title != null`. Navigation 3 1.1.7 caps popped/replaced entries at CREATED, so those callbacks are lifecycle-disabled; an entry covered by a push remains STARTED during the transition. This is pre-existing, not a demonstrated new misroute. Propagate active-entry ownership if strengthening that guarantee; do not describe the outer guard as covering every nested handler. 2. **The historical Shows timeout is consistent with suite isolation/timing, but its exact cause is not established by source alone.** The regression creates fresh session/navigation/read objects, explicitly sets the endpoint, fixtures and screen preference, waits for the actual Shows grid, and restores those globals in `@After` (`FirstSignInNavigationTest.kt:50–64,69–94,126–148`). Other tests mutate process-wide preference objects and `Dispatchers.Main`; `ShellStartupTimingTest` does not restore the prior fixture flag and restores its URL only on the normal path (`:35–42,96,101–104,161`). These are concrete suite-isolation surfaces, not proof that a particular one caused Shows to time out. The PR body reports a later serial full-suite pass before F6; I did not rerun or independently verify that history. No separate product blocker follows from the reported full-suite-only timeout. ### Checks and limits - Cached entry content now reads live shell/Discover composition locals; only the current host entry writes the route. Effects are keyed/cancelled; no new unbounded collector was added. Person switching, title push/pop, Settings return, picker ownership and rollout-off routing were traced. - The usable shell is published before saved-login I/O. Conductarr authentication/profile/audience/activity reads do not gate picker publication or the independent Home library read; failures publish usable/recovery state rather than the discarded launch skeleton. - Discover details now use the existing shared title page. Held film/series/episode reads enrich missing ProviderIds/People without replacing playback/watch identity. Missing/unparseable TMDB IDs skip Conductarr enrichment; empty cast/related rows stay omitted, not error rows. - Anime existence is one user-views request plus at most one `limit=1`, Series-only request per matching accessible library, with no pagination/polling or recomposition-triggered read. Jellyfin SDK 1.7.1 defaults enforce 6s connect and 30s total/read timeouts; its OkHttp factory applies them. Errors retain the current user's last confirmed availability; identity changes discard late results. Empty Home Anime shelves remain omitted. - The merged snapshot-read hotfix and both audit/runtime regression sources remain intact; `rememberedView` does not insert into snapshot state, and all three owned `snapshotFlow` bodies remain read-only. - No typography/token/geometry files changed. The only drawing change is the intended opaque scrolled nav band; the unscrolled brush is unchanged. The named locked-frame/spec paths are absent from this disposable worktree, so this is not a screenshot/frame-comparison sign-off. Actual player Back remains device-unverified as disclosed in the PR body. Commit status: `review/code-gpt` = failure for B1 on the reviewed SHA. No source edits, pushes, approvals or merges.

justinzeus referenced this pull request 2026-11-06 13:00:24 +01:00

Give TV cards roomy spacing and edge-straddling focus rings #40

justinzeus added 1 commit 2026-11-06 13:11:10 +01:00

Reconcile removed Anime roots without discarding pushed pages 6e0f9952cf

justinzeus merged commit 711dbebf28 into main 2026-11-06 13:16:43 +01:00

justinzeus referenced this pull request from a commit 2026-11-06 13:16:45 +01:00

Merge pull request 'Fix TV startup navigation and complete title pages' (#39) from fm/tv-walk-fix1 into main

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

exploring

Idea with a proof gate; not scheduled until the gate passes

No labels

exploring

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: justinzeus/wolfskin#39

No description provided.