justinzeus/wolfskin
1
0
Fork 0
Code Issues 2 Pull requests 4 Releases 3 Packages Activity

Fix Home focus, Continue watching, picker and Back exit #59

Closed
justinzeus wants to merge 0 commits from fm/tv-r2-home into fm/tv-round2
pull from: fm/tv-r2-home
merge into: justinzeus:fm/tv-round2
Conversation 4 Commits - Files changed -
justinzeus commented 2026-11-07 18:35:28 +01:00
Owner
Copy link

Outcome

Fix Home’s Back/Down focus loop at navigation-entry ownership; merge Resume and Next up into Continue watching; remove the artwork strip below landscape-card gradients. Also make Settings reachable from every picker avatar and make root Back finish Wolfskin’s Android task without an old-UI detour.

Base: fm/tv-round2. Previous full-suite baseline: 5811ab05556266ffba8b86e211b1684b2b96dd94. AI-assisted implementation and description for this internal Wolfskin fork.

Latest picker-centering correction

Current head: f375d88030abd14bf7b6b262792af6f5e20b3acf. Picker correction: 7c67596c; hero stability: f375d880. Restore the approved gear centred under the whole people row, while keeping every-avatar Down, origin-return Up and root task exit. The previous last-avatar placement was wrong and is removed from code, spec, README and proof labeling.

Touched-only verification: 3 PickerNavigationTest cases + st002_gear passed, zero failures/errors/skips. Native geometry covers 1–5 people; the 1920×1080 gear capture was inspected. No full suite, APK build or device work for this hotfix. The gallery now shows incorrect last-avatar placement → restored approved whole-row centring.

Home hero late-primary stability hotfix

  • Root cause: saved Home can paint while the fresh watch read is still pending. HomeHeader treated !watchReady as Details-only, then replaced that primary Details button with Resume/Play + secondary Details. The native failing-before regression observed Details move x=96 → 338 px.
  • Fix: reserve the actual measured primary-button footprint without placing/drawing/enabling the button until playback is verified. Details remains secondary in its final position and remains available for cached browsing. The primary uses the existing 200 ms late-line fade. No fetch-order delay, new width token, copy-layer restructuring, HeroMotion.kt change or Discover/title layout change. When integrating, retain the sibling Discover branch’s HeroMotionScope and actions-outside-copy structure: this hotfix changes only the action-row contents and Layout import, not the copy layer.
  • Approved reference: st-007 — Mindel’s Resume hero (st-007-mindel-resume-in-the-hero.jpg), plus st-006 — Justin’s Play-next hero. The absent local opus-final-2026-11-05 directory was not substituted with an invented image: the approved images were read from the published written specification. This retains the approved primary + secondary Details arrangement with existing native metrics; it is not a new full-frame pixel-parity claim.
  • Touched-only verification: HomeHeroActionsTest and HomeCaptureTest.st007_mindelResume both passed, zero failures/errors/skips. The bounds invariant covers Resume episode, Play film and Play episode labels. Native pending/ready Resume pairs and the actual household st-007 Home were inspected. Together with the picker check above: 6 selected tests/captures passed; no full suite, APK build or box work.

The gallery separates latest touched-only verification and its three repaired pending/ready hero pairs from the earlier full-suite baseline.

Latest Home-entry and Back-ownership additions

Home-entry production changes: c5dbe910; real outgoing-entry regressions: a27e922675e2142907408bd75e97270e319cd48c.

  • Justin’s Home rule: first entry into an unvisited Home row starts at its leftmost card. Visited rows retain their column, first-visible card and scroll offset across row changes and Home tab leave/return. Home does not copy Discover’s tab-leave reset; Library grid navigation is unchanged. The existing Home per-row memory is reused, and the obsolete nearest-centre calculation is removed. The Discover reference branch was read, not merged.
  • Outgoing title/person Back: TitleDestination gates its shared title/person Android Back handler with the existing ScreenEntry active-ownership flag via isScreenEntryActive(). A popped entry can remain composed for the exit animation without swallowing the following Home Back or accidentally exiting the task.

Touched-only verification for these additions

Selection Result
Design: HomeRowEntryTest, two adjusted visited-row cases, HomeNavigationTest, title/person viewport-Back cases 11 passed, zero failures/errors/skips
App: TitleEntryBackTest, active-title sheet return, episode-parent Back, title sheet Back, Home root/hardware/Coming-in Back 8 passed, zero failures/errors/skips

The two new integration cases mount actual TitleDestination and retain a loaded outgoing title/person after pop. They assert pop → incoming Home nav → task exit on the following Back, with no premature exit. Row-entry cases cover mixed card widths, visited column/track retention and tab leave/return.

Per the installation-priority scope, no full make test or make apk-debug was rerun for these additions. One full suite and Opt check belongs to the merged-round candidate and still gates merging. The full-suite/APK results below remain the earlier 5811ab05 baseline, not a new full run on this head. The gallery labels those earlier frames separately from the latest touched-only picker/hero evidence.

Root causes and changes

Home focus loop

Sequential Down alone did not reproduce the box report. Keeping an outgoing real episode surface alive while Home returns did: HomeNavigationTest.outgoingEpisodeDuringBackCannotTakeOverTheHomeScroller expected row 3 after Down but remained on row 2 before the fix.

The outgoing title/person surface reread shell.view after the shared current route changed to Home. Two lazy frames then shared Home’s LazyListState; the outgoing page could also reacquire native focus. Stable row IDs cannot fix a viewport owned by the wrong entry.

ScreenEntry now supplies each entry’s own remembered viewport and active state. Only the active entry acquires native focus or registers the shell’s logical key controller. No row-name special case, new scrolling timer, or focus request against an uncomposed card.

Shared changes in HomeScreen, ShellSurface, LibraryScreen, DiscoverScreen, TitlePage and PersonPage are limited to viewport/input ownership. The Discover/title edits are not hero-transition or page-layout work. Existing frame/shelf timing and pending-row skipping remain intact.

One watch lane, including saved settings

  • buildHomeRows merges in-progress titles and next-up episodes into cw, sorts by actual watch activity, and chooses one episode per series, preferring its resumable episode. Hero precedence is unchanged and its title/series stays off the shelf. Skeletons use the same merged lane.
  • HomeReadFacade projects stable series identity and latest actual series-watch dates. Optional enrichment is bounded to three concurrent series reads; unknown dates remain unknown. HomeLibraryCache persists optional fields and reopens older entries without them. Debug fixtures use their recorded dates.
  • Legacy opt-out Home also uses the combined row. HomeSettingsService normalizes local/server/web/plugin settings; the first saved watch row retains its position and view options. Old serialized variants still decode, and the existing combined preset identity is unchanged. Add-row settings offer only Continue watching; obsolete Split/Combine UI actions are removed. The legacy service deduplicates series and uses actual watched-episode activity rather than premiere dates.
  • Library grids, Discover, person filmography and title shelves contain no separate paired Resume/Next up lanes to merge. Existing per-library See all and Home See all navigation remain unchanged; the former watch rows had no See all tail.

Bottom gradients

HomeShelf.TileOverlay put the state-line reserve outside its gradient background, exposing artwork below it on unwatched cards. The reserve now sits inside the background. The gradient starts lower with Home.cardOverlayTop = 18 dp (was 27 dp), reaches the solid bottom backing, and retains the approved 92% opacity. Caption/state-line geometry is unchanged.

Applies to Continue watching/Next up episode tiles, Resume/wide cards, New in your library and other Home landscape shelves using that component. Title episode tiles already extend their gradient to the bottom without the offending outer reserve. Discover/Library poster cards do not use this bottom title scrim. Hero scrims, rating plates and the correct solid state-line backing remain unchanged.

Picker and task exit

  • The gear retains its approved whole-row-centred placement, independent of profile count or selection. Down from every avatar reaches Settings; Up returns to its originating avatar, including after visiting Settings. Left/Right behavior, the 4 px accent straddle ring, 1.08 scale and focus-only caption stay unchanged.
  • Root content Back moves to nav; the next Back emits ShellAction.Exit and calls Activity.finishAndRemoveTask. Home, Discover, Movies, Shows and Anime no longer detour through Home or an upstream setup interface.
  • rememberWolfskinBackHandler owns Android Back even for a singleton NavDisplay. NavigationManager.goBack pops a parent, or requests task exit when no parent exists. Cold picker Back exits; reopened picker Back first dismisses to nav. The loading picker also has a working exit callback.
  • Child menus/sheets/unsaved-settings prompts retain first refusal. Actual Updates and Home Settings native regressions cover parent pops, root exit and dirty-state protection before Discard. The Discard test now requests real focus and sends DPAD_CENTER: its former unfocused semantic click never entered the callback. No production Discard workaround or injectable test seam remains.

Choices I made

  • Continue watching matches existing copy. Use the existing Next up landscape-tile geometry for the merged Wolfskin lane. Keep series names on Home episode tiles; removing them belongs only to the separate title-page change.

  • Preserve hero precedence: resume → next-up → newest → pick, independently of shelf recency.

  • Keep cw and persisted combined-row identity. Older cached episodes without series IDs fall back to saved series names until refreshed. Equal/unknown dates retain stable source order after known dates.

  • Keep the first saved legacy watch row’s position and options, rather than resetting customized Home settings.

  • Gradient amount: 18 dp top padding is an implementation-selected amount within Justin’s request to lower the gradient; 92% bottom opacity is existing approved styling.

Root task exit and every-avatar Down are Justin’s explicit requests, not agent-selected product choices. His exact words are cited in data/tv-spec/spec.md and docker/README.md. Those documents now distinguish the two implementation-selected visual details above from requested behavior.

Previous full-suite verification baseline

Previous full gate on 5811ab05 Result
Full app JVM suite 747 passed; zero failures/errors/skips
Full design JVM/native suite 228 passed; zero failures/errors/skips
Conductarr client suite 26 passed; live HTTP/cache/DNS smoke also passed
make apk-debug Four debug APKs; branding passed 1016 resources per APK; version 1.1.0-174-g5811ab05-65

The final make test used fresh generated binary result history, unchanged heap/worker/fork settings and an external class-start observer. The session picker started 94/116 and saved-people picker 98/116, both passing late rather than being promoted to the front by previous failures.

The first merged-head full gate had one new SavedPeople timeout. Its await now publishes pending Compose snapshots and pumps Android’s host looper, matching the existing session-picker convention. Its five-second deadline, blocked first response, identity assertions and pre-network visibility requirement are unchanged. No ignored/excluded tests, longer timeout, serial/heap workaround, fake fallback or runtime exception suppression.

Changed behavior Exercised coverage
Home Down and episode Back Five real Home navigation cases, including overlapping outgoing title, loading, refresh, reordered/inserted/removed rows and monotonic 16 ms scroll frames
Universal viewport restoration Real Library, Discover, person and title surfaces restore selection and reach their last row with scrolling
Merge and migration Recency/series deduplication, actual series chronology, older cache fields and saved-preset option/position retention
Shared gradients Native focused/unfocused tile/wide edge-pixel checks and six same-data before/after pairs
Picker Every avatar and Up origin after Settings; corrected whole-row alignment with 1–5 people is verified in the latest picker hotfix
Root/activity Back Actual WolfskinApp roots, hardware Back, cold/reopened picker and Coming-in sheet; actual Updates and unsaved Home Settings

Native proof and limits

Home round-2 native proof: six same-data gradient pairs, actual merged Home, fifteen manually clocked scroll frames with CSV, picker before/after, and five actual-root content/nav pairs with observed task-exit state. All native source frames are 1920×1080.

The throwaway exit smoke passed 18 cases: five actual WolfskinApp roots, two recovery-picker surfaces and eleven singleton full-screen dispatch variants. The variant matrix does not mount each individual full-screen page; actual Updates/Settings UI is covered by permanent native regressions. Temporary probes/capture harnesses do not ship.

No TV/device access, ADB, installation, optimized APK or merge. Native JVM frames do not certify physical GPU jank or Projectivy foreground behavior after task removal; those remain for the integrated box walk. Discover hero transitions, HeroMotion.kt and title/episode layout are owned by the sibling changes, not this slice.

## Outcome Fix Home’s Back/Down focus loop at navigation-entry ownership; merge Resume and Next up into **Continue watching**; remove the artwork strip below landscape-card gradients. Also make Settings reachable from every picker avatar and make root Back finish Wolfskin’s Android task without an old-UI detour. Base: `fm/tv-round2`. Previous full-suite baseline: `5811ab05556266ffba8b86e211b1684b2b96dd94`. AI-assisted implementation and description for this internal Wolfskin fork. ## Latest picker-centering correction Current head: `f375d88030abd14bf7b6b262792af6f5e20b3acf`. Picker correction: `7c67596c`; hero stability: `f375d880`. Restore the approved gear centred under the **whole people row**, while keeping every-avatar Down, origin-return Up and root task exit. The previous last-avatar placement was wrong and is removed from code, spec, README and proof labeling. Touched-only verification: **3 PickerNavigationTest cases + st002_gear passed**, zero failures/errors/skips. Native geometry covers 1–5 people; the 1920×1080 gear capture was inspected. No full suite, APK build or device work for this hotfix. The gallery now shows **incorrect last-avatar placement → restored approved whole-row centring**. ## Home hero late-primary stability hotfix - **Root cause:** saved Home can paint while the fresh watch read is still pending. `HomeHeader` treated `!watchReady` as Details-only, then replaced that primary Details button with Resume/Play + secondary Details. The native failing-before regression observed Details move **x=96 → 338 px**. - **Fix:** reserve the actual measured primary-button footprint without placing/drawing/enabling the button until playback is verified. Details remains secondary in its final position and remains available for cached browsing. The primary uses the existing 200 ms late-line fade. No fetch-order delay, new width token, copy-layer restructuring, `HeroMotion.kt` change or Discover/title layout change. When integrating, retain the sibling Discover branch’s `HeroMotionScope` and actions-outside-copy structure: this hotfix changes only the action-row contents and Layout import, not the copy layer. - **Approved reference:** **st-007 — Mindel’s Resume hero** (`st-007-mindel-resume-in-the-hero.jpg`), plus **st-006 — Justin’s Play-next hero**. The absent local `opus-final-2026-11-05` directory was not substituted with an invented image: the approved images were read from the published written specification. This retains the approved primary + secondary Details arrangement with existing native metrics; it is not a new full-frame pixel-parity claim. - **Touched-only verification:** `HomeHeroActionsTest` and `HomeCaptureTest.st007_mindelResume` both passed, zero failures/errors/skips. The bounds invariant covers Resume episode, Play film and Play episode labels. Native pending/ready Resume pairs and the actual household st-007 Home were inspected. Together with the picker check above: **6 selected tests/captures passed**; no full suite, APK build or box work. The gallery separates [latest touched-only verification](https://pages.justinvisser.org/wolfskin/tv-round2-home) and its three repaired pending/ready hero pairs from the earlier full-suite baseline. ## Latest Home-entry and Back-ownership additions Home-entry production changes: `c5dbe910`; real outgoing-entry regressions: `a27e922675e2142907408bd75e97270e319cd48c`. - **Justin’s Home rule:** first entry into an unvisited Home row starts at its leftmost card. Visited rows retain their column, first-visible card and scroll offset across row changes and Home tab leave/return. Home does **not** copy Discover’s tab-leave reset; Library grid navigation is unchanged. The existing Home per-row memory is reused, and the obsolete nearest-centre calculation is removed. The Discover reference branch was read, not merged. - **Outgoing title/person Back:** `TitleDestination` gates its shared title/person Android Back handler with the existing `ScreenEntry` active-ownership flag via `isScreenEntryActive()`. A popped entry can remain composed for the exit animation without swallowing the following Home Back or accidentally exiting the task. ### Touched-only verification for these additions | Selection | Result | |---|---| | Design: `HomeRowEntryTest`, two adjusted visited-row cases, `HomeNavigationTest`, title/person viewport-Back cases | **11 passed**, zero failures/errors/skips | | App: `TitleEntryBackTest`, active-title sheet return, episode-parent Back, title sheet Back, Home root/hardware/Coming-in Back | **8 passed**, zero failures/errors/skips | The two new integration cases mount actual `TitleDestination` and retain a loaded outgoing title/person after pop. They assert **pop → incoming Home nav → task exit on the following Back**, with no premature exit. Row-entry cases cover mixed card widths, visited column/track retention and tab leave/return. Per the installation-priority scope, **no full `make test` or `make apk-debug` was rerun for these additions**. One full suite and Opt check belongs to the merged-round candidate and still gates merging. The full-suite/APK results below remain the earlier `5811ab05` baseline, not a new full run on this head. The gallery labels those earlier frames separately from the latest touched-only picker/hero evidence. ## Root causes and changes ### Home focus loop Sequential Down alone did not reproduce the box report. Keeping an outgoing real episode surface alive while Home returns did: `HomeNavigationTest.outgoingEpisodeDuringBackCannotTakeOverTheHomeScroller` expected row 3 after Down but remained on row 2 before the fix. The outgoing title/person surface reread `shell.view` after the shared current route changed to Home. Two lazy frames then shared Home’s `LazyListState`; the outgoing page could also reacquire native focus. Stable row IDs cannot fix a viewport owned by the wrong entry. `ScreenEntry` now supplies each entry’s own remembered viewport and active state. Only the active entry acquires native focus or registers the shell’s logical key controller. No row-name special case, new scrolling timer, or focus request against an uncomposed card. Shared changes in `HomeScreen`, `ShellSurface`, `LibraryScreen`, `DiscoverScreen`, `TitlePage` and `PersonPage` are limited to viewport/input ownership. The Discover/title edits are not hero-transition or page-layout work. Existing frame/shelf timing and pending-row skipping remain intact. ### One watch lane, including saved settings - `buildHomeRows` merges in-progress titles and next-up episodes into `cw`, sorts by actual watch activity, and chooses one episode per series, preferring its resumable episode. Hero precedence is unchanged and its title/series stays off the shelf. Skeletons use the same merged lane. - `HomeReadFacade` projects stable series identity and latest actual series-watch dates. Optional enrichment is bounded to three concurrent series reads; unknown dates remain unknown. `HomeLibraryCache` persists optional fields and reopens older entries without them. Debug fixtures use their recorded dates. - Legacy opt-out Home also uses the combined row. `HomeSettingsService` normalizes local/server/web/plugin settings; the first saved watch row retains its position and view options. Old serialized variants still decode, and the existing combined preset identity is unchanged. Add-row settings offer only Continue watching; obsolete Split/Combine UI actions are removed. The legacy service deduplicates series and uses actual watched-episode activity rather than premiere dates. - Library grids, Discover, person filmography and title shelves contain no separate paired Resume/Next up lanes to merge. Existing per-library See all and Home See all navigation remain unchanged; the former watch rows had no See all tail. ### Bottom gradients `HomeShelf.TileOverlay` put the state-line reserve outside its gradient background, exposing artwork below it on unwatched cards. The reserve now sits inside the background. The gradient starts lower with `Home.cardOverlayTop = 18 dp` (was 27 dp), reaches the solid bottom backing, and retains the approved 92% opacity. Caption/state-line geometry is unchanged. Applies to Continue watching/Next up episode tiles, Resume/wide cards, New in your library and other Home landscape shelves using that component. Title episode tiles already extend their gradient to the bottom without the offending outer reserve. Discover/Library poster cards do not use this bottom title scrim. Hero scrims, rating plates and the correct solid state-line backing remain unchanged. ### Picker and task exit - The gear retains its **approved whole-row-centred placement**, independent of profile count or selection. Down from **every avatar** reaches Settings; Up returns to its originating avatar, including after visiting Settings. Left/Right behavior, the 4 px accent straddle ring, 1.08 scale and focus-only caption stay unchanged. - Root content Back moves to nav; the next Back emits `ShellAction.Exit` and calls `Activity.finishAndRemoveTask`. Home, Discover, Movies, Shows and Anime no longer detour through Home or an upstream setup interface. - `rememberWolfskinBackHandler` owns Android Back even for a singleton `NavDisplay`. `NavigationManager.goBack` pops a parent, or requests task exit when no parent exists. Cold picker Back exits; reopened picker Back first dismisses to nav. The loading picker also has a working exit callback. - Child menus/sheets/unsaved-settings prompts retain first refusal. Actual Updates and Home Settings native regressions cover parent pops, root exit and dirty-state protection before Discard. The Discard test now requests real focus and sends DPAD_CENTER: its former unfocused semantic click never entered the callback. No production Discard workaround or injectable test seam remains. ## Choices I made - **Continue watching** matches existing copy. Use the existing Next up landscape-tile geometry for the merged Wolfskin lane. Keep series names on Home episode tiles; removing them belongs only to the separate title-page change. - Preserve hero precedence: resume → next-up → newest → pick, independently of shelf recency. - Keep `cw` and persisted combined-row identity. Older cached episodes without series IDs fall back to saved series names until refreshed. Equal/unknown dates retain stable source order after known dates. - Keep the first saved legacy watch row’s position and options, rather than resetting customized Home settings. - **Gradient amount:** 18 dp top padding is an implementation-selected amount within Justin’s request to lower the gradient; 92% bottom opacity is existing approved styling. Root task exit and every-avatar Down are **Justin’s explicit requests**, not agent-selected product choices. His exact words are cited in `data/tv-spec/spec.md` and `docker/README.md`. Those documents now distinguish the two implementation-selected visual details above from requested behavior. ## Previous full-suite verification baseline | Previous full gate on `5811ab05` | Result | |---|---| | Full app JVM suite | 747 passed; zero failures/errors/skips | | Full design JVM/native suite | 228 passed; zero failures/errors/skips | | Conductarr client suite | 26 passed; live HTTP/cache/DNS smoke also passed | | `make apk-debug` | Four debug APKs; branding passed 1016 resources per APK; version `1.1.0-174-g5811ab05-65` | The final `make test` used fresh generated binary result history, unchanged heap/worker/fork settings and an external class-start observer. The session picker started **94/116** and saved-people picker **98/116**, both passing late rather than being promoted to the front by previous failures. The first merged-head full gate had one new SavedPeople timeout. Its await now publishes pending Compose snapshots and pumps Android’s host looper, matching the existing session-picker convention. Its **five-second deadline, blocked first response, identity assertions and pre-network visibility requirement are unchanged**. No ignored/excluded tests, longer timeout, serial/heap workaround, fake fallback or runtime exception suppression. | Changed behavior | Exercised coverage | |---|---| | Home Down and episode Back | Five real Home navigation cases, including overlapping outgoing title, loading, refresh, reordered/inserted/removed rows and monotonic 16 ms scroll frames | | Universal viewport restoration | Real Library, Discover, person and title surfaces restore selection and reach their last row with scrolling | | Merge and migration | Recency/series deduplication, actual series chronology, older cache fields and saved-preset option/position retention | | Shared gradients | Native focused/unfocused tile/wide edge-pixel checks and six same-data before/after pairs | | Picker | Every avatar and Up origin after Settings; corrected whole-row alignment with 1–5 people is verified in the latest picker hotfix | | Root/activity Back | Actual WolfskinApp roots, hardware Back, cold/reopened picker and Coming-in sheet; actual Updates and unsaved Home Settings | ## Native proof and limits [Home round-2 native proof](https://pages.justinvisser.org/wolfskin/tv-round2-home): six same-data gradient pairs, actual merged Home, fifteen manually clocked scroll frames with CSV, picker before/after, and five actual-root content/nav pairs with observed task-exit state. All native source frames are 1920×1080. The throwaway exit smoke passed **18 cases**: five actual WolfskinApp roots, two recovery-picker surfaces and eleven singleton full-screen dispatch variants. The variant matrix does not mount each individual full-screen page; actual Updates/Settings UI is covered by permanent native regressions. Temporary probes/capture harnesses do not ship. No TV/device access, ADB, installation, optimized APK or merge. Native JVM frames do not certify physical GPU jank or Projectivy foreground behavior after task removal; those remain for the integrated box walk. Discover hero transitions, `HeroMotion.kt` and title/episode layout are owned by the sibling changes, not this slice.
justinzeus added 5 commits 2026-11-07 18:35:28 +01:00
Synchronize saved-picker test with Android main queue
All checks were successful
review/code-claude No blocking findings; 8 non-blocking notes (see PR comment)
review/design-claude Design review PASS: no blocking finding; 8 non-blocking notes
Details
5811ab0555
justinzeus commented 2026-11-07 18:44:02 +01:00
Author
Owner
Copy link

Code review (Claude): no blocking findings. Reviewed head 5811ab05556266ffba8b86e211b1684b2b96dd94 (read-only; no Gradle run; the author's test gate was not reproduced).

Checked: the ScreenEntry ownership change across Home, shell, Library, Discover, title and person pages (an outgoing entry can no longer take native focus, register the key controller, or share Home's list state); the merged Continue watching lane (one episode per series, resumable preferred, newest watch activity first, unknown dates last, hero precedence kept, skeleton lane); cache compatibility for older saved Home entries; saved-settings migration (position and options of the first watch row kept, old serialized rows still decode, nothing rewritten until the user saves); root Back, task exit and the picker Down-to-Settings change; and that no test was excluded or weakened (the saved-people await keeps its 5 s deadline and every assertion).

Non-blocking notes:

  1. A quick double Back from a title page can now close the app. The outgoing title entry keeps its own Back handler enabled during the exit animation, its Pop reaches the new "go back at stack size 1 exits" path. Before this PR that second Back did nothing. Not run (inferred from code); I would fix it before the box walk by gating that handler on the entry being current, or by making Pop not exit.
  2. Merge conflict with the Discover PR (#58) in DiscoverScreen.kt's DisposableEffect: keep this PR's active-entry gate and the Discover PR's resetLaneMemory() in onDispose; the resolution is additive and safe. TitlePage.kt and spec.md merge cleanly; HomeHeader.kt and ShellState.kt are touched by only one PR.
  3. Discover is only partly entry-scoped: DiscoverState and DiscoverShelf still read the current route's view. Pre-existing, visual-only during exit animations.
  4. The exit signal is a conflated channel that buffers if nothing is collecting; no current path triggers it.
  5. HomeRowsTest.firstReadSkeletonHasNoFocusableContent lost its row-shape assertion instead of getting the new 4-row shape.
  6. The legacy combined row and Watch Next now order by latest watched episode, not premiere date; its getItems call passes no user id.
  7. The picker gear now sits under the last avatar; that placement is a design choice, not Justin's words.
  8. An outgoing Library entry can still issue one ratings read.

Full report: /root/.treehouse/firstmate-7bab20/2/firstmate/data/review-59-code/report.md.

**Code review (Claude): no blocking findings.** Reviewed head `5811ab05556266ffba8b86e211b1684b2b96dd94` (read-only; no Gradle run; the author's test gate was not reproduced). Checked: the `ScreenEntry` ownership change across Home, shell, Library, Discover, title and person pages (an outgoing entry can no longer take native focus, register the key controller, or share Home's list state); the merged Continue watching lane (one episode per series, resumable preferred, newest watch activity first, unknown dates last, hero precedence kept, skeleton lane); cache compatibility for older saved Home entries; saved-settings migration (position and options of the first watch row kept, old serialized rows still decode, nothing rewritten until the user saves); root Back, task exit and the picker Down-to-Settings change; and that no test was excluded or weakened (the saved-people await keeps its 5 s deadline and every assertion). Non-blocking notes: 1. A quick double Back from a title page can now close the app. The outgoing title entry keeps its own Back handler enabled during the exit animation, its Pop reaches the new "go back at stack size 1 exits" path. Before this PR that second Back did nothing. Not run (inferred from code); I would fix it before the box walk by gating that handler on the entry being current, or by making Pop not exit. 2. Merge conflict with the Discover PR (https://git.justinvisser.org/justinzeus/wolfskin/pulls/58) in `DiscoverScreen.kt`'s `DisposableEffect`: keep this PR's active-entry gate and the Discover PR's `resetLaneMemory()` in `onDispose`; the resolution is additive and safe. `TitlePage.kt` and `spec.md` merge cleanly; `HomeHeader.kt` and `ShellState.kt` are touched by only one PR. 3. Discover is only partly entry-scoped: `DiscoverState` and `DiscoverShelf` still read the current route's view. Pre-existing, visual-only during exit animations. 4. The exit signal is a conflated channel that buffers if nothing is collecting; no current path triggers it. 5. `HomeRowsTest.firstReadSkeletonHasNoFocusableContent` lost its row-shape assertion instead of getting the new 4-row shape. 6. The legacy combined row and Watch Next now order by latest watched episode, not premiere date; its `getItems` call passes no user id. 7. The picker gear now sits under the last avatar; that placement is a design choice, not Justin's words. 8. An outgoing Library entry can still issue one ratings read. Full report: `/root/.treehouse/firstmate-7bab20/2/firstmate/data/review-59-code/report.md`.
justinzeus commented 2026-11-07 18:44:48 +01:00
Author
Owner
Copy link

Design review (claude): PASS, no blocking finding

Reviewed SHA: 5811ab05556266ffba8b86e211b1684b2b96dd94. Full report: /root/.treehouse/firstmate-7bab20/2/firstmate/data/review-59-design/report.md. Evidence: the published frames at /opt/stacks/pages/store/wolfskin/tv-round2-home/ (https://pages.justinvisser.org/wolfskin/tv-round2-home), compared with PR 54's baseline /opt/stacks/pages/store/wolfskin/tv-card-meta-build/native/home-wide-unwatched.png and the approved data/tv-v2/opus-final-2026-11-05/assets/home-justin.jpg.

What I checked, with numbers:

  • Gradient pairs (6): the artwork strip below the scrim is gone on next/plain/resume, focused and unfocused. Gradient now spans 96 px (36 top + 32 title + 18 + 10 reserve) and starts 17-18 px lower (57% down a 224 px tile); cardOverlayTop 36 px / 18 dp agrees across home-tokens.json, tokens.css and WolfskinTokens.kt; opacity stays 0.92. No hard edge; captions legible. Settled by Justin's "starts too high / image leaks out below".
  • Merged Continue watching: one lane, existing title, 400x224 px tile at 440 px pitch, the same as the approved Next up tile and PR 54's row spacing; series names kept on tiles; hero excluded.
  • Scroll sequence: measured shifts between frames are 131, 104, 84, 65, 50, 37, 25, 16, 9, 3, 1, 0 px (total 524 px), strictly decelerating and monotonic, matching the ease-out cubic at 180 ms; no snap, overshoot or backward step.
  • Picker: gear centre x 1269.5 px = last avatar centre 1269.5 px; y, ring, caption unchanged.
  • Root-exit pairs (5): content ring -> nav pill, nothing else moves; JSONs show second Back = finishAndRemoveTask.

Non-blocking notes (full wording in the report):

  • N1 merged-Home frame has no resumable tile, so progress line and Resume/Next-up ordering are shown only by tests.
  • N2 scroll frames use a synthetic two-lane fixture that real Home no longer produces; HomeCardKind.Wide has no production producer left.
  • N3 first movement appears 48 ms (3 frames) after the key; likely the test clock, check on the box.
  • N4 focus ring cross-fades during the scroll (new ring about 57% at 80 ms); not part of this diff.
  • N5 caption contrast on white art drops to about 3.1:1 at glyph tops (was about 4:1); still legible, tuning knob is cardOverlayTop.
  • N6 bottom corners of dark-art tiles blend into the page (approved 0.92 backing).
  • N7 Title-page episode tiles still use a 54 px scrim top, Home now 36 px (Title lane).
  • N8 with 4-5 people the gear sits far right of centre; no frame was published for it.

Overlap with the Discover PR (#58, not reviewed): the Discover root frames show its hero pager; shared files DiscoverScreen.kt, TitlePage.kt, spec.md.

Not run: Gradle/make/tests. Physical Projectivy foreground and GPU jank remain for the box walk.

**Design review (claude): PASS, no blocking finding** Reviewed SHA: `5811ab05556266ffba8b86e211b1684b2b96dd94`. Full report: `/root/.treehouse/firstmate-7bab20/2/firstmate/data/review-59-design/report.md`. Evidence: the published frames at `/opt/stacks/pages/store/wolfskin/tv-round2-home/` (https://pages.justinvisser.org/wolfskin/tv-round2-home), compared with PR 54's baseline `/opt/stacks/pages/store/wolfskin/tv-card-meta-build/native/home-wide-unwatched.png` and the approved `data/tv-v2/opus-final-2026-11-05/assets/home-justin.jpg`. What I checked, with numbers: - Gradient pairs (6): the artwork strip below the scrim is gone on next/plain/resume, focused and unfocused. Gradient now spans 96 px (36 top + 32 title + 18 + 10 reserve) and starts 17-18 px lower (57% down a 224 px tile); `cardOverlayTop` 36 px / 18 dp agrees across home-tokens.json, tokens.css and WolfskinTokens.kt; opacity stays 0.92. No hard edge; captions legible. Settled by Justin's "starts too high / image leaks out below". - Merged Continue watching: one lane, existing title, 400x224 px tile at 440 px pitch, the same as the approved Next up tile and PR 54's row spacing; series names kept on tiles; hero excluded. - Scroll sequence: measured shifts between frames are 131, 104, 84, 65, 50, 37, 25, 16, 9, 3, 1, 0 px (total 524 px), strictly decelerating and monotonic, matching the ease-out cubic at 180 ms; no snap, overshoot or backward step. - Picker: gear centre x 1269.5 px = last avatar centre 1269.5 px; y, ring, caption unchanged. - Root-exit pairs (5): content ring -> nav pill, nothing else moves; JSONs show second Back = finishAndRemoveTask. Non-blocking notes (full wording in the report): - N1 merged-Home frame has no resumable tile, so progress line and Resume/Next-up ordering are shown only by tests. - N2 scroll frames use a synthetic two-lane fixture that real Home no longer produces; `HomeCardKind.Wide` has no production producer left. - N3 first movement appears 48 ms (3 frames) after the key; likely the test clock, check on the box. - N4 focus ring cross-fades during the scroll (new ring about 57% at 80 ms); not part of this diff. - N5 caption contrast on white art drops to about 3.1:1 at glyph tops (was about 4:1); still legible, tuning knob is `cardOverlayTop`. - N6 bottom corners of dark-art tiles blend into the page (approved 0.92 backing). - N7 Title-page episode tiles still use a 54 px scrim top, Home now 36 px (Title lane). - N8 with 4-5 people the gear sits far right of centre; no frame was published for it. Overlap with the Discover PR (https://git.justinvisser.org/justinzeus/wolfskin/pulls/58, not reviewed): the Discover root frames show its hero pager; shared files DiscoverScreen.kt, TitlePage.kt, spec.md. Not run: Gradle/make/tests. Physical Projectivy foreground and GPU jank remain for the box walk.
justinzeus added 1 commit 2026-11-07 18:52:31 +01:00
justinzeus added 1 commit 2026-11-07 19:19:47 +01:00
Cover outgoing title and person Back ownership
Some checks failed
review/code-claude Code APPROVE: Home delta fine, no blocker
Details
review/design-claude Design BLOCK: picker gear moved without approval (B1)
Details
a27e922675
justinzeus commented 2026-11-07 19:27:06 +01:00
Author
Owner
Copy link

Combined round-2 review (code + design) of the Home PR at head a27e922675e2142907408bd75e97270e319cd48c.

Code verdict: APPROVE (status review/code-claude = success).
Design verdict: BLOCK (status review/design-claude = failure), one blocker.

Blocking (B1) - profile picker Settings gear moved without Justin asking. Commit 9cfe9362 moved the gear from centred beneath the whole people row (approved, spec revision 5, Justin confirmed) to centred beneath the last avatar (WhosWatching.kt:56), made Down reach it from any avatar and Up return to the origin (ShellState.kt:200), and rewrote data/tv-spec/spec.md:7 (also docker/README.md:237 and :683) as a "Round-2 correction". Justin's round-2 words do not cover the picker. Fix: revert the position, the Down/Up rule and the spec/README text; a delta check of the fix follows. (My earlier design pass on 5811ab05 wrongly accepted this; withdrawn.)

Delta checked and fine (code):

  • Leftmost-row rule (HomeFocus.kt): an unvisited row starts at column 0 and first-visible 0, a visited row keeps its column and first-visible index, nothing resets Home memory on tab leave (HomeRowEntryTest goes Home -> Discover -> Home and asserts the same view, column, track offset). The focus-loop fix and ScreenEntry ownership are untouched; Library is not in the delta.
  • Back gating (TitleDestination.kt:343): an outgoing title/person entry is inactive, so its handler is disabled and the second Back reaches the app-level handler for the incoming Home. The first Back still pops; sheets and menus keep first refusal; the unsaved-settings handler is untouched. TitleEntryBackTest covers title and person.

Non-blocking notes:

  1. Root Back now finishes the Android task (ShellState.back, spec.md:9, README.md:248) and cold-launch picker Back exits. Justin's words in context.md do not mention it; confirm it was requested.
  2. Same commit: a reopened picker no longer resets pickerIndex to the last avatar; this goes away with the B1 revert.
  3. Continue-watching order by latest watch activity, series once with resume preferred (spec.md:21), is a reasonable reading of "merge resume and next up", not his words; it belongs under "Choices I made".
  4. Residual window: the active flag flips on the recomposition after the pop, so a second Back within the same frame (<16 ms) would still hit the outgoing handler. Not realistic from a remote. [INFERENCE]
  5. Spec/token edits otherwise covered by his words (gradient 54 -> 36 px in home-tokens.json, entry ownership, continue-watching lane). No approved frame file was edited.
  6. Integration with PR 58: one conflict in DiscoverScreen.kt, resolved correctly (keep this PR's entryActive key plus Discover's resetLaneMemory() in onDispose). After both land, spec.md line 72 (Discover branch's sentence that Home's nearest-centre shelves are unchanged) contradicts this PR's leftmost rule.

Full report: /root/.treehouse/firstmate-7bab20/2/firstmate/data/round2-review/report.md. Not run: Gradle/make (separate gate), nothing on the box.

Combined round-2 review (code + design) of the Home PR at head `a27e922675e2142907408bd75e97270e319cd48c`. **Code verdict: APPROVE (status `review/code-claude` = success).** **Design verdict: BLOCK (status `review/design-claude` = failure), one blocker.** **Blocking (B1) - profile picker Settings gear moved without Justin asking.** Commit `9cfe9362` moved the gear from centred beneath the whole people row (approved, spec revision 5, Justin confirmed) to centred beneath the last avatar (`WhosWatching.kt:56`), made Down reach it from any avatar and Up return to the origin (`ShellState.kt:200`), and rewrote `data/tv-spec/spec.md:7` (also `docker/README.md:237` and `:683`) as a "Round-2 correction". Justin's round-2 words do not cover the picker. Fix: revert the position, the Down/Up rule and the spec/README text; a delta check of the fix follows. (My earlier design pass on 5811ab05 wrongly accepted this; withdrawn.) **Delta checked and fine (code):** - Leftmost-row rule (`HomeFocus.kt`): an unvisited row starts at column 0 and first-visible 0, a visited row keeps its column and first-visible index, nothing resets Home memory on tab leave (`HomeRowEntryTest` goes Home -> Discover -> Home and asserts the same view, column, track offset). The focus-loop fix and `ScreenEntry` ownership are untouched; Library is not in the delta. - Back gating (`TitleDestination.kt:343`): an outgoing title/person entry is inactive, so its handler is disabled and the second Back reaches the app-level handler for the incoming Home. The first Back still pops; sheets and menus keep first refusal; the unsaved-settings handler is untouched. `TitleEntryBackTest` covers title and person. **Non-blocking notes:** 1. Root Back now finishes the Android task (`ShellState.back`, `spec.md:9`, `README.md:248`) and cold-launch picker Back exits. Justin's words in context.md do not mention it; confirm it was requested. 2. Same commit: a reopened picker no longer resets `pickerIndex` to the last avatar; this goes away with the B1 revert. 3. Continue-watching order by latest watch activity, series once with resume preferred (`spec.md:21`), is a reasonable reading of "merge resume and next up", not his words; it belongs under "Choices I made". 4. Residual window: the active flag flips on the recomposition after the pop, so a second Back within the same frame (<16 ms) would still hit the outgoing handler. Not realistic from a remote. [INFERENCE] 5. Spec/token edits otherwise covered by his words (gradient 54 -> 36 px in `home-tokens.json`, entry ownership, continue-watching lane). No approved frame file was edited. 6. Integration with PR 58: one conflict in `DiscoverScreen.kt`, resolved correctly (keep this PR's `entryActive` key plus Discover's `resetLaneMemory()` in `onDispose`). After both land, `spec.md` line 72 (Discover branch's sentence that Home's nearest-centre shelves are unchanged) contradicts this PR's leftmost rule. Full report: `/root/.treehouse/firstmate-7bab20/2/firstmate/data/round2-review/report.md`. Not run: Gradle/make (separate gate), nothing on the box.
justinzeus added 1 commit 2026-11-07 19:38:56 +01:00
justinzeus added 1 commit 2026-11-07 19:50:38 +01:00
Reserve Home hero primary action space
All checks were successful
review/code-claude Code APPROVE: delta f375d880 fine
Details
review/design-claude Design PASS: gear restored, hero buttons stationary
Details
f375d88030
justinzeus commented 2026-11-07 20:00:48 +01:00
Author
Owner
Copy link

Delta check (fix only) of the Home PR at head f375d88030abd14bf7b6b262792af6f5e20b3acf, diff a27e9226..f375d880.

Code verdict: APPROVE (review/code-claude = success).
Design verdict: PASS (review/design-claude = success). The blocker B1 is resolved and nothing new blocks.

B1, Settings gear: resolved.

  • WhosWatching.kt:56 is (stageWidth - gear) / 2 again, the same formula as origin/fm/tv-round2, so the gear is centred beneath the whole people row for any count. PickerNavigationTest now pins the gear centre to the midpoint of the first and last avatar centres for 1 to 5 people. The native frame picker-restored.png shows the gear centred under the middle avatar, with ring and caption as approved.
  • spec.md:7 carries the revision-5 text again. Down from any avatar, Up back to the sending avatar and the root-Back exit are now stated separately with Justin's quoted words (spec and README), not as a "Round-2 correction". I took the quotes from firstmate's brief; I could not check them against a source I can read.

Hero buttons stay put when Resume/Play data arrives: PASS.

  • The primary is measured in a Layout slot and placed only when watchReady. Unplaced, it is neither drawn nor focusable. Details sits after the reserved slot in both states, so its position cannot change.
  • HomeHeroActionsTest asserts identical Details bounds for Resume episode, Play film and Play episode. hotfix-verification.json shows the pre-fix failure (Details x 96 -> 338). The native pending/ready frames and hero-st007.png match the approved st-007/st-006 layout.
  • No wrong-button flash: the primary reveals by alpha only, over the 200 ms lateLineFade token.
  • Cached browsing works: pending has count = 1, Details is column 0 and activates Details, and Right is clamped. Ready gives Resume column 0 and Details column 1, as in the approved frames.

Merge with the Discover PR (79f64f6e): exactly one conflict, the known DiscoverScreen.kt hunk, resolved the same way. HomeHeader.kt now changes on both sides and auto-merges cleanly. The merged file keeps HeroMotionScope and HeroCopyHandoff around the copy only, with the buttons outside it.

Non-blocking notes:

  1. While pending, Details sits at its final position with an empty slot to its left. That is the intended consequence of reserving the footprint. If the label or kind changes at verification, the reserved width could differ [INFERENCE].
  2. At readiness the focus ring moves from Details to Resume (column 0 becomes the primary), and Details has no ring until Resume fades in. This is unchanged from before the fix, not a regression.
  3. The primary's fade replays on a hero identity change even when already ready. Rare, because Home's hero is non-rotating.
  4. Carried over: spec.md:72 after the merge still says Home's nearest-centre shelves are unchanged.

Full report (delta section 2): /root/.treehouse/firstmate-7bab20/2/firstmate/data/round2-review/report.md. Not run: Gradle/make, nothing on the box.

Delta check (fix only) of the Home PR at head `f375d88030abd14bf7b6b262792af6f5e20b3acf`, diff `a27e9226..f375d880`. **Code verdict: APPROVE (`review/code-claude` = success).** **Design verdict: PASS (`review/design-claude` = success).** The blocker B1 is resolved and nothing new blocks. **B1, Settings gear: resolved.** - `WhosWatching.kt:56` is `(stageWidth - gear) / 2` again, the same formula as `origin/fm/tv-round2`, so the gear is centred beneath the whole people row for any count. `PickerNavigationTest` now pins the gear centre to the midpoint of the first and last avatar centres for 1 to 5 people. The native frame `picker-restored.png` shows the gear centred under the middle avatar, with ring and caption as approved. - `spec.md:7` carries the revision-5 text again. Down from any avatar, Up back to the sending avatar and the root-Back exit are now stated separately with Justin's quoted words (spec and README), not as a "Round-2 correction". I took the quotes from firstmate's brief; I could not check them against a source I can read. **Hero buttons stay put when Resume/Play data arrives: PASS.** - The primary is measured in a `Layout` slot and placed only when `watchReady`. Unplaced, it is neither drawn nor focusable. Details sits after the reserved slot in both states, so its position cannot change. - `HomeHeroActionsTest` asserts identical Details bounds for Resume episode, Play film and Play episode. `hotfix-verification.json` shows the pre-fix failure (Details x 96 -> 338). The native pending/ready frames and `hero-st007.png` match the approved st-007/st-006 layout. - No wrong-button flash: the primary reveals by alpha only, over the 200 ms `lateLineFade` token. - Cached browsing works: pending has `count = 1`, Details is column 0 and activates Details, and Right is clamped. Ready gives Resume column 0 and Details column 1, as in the approved frames. **Merge with the Discover PR (`79f64f6e`):** exactly one conflict, the known `DiscoverScreen.kt` hunk, resolved the same way. `HomeHeader.kt` now changes on both sides and auto-merges cleanly. The merged file keeps `HeroMotionScope` and `HeroCopyHandoff` around the copy only, with the buttons outside it. **Non-blocking notes:** 1. While pending, Details sits at its final position with an empty slot to its left. That is the intended consequence of reserving the footprint. If the label or kind changes at verification, the reserved width could differ [INFERENCE]. 2. At readiness the focus ring moves from Details to Resume (column 0 becomes the primary), and Details has no ring until Resume fades in. This is unchanged from before the fix, not a regression. 3. The primary's fade replays on a hero identity change even when already ready. Rare, because Home's hero is non-rotating. 4. Carried over: `spec.md:72` after the merge still says Home's nearest-centre shelves are unchanged. Full report (delta section 2): `/root/.treehouse/firstmate-7bab20/2/firstmate/data/round2-review/report.md`. Not run: Gradle/make, nothing on the box.
justinzeus closed this pull request 2026-11-07 21:14:29 +01:00
All checks were successful
review/code-claude Code APPROVE: delta f375d880 fine
Details
review/design-claude Design PASS: gear restored, hero buttons stationary
Details

Pull request closed

Please reopen this pull request to perform a merge.
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
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#59
No description provided.
Delete branch "fm/tv-r2-home"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?