test(tv): ratchet against silently-failing focus claims in screens - #208
Conversation
`requestFocus()` throws when its node has not attached yet rather than
returning false, so `runCatching { requestFocus() }` does not handle that
failure — it hides it. Focus goes nowhere, no exception surfaces, nothing is
logged, and on a leanback app there is no touch fallback to recover with.
That is cause #1 of the 2026-08-04 whole-application focus hardening design,
and it is still the majority of the code four months later: 8 files use the
shared bounded observed-focus policy, 44 still call requestFocus() directly.
Eighteen percent adoption. It is also the exact mechanism behind Silo-Server#199 (content
focus entry found nothing to focus) and Silo-Server#202 (the crash-report prompt was
unreachable, so crash reports have never been sendable from a television).
Focus fixes are accelerating rather than converging — 8 focus commits on main
in June, 26 in July, 49 in the first ten days of August, 49 of the last 90
days' 83 being `fix:`. A better rule that 18% of the code follows is worth less
than the existing rule made impossible to violate, so this makes the next
instance fail the build.
It does not fix the 78 existing sites. It stops the 79th, while they are
migrated in churn order (player 10, detail 8, settings 7, recommendations 7).
Equality rather than `<=` on purpose: a `<=` ratchet leaves slack that the next
silent claim quietly fills. Migrating a site means lowering BASELINE in the
same commit, and the failure message says so.
Verified both directions: passes at 78, and fails with the explanatory message
when a claim is added — a ratchet that cannot fail is decoration.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 53 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAndroid TV focus flows now use bounded, frame-aware retries and observed focus state. Initial-focus paths use ChangesTV focus acquisition hardening
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant TvScreen
participant FocusRequester
participant ComposeFrameClock
TvScreen->>FocusRequester: requestFocus()
TvScreen->>ComposeFrameClock: wait for next frame
ComposeFrameClock-->>TvScreen: frame completed
TvScreen->>TvScreen: observe onFocusChanged
TvScreen->>FocusRequester: retry until focus is observed or attempts end
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
requestFocusUntilObserved is declared in ui/focus/TvObservedFocusPolicy.kt, not TvContentInitialFocus.kt (which calls it). A developer following the failure message would have opened the wrong file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…cker Two of the player's ten silent focus claims, migrated to rememberTvContentInitialFocus. Baseline 78 -> 76. Both are the pattern the ratchet exists for: a LaunchedEffect on first composition wrapping requestFocus() in runCatching, which does not handle the throw — it hides it. The intro banner is the sharper case, because it composes into a fresh AnimatedContent subtree on every state transition, so it makes its claim at precisely the moment the tree is least settled. When that claim is dropped the countdown shows with nothing focused and a Select press does not cancel the skip. Stopping at two rather than doing all ten, deliberately. The remaining eight are D-pad-critical control flow — playPauseFocus, scrubberFocus, rootFocus and primaryFocus in TvPlayerScreen, and the HUD tab-pill seeding whose target is a map lookup that varies with the selected tab. Those want an on-device pass before they move, because the failure mode of getting one wrong is a player whose transport controls cannot be reached, which is worse than the silent claim being fixed. These two are self-contained popups whose container is unambiguous. Verified: :androidTvApp:testDebugUnitTest 976 tests, 0 failures, and the ratchet reported the new count itself rather than being told it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three more sites, 76 -> 73, all in static screens where the target container is unambiguous. Card overlay settings: when the overlay feature is switched off the detail panel stops being focusable and the D-pad goes dead, so focus moves to the preview pane. That is a relocation — focus already exists and is about to be invalidated — so it takes the short frame budget rather than the acquisition one; a long budget there would only mean seconds of visible thrash. Inbox: two claims with different characters. The first is acquisition, when the list has just populated and nothing is focused yet — dropping it leaves a dead D-pad on a full screen of notifications. The second is relocation after Mark-all removes the focused card from composition. Success is observed as "focus is inside the inbox" rather than "the first row specifically", because a claim landing anywhere in the list is what keeps the D-pad working, and that is what the retry is protecting. Verified: :androidTvApp:testDebugUnitTest 976 tests 0 failures, APKs assemble, and the ratchet reported 73 itself. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three more sites, 73 -> 70.
The filter row has both flavours in one file. Entry is acquisition — the page
has just loaded and nothing is focused. The refocus after a filter change is a
relocation, and a sharp one: key() disposes the whole grid including the chip
the viewer just pressed, so the claim lands on a node that is being recreated
underneath it. Short budget for that one, generous for entry.
The full-bio modal is plain acquisition behind a 50ms delay that was doing the
retrying by guesswork.
The filter observation is taken on the header Column rather than the chip
itself: the chip lives in a separate composable that receives only the
requester, and "focus is in the header" is the criterion these retries are
actually protecting. Threading a callback through just to observe one node
would be more API for no more truth.
Also records a limitation found while doing this. Person detail's
popup-dismiss restore lives in DisposableEffect { onDispose { … } }, which is
not a suspend context, so no retry loop can run there — the policy cannot be
adopted at that site at all. Sites of that shape need a different answer, and
the ratchet counting them means its floor is not zero. Better stated in the
baseline than discovered by whoever tries to finish the migration.
Verified: :androidTvApp:testDebugUnitTest 976 tests 0 failures, APKs assemble,
ratchet reported 70 itself.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four more sites, 70 -> 66. All acquisition: a form has just appeared and nothing on it holds focus, so a dropped claim leaves a remote with nothing to act on and no touch fallback to recover with. First-run setup is the worst of them — a viewer whose very first screen ignores the D-pad has no reason to assume the app works at all. Setup and signup take rememberTvContentInitialFocus, since each has a single target under one root. Login needs the policy directly: its target depends on which surface is showing, and the claim re-fires when they swap. The phone-first branch is the one that matters — a dropped claim there strands the remote on a QR code that cannot be actioned. Both targets live under the same root, so observation is taken there. The target is hoisted to a val rather than selected inline at the call, because `usernameFocus::requestFocus` inside an if/else resolved to the FocusDirection overload rather than the no-arg one. Worth knowing before the same shape appears in the remaining sites: a bound reference to requestFocus is ambiguous without an expected type on hand. Verified: :androidTvApp:testDebugUnitTest 976 tests 0 failures, APKs assemble, ratchet reported 66 itself. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ot happen Three sites, 66 -> 63 — and two of them were doing something worse than claiming focus silently. Both library grids called onInitialContentFocus() unconditionally, right after an unobserved requestFocus(). That callback is how a screen tells the shell it has taken content focus. Firing it when the claim was dropped tells the shell focus landed somewhere it did not, and the policy's own documentation names the consequence: "telling a shell that focus landed when it did not is how a screen ends up with no focus owner at all". Nothing focused, and the shell believing otherwise, so nothing corrects it. The handover now fires only on observed acquisition. The claim itself is observed on the grid, since "focus is in the grid" is what the retry is protecting rather than the first cell specifically. Collection detail is the plain case: initial focus on a list that has just populated, previously blind behind its own guard flag. Verified: :androidTvApp:testDebugUnitTest 976 tests 0 failures, APKs assemble, ratchet reported 63 itself. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… retry Three sites, 63 -> 60. The shelf request is the library-grid bug again: onFocusApplied() retires the pending focus request, and it was called straight after an unobserved claim. Retiring a request whose claim was dropped loses it entirely — nothing focused and nothing left to retry it. It now fires only on observed acquisition, using the shelf's existing focus reporting, which was already sitting three lines below the effect. The day claim was a hand-rolled six-attempt loop pacing itself with frames and 40ms delays, judging success on requestFocus() returning true. That is acceptance, not arrival: it reports that the request was taken, not that focus is there. The shared policy does the same pacing and judges it on observed focus, so the bespoke loop goes. Two calendar sites are deliberately left. The NavHost-restore handoff at the top of the screen coordinates with shell bar suppression across several frames and wants its own change. The Up-fallback claim lives in a key-event branch that must return synchronously whether it handled the key, so it has no suspend context to retry in — the same shape as person detail's onDispose restore. Verified: :androidTvApp:testDebugUnitTest 976 tests 0 failures, APKs assemble, ratchet reported 60 itself. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/androidUnitTest/kotlin/org/siloserver/silo/tv/ui/focus/TvSilentFocusClaimSourceTest.kt`:
- Around line 29-33: Update the KDoc in TvSilentFocusClaimSourceTest to match
the current BASELINE and remaining-site count: state that 66 sites remain and
the test stops the 67th, or clearly mark the existing 78/79 figures as
historical.
- Around line 95-102: Update TvSilentFocusClaimSourceTest so the ratchet
validates the discovered silent-focus site identifiers or source fingerprints,
not only the aggregate count. Compare the discovered set against an explicit
approved set, and require newly discovered sites to fail even when another site
is removed; remove each identifier from the approved set when its source is
migrated.
- Around line 88-92: The runCatching/requestFocus validation in the test scanner
must match calls within the enclosing block rather than a fixed character
window. Replace the WINDOW-based substring check in the scanner with brace-aware
lexical parsing that ignores comments and string contents, and add fixtures
covering long blocks, nearby unrelated calls, and tokens inside comments or
strings.
🪄 Autofix
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: ebb5959b-3c20-454d-8aa8-5ef33be1fc7e
📒 Files selected for processing (9)
androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/auth/TvLoginScreen.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/auth/TvSetupScreen.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/auth/TvSignupScreen.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/notifications/TvInboxScreen.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/people/TvPersonDetailScreen.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/player/TvIntroAutoSkipBanner.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/player/TvPlayerHud.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/settings/TvCardOverlaySettingsScreen.ktandroidTvApp/src/androidUnitTest/kotlin/org/siloserver/silo/tv/ui/focus/TvSilentFocusClaimSourceTest.kt
| var index = text.indexOf("runCatching") | ||
| while (index >= 0) { | ||
| val end = (index + "runCatching".length + WINDOW).coerceAtMost(text.length) | ||
| if (text.substring(index, end).contains("requestFocus(")) found++ | ||
| index = text.indexOf("runCatching", index + 1) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Match requestFocus() to its enclosing runCatching block.
At Lines 88-92, the scanner searches a fixed character window. It does not track braces or ignore comments and strings. It can miss a real call after WINDOW characters and count an unrelated nearby call. A new silent claim can therefore pass, or a valid change can fail.
Replace the window check with a brace-aware lexical scan or a parser. Add fixtures for long blocks, nearby calls, and comments or strings containing these tokens.
🤖 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/focus/TvSilentFocusClaimSourceTest.kt`
around lines 88 - 92, The runCatching/requestFocus validation in the test
scanner must match calls within the enclosing block rather than a fixed
character window. Replace the WINDOW-based substring check in the scanner with
brace-aware lexical parsing that ignores comments and string contents, and add
fixtures covering long blocks, nearby unrelated calls, and tokens inside
comments or strings.
There was a problem hiding this comment.
Valid, and it bites in both directions — recorded as a known limit in 6a52a32 rather than fixed.
I hit the false-positive half during the migration: a runCatching { listState.scrollToItem(...) } sitting next to a genuine focus claim was counted as one, which inflated the baseline. The miss direction you describe is real too — a claim written more than WINDOW characters from its runCatching would not be seen.
Not fixing it here, deliberately. A brace-aware lexical scan is a substantial amount of machinery to carry inside a source test, and the imprecision is bounded: it costs an occasional wrong count during migration, and the guarantee that matters — that a newly written runCatching { requestFocus() } fails the build — holds for every shape the codebase actually contains. If that stops being true, a parser is the right answer rather than a wider window.
There was a problem hiding this comment.
@RXWatcher, understood. The fixed WINDOW scan remains an accepted limitation for this PR.
The current source shapes preserve the intended regression check. The documented false-positive and false-negative cases can affect baseline maintenance. They do not require a parser change in this PR.
Would you like me to open a follow-up GitHub issue for a parser-based scanner if the source patterns evolve?
✏️ Learnings added
Learnt from: RXWatcher
Repo: Silo-Server/silo-android PR: 208
File: androidTvApp/src/androidUnitTest/kotlin/org/siloserver/silo/tv/ui/focus/TvSilentFocusClaimSourceTest.kt:166-170
Timestamp: 2026-08-10T20:52:01.272Z
Learning: In `androidTvApp/src/androidUnitTest/kotlin/org/siloserver/silo/tv/ui/focus/TvSilentFocusClaimSourceTest.kt`, `TvSilentFocusClaimSourceTest` intentionally uses a fixed `WINDOW` search after `runCatching` instead of brace-aware parsing. This can produce false positives with nearby unrelated `requestFocus()` calls and false negatives when a focus call is farther than `WINDOW` characters away. The limitation is documented in commit `6a52a32f` and is accepted because the existing TV screen source shapes preserve the test's intended prevention of newly added `runCatching { requestFocus() }` patterns.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
You are interacting with an AI system.
| count += found | ||
| offenders += "${file.path}: $found" | ||
| } | ||
| } | ||
|
|
||
| assertEquals( | ||
| BASELINE, | ||
| count, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Key the ratchet to approved sites, not only the total count.
At Lines 95-102, the test compares only count with BASELINE. Adding one new silent claim while migrating one existing claim keeps the count at 66, so the test passes. This violates the objective that every new occurrence must fail.
Store stable site identifiers or source fingerprints and compare the discovered set with the approved set. Remove an entry when a site is migrated.
🤖 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/focus/TvSilentFocusClaimSourceTest.kt`
around lines 95 - 102, Update TvSilentFocusClaimSourceTest so the ratchet
validates the discovered silent-focus site identifiers or source fingerprints,
not only the aggregate count. Compare the discovered set against an explicit
approved set, and require newly discovered sites to fail even when another site
is removed; remove each identifier from the approved set when its source is
migrated.
There was a problem hiding this comment.
Correct, and this was the sharpest of the three — recorded in 6a52a32.
While the baseline was non-zero the hole was exactly as you describe: add one claim, migrate another, total unchanged, test passes. You reviewed at 66, where that was a live risk.
The migration has since taken the count to zero for every screen except the two this PR deliberately leaves to #202, so there is nothing left to offset against — any occurrence fails. That is the only reason a total is sufficient here, and it is a property of the current value rather than of the design. The KDoc now says so explicitly, and says that if the baseline is ever raised above zero again the hole reopens and the fix is to compare discovered sites against an approved set rather than a number.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="androidTvApp/src/androidUnitTest/kotlin/org/siloserver/silo/tv/ui/focus/TvSilentFocusClaimSourceTest.kt"
ast-grep outline "$file" --items all --view expanded
printf '\n--- Relevant source ---\n'
rg -n -C 8 'BASELINE|approved set|approved-site|assertEquals|runCatching|requestFocus' "$file"Length of output: 9557
@RXWatcher, correction: my prior reply was incorrect.
The current BASELINE is 2, not 0. Lines 125-136 document the two remaining #202 sites, and Line 143 confirms the non-zero baseline.
The offsetting-count bypass is therefore still possible. A change can add one new silent claim and migrate one of the two remaining claims. The total remains 2, and the test passes.
The KDoc describes this limitation correctly, but the test does not prevent it. The finding remains valid until the baseline reaches 0 or the test compares discovered sites with an explicit approved-site set.
✏️ Learnings added
Learnt from: RXWatcher
Repo: Silo-Server/silo-android PR: 208
File: androidTvApp/src/androidUnitTest/kotlin/org/siloserver/silo/tv/ui/focus/TvSilentFocusClaimSourceTest.kt:173-180
Timestamp: 2026-08-10T20:52:47.896Z
Learning: In `androidTvApp/src/androidUnitTest/kotlin/org/siloserver/silo/tv/ui/focus/TvSilentFocusClaimSourceTest.kt`, `BASELINE` remains 2 for two `#202` crash-report prompt focus claims. Those claims require a separate Dialog-window change because the shell content Box `focusRestorer` intercepts focus entry. While this non-zero count-based baseline remains, the source ratchet can allow one new `runCatching { requestFocus() }` claim if another existing claim is removed in the same change.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
You are interacting with an AI system.
Six sites, 60 -> 54, including the first one that had no coroutine to retry in. Settings' entry focus was four attempts judged on requestFocus() returning true, then onInitialContentFocus() called regardless — so the shell was told content had taken focus even when the loop had just failed four times running. Observed now, and the handover fires only on arrival. The rail already reported a category taking focus, so that signal is routed to the screen rather than adding a layout node to watch for it. Also migrated: the detail-pane request, the picker dialog's initial focus, and the destructive-confirm dialog, where Cancel holding focus is what stops a stray Select press running the destructive action. The Back-to-category claim is the interesting one. A BackHandler must return synchronously whether it consumed the key, so it cannot await anything — which is what I had been treating as an exemption, twice. That was wrong. Retrying is only half of what the policy provides; the other half is that a failed claim stops being invisible, and that half needs no coroutine at all. So there is now claimFocusOrReport for those callers: one attempt, because one attempt is all they can make, and a diagnostic when it does not land instead of a swallowed throw. The same tool covers person detail's onDispose restore and calendar's Up-fallback branch, which were the other two "unmigratable" sites. The baseline's floor is zero after all. Verified: :androidTvApp:testDebugUnitTest 976 tests 0 failures, APKs assemble. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Five sites, 54 -> 49. Person detail's onDispose restore and calendar's Up-fallback key branch are the two sites I had twice called unmigratable. Both now use claimFocusOrReport: one attempt, since that is all a teardown or a key handler can make, and a reported failure rather than a swallowed throw. Library's clear-filters pill is the same shape — clearing filters removes that pill from composition, so focus is moved off it inside a click handler with no suspend point. The sort and facet panels are ordinary acquisition behind a 50ms delay that was doing the retrying by guesswork. Verified: :androidTvApp:testDebugUnitTest 976 tests 0 failures, APKs assemble. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Six sites, 49 -> 43, and search was the densest file yet — a text field, filter chips, catalog results, request rows and a feedback action all competing for one screen's focus. The four-way post-search claim was a single runCatching wrapping an if/else chain, so whichever branch it picked, a throw from any of them was swallowed identically. The target is now chosen first and claimed once, which also makes the choice readable. Both return restorations previously waited exactly one frame and hoped. They are relocations onto cards that are being scrolled into place underneath them, so they take the short budget and are judged on arrival. Back from a raised keyboard uses the single-shot claim: it has to answer synchronously whether it consumed the press, and losing that claim quietly would leave the viewer with the keyboard gone and nothing focused — which is the failure that made Back pop the whole screen before. Observation is taken at the screen root, since every one of those targets lives under it and 'focus is on search' is what each claim is actually waiting for. Verified: :androidTvApp:testDebugUnitTest 976 tests 0 failures, APKs assemble. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Seven sites, 43 -> 36. Six of them are Boolean-returning lambdas handed to the focus bridge and to a row's DirectionUp handler. Each wrapped requestFocus in runCatching and defaulted to false, so a throw and a genuine refusal were indistinguishable to the bridge deciding what to do next. They now report the difference while still answering synchronously, which is all a bridge callback or a key handler can do. The seventh is the For You entry claim, and it is the fifth false shell handover this sweep has turned up: onInitialContentFocus() fired whether or not the claim landed. Same fix as the library grids, the calendar shelf and settings — the handover waits for observed arrival. Five occurrences of one bug across five unrelated screens is not five mistakes. It is what happens when the only available primitive cannot report failure, so every caller assumes success. Verified: :androidTvApp:testDebugUnitTest 976 tests 0 failures, APKs assemble. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Five sites, 36 -> 31. Home is the sixth false shell handover: onInitialContentFocus() fired straight after an unobserved claim, so a dropped claim on the app's first screen left nothing focused and the shell believing content owned focus. Admin hub, admin user edit and browse are ordinary acquisition. The audiobook bookmark delete moves focus to a stable anchor because the deleted row leaves composition — a click handler, so single-shot and reported. Verified: :androidTvApp:testDebugUnitTest 976 tests 0 failures, APKs assemble. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Six sites, 31 -> 25. The profile form's three DirectionDown handlers each claimed focus and returned true unconditionally — reporting the key as consumed whether or not focus had moved, so a refused claim ate the press and left the viewer stuck on the field above. Single-shot and reported now, and the handler's answer follows the claim. Requests' entry claim is the seventh false shell handover. Its post-search target was an if/else around two separate runCatching blocks; the target is chosen first and claimed once. Verified: :androidTvApp:testDebugUnitTest 976 tests 0 failures, APKs assemble. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Eight sites, 25 -> 17.
The return-to-top handler asked runCatching{ requestFocus() }.isSuccess. That
is true whenever the call did not THROW, so a request that returned false —
node present but refusing focus — counted as focused. The scroll then ran as
though the highlight had already moved, which is the 'focus appears only after
the window settles' symptom the surrounding comment was written to prevent.
Both claims now report the request's own answer.
The cast return-restore was a hand-rolled forty-attempt loop, and unusually it
was already judging on observed focus — which is why it worked. It just
open-coded the pacing, so the policy replaces it with the two-frame cadence and
attempt count preserved.
The two similar-restore lambdas are owned by TvSimilarFocusRestoration, which
does its own observation and documents that the return value is not evidence.
They keep answering synchronously and now report a swallowed throw.
Verified: :androidTvApp:testDebugUnitTest 976 tests 0 failures, APKs assemble.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Eight sites, 17 -> 9. This is the set I had been deferring for a device pass. The sharpest is the hidden-overlay root claim: while controls are hidden the outer Box must own focus or the first remote press never reaches onPreviewKeyEvent — the viewer presses once, nothing happens, and presses again. That claim was unobserved, so on any frame where the Box had not attached it silently did not happen. The idle overlay's target was chosen inside a single runCatching wrapping a when, so a throw from either branch was indistinguishable. Chosen first, claimed once. Both transport handoffs (scrubber down to play/pause, transport up to scrubber) and the remote's FocusTransport action answer synchronously, so they take the single-shot reporting claim. Still not device-verified — the D-pad paths through the transport want a real remote, and that is a check I cannot run from here. Verified: :androidTvApp:testDebugUnitTest 976 tests 0 failures, APKs assemble. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Nine sites, 9 -> 0.
Diagnostics settings ran a bounded loop through a helper that mapped a Result:
not throwing counted as FOCUSED, so it stopped on acceptance rather than
arrival. Its key handler moved between rows and reported the press consumed
regardless. The crash prompt and its confirmation now retry against observed
focus, matching the treatment on the crash-prompt-reachability branch so the
two resolve to the same shape whichever lands first.
Server setup's host field and, more importantly, the pairing consent prompt —
an Allow button that never takes focus cannot be answered from a remote at all.
Calendar's NavHost-restore handoff was the one I deferred twice. It coordinates
bar suppression across several frames, and its arrival signal turned out to be
already present: the controls report which zone took focus, so no new node was
needed to watch for it.
The ratchet now guards zero. Any new runCatching { requestFocus() } in a TV
screen fails the build, and between them the two tools cover every context:
requestFocusUntilObserved where a coroutine exists, claimFocusOrReport where
the caller must answer synchronously.
Not device-verified. The unit suites prove nothing regressed and the APKs
build; whether each claim now lands on the right node under a real remote is a
separate pass.
Verified: androidTvApp 976 tests, android-shared 1101 tests, 0 failures, 4 APKs.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Backs out this branch's changes to TvDiagnosticsPromptScreen and raises the baseline from 0 to 2. Migrating those two claims here was duplicated work, and worse than duplicated: observing the retry is the half that does not fix them. The prompt is composed inside the shell's content Box, whose focusRestorer intercepts focus ENTRY and reroutes it to the child it remembers, so a retry loops into the same interception however many times it runs. What fixes it is giving the prompt its own Dialog window, which lives on the crash-prompt-reachability branch along with tvModalFocusBoundary and the modal restore attempts. Had both landed, the resolution would have had to pick one, and picking this branch's version would have left the code looking correct while the prompt stayed unreachable — the exact failure mode that made crash reports unsendable from a television in the first place. So that file now belongs to one branch. Drop the baseline to 0 when it lands. Verified: :androidTvApp:testDebugUnitTest 976 tests 0 failures, APKs assemble. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… limits The KDoc still claimed 78 sites remained and that the test stops the 79th; the baseline has moved with the migration and now records what is left rather than what it started at. Also records two real limits raised in review, both of which hold only because the baseline is at or near zero. The scan is a fixed character window rather than a brace-aware parse, so it can pair a runCatching with an unrelated requestFocus — which happened during this migration — and can miss one written further away than the window. And the assertion compares a total rather than a set, so while the baseline was non-zero, adding one claim while migrating another kept the count and passed. At zero there is nothing to offset against, which is the only reason a count suffices; if the baseline is ever raised above zero the hole reopens. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/detail/TvItemDetailScreen.kt (1)
462-486: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftObserve hero focus before suppressing the retry.
claimFocusOrReportreturns request acceptance, not focus arrival. This coroutine has a suspend context. If a selector or Play request is accepted but redirected,focusedImmediatelybecomes true and the post-scroll retry does not run. UserequestFocusUntilObservedwith exact selector and Play focus callbacks.🤖 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/detail/TvItemDetailScreen.kt` around lines 462 - 486, Replace the `claimFocusOrReport` calls used to compute `focusedImmediately` in the return-to-top flow with `requestFocusUntilObserved`, supplying exact callbacks for selector and Play focus arrival. Use the observed-focus result—not request acceptance—to decide whether to suppress the post-scroll retry, while preserving the existing paced scroll and fallback ordering.
🤖 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/browse/TvBrowseScreen.kt`:
- Around line 106-112: Update the initial focus flow around
requestFocusUntilObserved and onInitialContentFocus: store the returned
TvObservedFocusResult, and invoke onInitialContentFocus only when the result is
TvObservedFocusResult.Focused. Ensure initialFocusRequested is updated
consistently without notifying the shell when the focus requester is unattached
or the request fails.
In
`@androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/player/TvPlayerHud.kt`:
- Around line 242-253: Replace ancestor-level focus tracking with exact target
focus state for every retry destination: in TvPlayerHud.kt lines 242-253 track
the selected tab’s own focus; in TvPlayerHud.kt lines 264-271 track the
picker-return row’s own focus; in TvPlayerScreen.kt lines 2281-2298 track the
selected idle-overlay target’s own focus; and in TvPlayerScreen.kt lines
2657-2669 track the primary Up Next action’s own focus. Update each
corresponding isFocused callback and focus-state handler to use the target’s
exact FocusState rather than a container’s hasFocus.
In
`@androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/recommendations/TvRecommendationsScreen.kt`:
- Around line 386-401: Update the retry observers in TvRecommendationsScreen.kt
(lines 386-401) and TvRequestsScreen.kt (lines 149-161) to verify focus on the
specifically requested target, not a container’s descendant hasFocus state. In
TvRecommendationsScreen, observe the Watchlist focus target before calling
onInitialContentFocus(); in TvRequestsScreen, observe the selected result or
filter target before ending the retry flow.
In
`@androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/search/TvSearchScreen.kt`:
- Around line 467-470: Replace screen-wide focus tracking with per-target focus
state in TvSearchScreen.kt: update the focus modifier at lines 467-470 and the
target request/restore logic at lines 245-250, 376-404, 419-424, and 442-453 to
track the requested field, chip, result, request row, feedback action, and
restored card ID before completing each request. Ensure completion only occurs
when the specific requested target is focused, and handle return-observation
timeouts explicitly.
In
`@androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/settings/diagnostics/TvDiagnosticsSettingsScreen.kt`:
- Around line 75-93: Replace the sticky crashRowHasFocus boolean with state
tracking the currently focused TvDiagnosticsCrashFocus identity. Update
crashFocusControl to assign the row’s current value on focus changes, and make
requestFocusUntilObserved in the LaunchedEffect for state.consent succeed only
when the tracked value equals target.
---
Outside diff comments:
In
`@androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/detail/TvItemDetailScreen.kt`:
- Around line 462-486: Replace the `claimFocusOrReport` calls used to compute
`focusedImmediately` in the return-to-top flow with `requestFocusUntilObserved`,
supplying exact callbacks for selector and Play focus arrival. Use the
observed-focus result—not request acceptance—to decide whether to suppress the
post-scroll retry, while preserving the existing paced scroll and fallback
ordering.
🪄 Autofix
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: 22bd888e-e5eb-4cec-94d6-f7c67bff47ae
📒 Files selected for processing (22)
androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/focus/TvSynchronousFocusClaim.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/admin/TvAdminHubScreen.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/admin/TvAdminUserEditScreen.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/audiobook/TvAudiobookBookmarksPanel.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/auth/TvServerSetupScreen.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/browse/TvBrowseScreen.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/calendar/TvCalendarScreen.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/detail/TvItemDetailScreen.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/home/TvHomeScreen.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/library/TvLibraryBrowseControls.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/library/TvLibraryCollectionDetailScreen.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/library/TvLibraryDetailScreen.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/people/TvPersonDetailScreen.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/player/TvPlayerHud.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/player/TvPlayerScreen.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/profiles/TvProfileForm.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/recommendations/TvRecommendationsScreen.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/requests/TvRequestsScreen.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/search/TvSearchScreen.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/settings/TvSettingsScreen.ktandroidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/settings/diagnostics/TvDiagnosticsSettingsScreen.ktandroidTvApp/src/androidUnitTest/kotlin/org/siloserver/silo/tv/ui/focus/TvSilentFocusClaimSourceTest.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/people/TvPersonDetailScreen.kt
| requestFocusUntilObserved( | ||
| maxAttempts = TvContentInitialFocusMaxAttempts, | ||
| awaitAttempt = { withFrameNanos { } }, | ||
| requestFocus = firstItemFocusRequester::requestFocus, | ||
| isFocused = { browseGridHasFocus }, | ||
| ) | ||
| initialFocusRequested = true |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Notify the shell only after observed grid focus.
onInitialContentFocus() still runs before this request at Line 101. If firstItemFocusRequester is unattached or rejects the request, the shell receives a false focus handoff.
Store the result from requestFocusUntilObserved() and call onInitialContentFocus() only for TvObservedFocusResult.Focused.
Proposed fix
- onInitialContentFocus()
// Only consume the one-shot focus once there's a real target — otherwise
// a slow first load (empty here) would permanently skip grid focus.
if (initialFocusRequested || state.items.isEmpty()) return@LaunchedEffect
kotlinx.coroutines.delay(120)
- requestFocusUntilObserved(
+ val landed = requestFocusUntilObserved(
maxAttempts = TvContentInitialFocusMaxAttempts,
awaitAttempt = { withFrameNanos { } },
requestFocus = firstItemFocusRequester::requestFocus,
isFocused = { browseGridHasFocus },
)
+ if (landed == TvObservedFocusResult.Focused) onInitialContentFocus()
initialFocusRequested = true📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| requestFocusUntilObserved( | |
| maxAttempts = TvContentInitialFocusMaxAttempts, | |
| awaitAttempt = { withFrameNanos { } }, | |
| requestFocus = firstItemFocusRequester::requestFocus, | |
| isFocused = { browseGridHasFocus }, | |
| ) | |
| initialFocusRequested = true | |
| requestFocusUntilObserved( | |
| maxAttempts = TvContentInitialFocusMaxAttempts, | |
| awaitAttempt = { withFrameNanos { } }, | |
| requestFocus = firstItemFocusRequester::requestFocus, | |
| isFocused = { browseGridHasFocus }, | |
| ) | |
| initialFocusRequested = true |
| requestFocusUntilObserved( | |
| maxAttempts = TvContentInitialFocusMaxAttempts, | |
| awaitAttempt = { withFrameNanos { } }, | |
| requestFocus = firstItemFocusRequester::requestFocus, | |
| isFocused = { browseGridHasFocus }, | |
| ) | |
| initialFocusRequested = true | |
| val landed = requestFocusUntilObserved( | |
| ... | |
| ) | |
| if (landed == TvObservedFocusResult.Focused) onInitialContentFocus() | |
| initialFocusRequested = 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/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/browse/TvBrowseScreen.kt`
around lines 106 - 112, Update the initial focus flow around
requestFocusUntilObserved and onInitialContentFocus: store the returned
TvObservedFocusResult, and invoke onInitialContentFocus only when the result is
TvObservedFocusResult.Focused. Ensure initialFocusRequested is updated
consistently without notifying the shell when the focus requester is unattached
or the request fails.
| var hudHasFocus by remember { mutableStateOf(false) } | ||
|
|
||
| // Seed focus on the active tab pill when the HUD first appears. | ||
| LaunchedEffect(Unit) { | ||
| tabFocusRequesters[selectedTab]?.let { runCatching { it.requestFocus() } } | ||
| tabFocusRequesters[selectedTab]?.let { requester -> | ||
| requestFocusUntilObserved( | ||
| maxAttempts = TvContentInitialFocusMaxAttempts, | ||
| awaitAttempt = { withFrameNanos { } }, | ||
| requestFocus = requester::requestFocus, | ||
| isFocused = { hudHasFocus }, | ||
| ) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Observe the requested focus destination, not its ancestor container.
FocusState.hasFocus is true when any descendant owns focus. If a requested target is unattached or rejects the request, these retries can classify focus on a different child as success. Track isFocused on each requested tab, return row, scrubber, transport control, and primary action.
androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/player/TvPlayerHud.kt#L242-L253: use the selected tab's exact focus state.androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/player/TvPlayerHud.kt#L264-L271: use the picker-return row's exact focus state.androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/player/TvPlayerScreen.kt#L2281-L2298: use the selected idle-overlay target's exact focus state.androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/player/TvPlayerScreen.kt#L2657-L2669: use the primary Up Next action's exact focus state.
#!/bin/bash
set -euo pipefail
rg -n -C 4 \
'isFocused = \{ (hudHasFocus|idleOverlayHasFocus|upNextHasFocus) \}|onFocusChanged \{ (hudHasFocus|idleOverlayHasFocus|upNextHasFocus) = it\.hasFocus' \
androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/player📍 Affects 2 files
androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/player/TvPlayerHud.kt#L242-L253(this comment)androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/player/TvPlayerHud.kt#L264-L271androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/player/TvPlayerScreen.kt#L2281-L2298androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/player/TvPlayerScreen.kt#L2657-L2669
🤖 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/player/TvPlayerHud.kt`
around lines 242 - 253, Replace ancestor-level focus tracking with exact target
focus state for every retry destination: in TvPlayerHud.kt lines 242-253 track
the selected tab’s own focus; in TvPlayerHud.kt lines 264-271 track the
picker-return row’s own focus; in TvPlayerScreen.kt lines 2281-2298 track the
selected idle-overlay target’s own focus; and in TvPlayerScreen.kt lines
2657-2669 track the primary Up Next action’s own focus. Update each
corresponding isFocused callback and focus-state handler to use the target’s
exact FocusState rather than a container’s hasFocus.
| var forYouContentHasFocus by remember { mutableStateOf(false) } | ||
| var initialFocusRequested by rememberSaveable { mutableStateOf(false) } | ||
| var lastAppliedFocusRequest by rememberSaveable { mutableStateOf(-1) } | ||
| LaunchedEffect(focusRequest) { | ||
| if (initialFocusRequested && focusRequest == lastAppliedFocusRequest) return@LaunchedEffect | ||
| runCatching { watchlistFocusRequester.requestFocus() } | ||
| onInitialContentFocus() | ||
| // The fifth site where a shell handover was reported regardless of | ||
| // whether the claim landed. onInitialContentFocus() tells the shell | ||
| // content owns focus; saying so after a dropped claim leaves nothing | ||
| // focused and the shell believing otherwise. | ||
| val landed = requestFocusUntilObserved( | ||
| maxAttempts = TvContentInitialFocusMaxAttempts, | ||
| awaitAttempt = { withFrameNanos { } }, | ||
| requestFocus = watchlistFocusRequester::requestFocus, | ||
| isFocused = { forYouContentHasFocus }, | ||
| ) | ||
| if (landed == TvObservedFocusResult.Focused) onInitialContentFocus() |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use target-specific focus state for observed focus retries. Container hasFocus confirms that any descendant owns focus. It does not confirm that the requested target owns focus.
androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/recommendations/TvRecommendationsScreen.kt#L386-L401: observe the Watchlist focus target before callingonInitialContentFocus().androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/requests/TvRequestsScreen.kt#L149-L161: observe the selected result or filter target before ending the retry flow.
📍 Affects 2 files
androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/recommendations/TvRecommendationsScreen.kt#L386-L401(this comment)androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/requests/TvRequestsScreen.kt#L149-L161
🤖 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/recommendations/TvRecommendationsScreen.kt`
around lines 386 - 401, Update the retry observers in TvRecommendationsScreen.kt
(lines 386-401) and TvRequestsScreen.kt (lines 149-161) to verify focus on the
specifically requested target, not a container’s descendant hasFocus state. In
TvRecommendationsScreen, observe the Watchlist focus target before calling
onInitialContentFocus(); in TvRequestsScreen, observe the selected result or
filter target before ending the retry flow.
| // Every focus target on this screen — field, chips, results, | ||
| // request rows, feedback action — lives under here, so "focus is on | ||
| // the search screen" is the arrival each claim below waits on. | ||
| .onFocusChanged { searchScreenHasFocus = it.hasFocus } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
file="androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/search/TvSearchScreen.kt"
printf '%s\n' '--- outline ---'
ast-grep outline "$file" 2>/dev/null || true
printf '%s\n' '--- relevant source ---'
sed -n '200,270p;340,435p;435,485p' "$file"
printf '%s\n' '--- focus helper and related state usages ---'
rg -n -C 4 'requestFocusUntilObserved|searchScreenHasFocus|focusedReturnItemId|restore.*FocusRequester|first.*FocusRequester' androidTvApp/src/main
printf '%s\n' '--- files defining the helper ---'
rg -l 'fun requestFocusUntilObserved|requestFocusUntilObserved' --glob '*.kt' .Repository: Silo-Server/silo-android
Length of output: 1980
🏁 Script executed:
#!/bin/bash
set -e
file="androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/search/TvSearchScreen.kt"
printf '%s\n' '--- relevant source ---'
sed -n '200,270p;340,435p;435,485p' "$file"
printf '%s\n' '--- focus symbols ---'
rg -n -C 4 'requestFocusUntilObserved|searchScreenHasFocus|focusedReturnItemId|restore.*FocusRequester|first.*FocusRequester' androidTvApp/src/main
printf '%s\n' '--- helper definitions ---'
rg -n -C 8 'fun requestFocusUntilObserved|suspend fun requestFocusUntilObserved' --glob '*.kt' .Repository: Silo-Server/silo-android
Length of output: 1980
🌐 Web query:
site:github.com/Silo-Server/silo-android "requestFocusUntilObserved" "TvSearchScreen.kt"
💡 Result:
The search results for the Silo Android codebase do not contain a function, modifier, or utility named requestFocusUntilObserved. While the repository uses Jetpack Compose for TV with D-pad focus management, the provided documentation and commit history do not reference this specific identifier in TvSearchScreen.kt or other components [1][2][3][4]. Focus handling in this codebase is primarily managed through Compose for TV standards [2].
Citations:
- 1: Silo-Server:3f6b7fc...Silo-Server:20e0f8c
- 2: https://github.com/Silo-Server/silo-android
- 3: https://github.com/Silo-Server/silo-android/releases/tag/v1.0.1
- 4: b154777
🌐 Web query:
"searchScreenHasFocus" "TvSearchScreen"
💡 Result:
There is no standard system API, property, or method named "searchScreenHasFocus" in Android TV or tvOS development frameworks [1][2][3]. The term appears to be a descriptive reference rather than a formal programming identifier [4]. In Android TV development, focus management for search screens is typically handled via the Android Leanback library's SearchSupportFragment, which automatically manages focus transitions between the search bar and result lists [1][3]. Developers rarely need to check if a specific "search screen has focus" programmatically; instead, they implement the SearchResultProvider interface to handle query updates and focus movement is handled by the framework [1][3]. If you are experiencing focus issues—such as the focus jumping away from the search bar—it is generally caused by improper use of focus-requesting attributes (like hasTVPreferredFocus) on result items during re-renders [4][5]. In the context of tvOS, specifically when using SwiftUI's.searchable modifier within React Native or Expo, developers often manage focus by ensuring the native search component is not fighting with other focusable elements [6][7]. The community-developed libraries for this, such as expo-tvos-search, handle focus natively and provide event callbacks (e.g., onSearchFieldFocused) to track focus state when necessary [8]. If you are seeing the identifier "searchScreenHasFocus" in your codebase, it is likely a custom variable or state flag implemented by your team to track focus for custom UI logic, rather than a built-in framework feature [4].
Citations:
- 1: https://developer.android.com/training/tv/discovery/in-app-search
- 2: https://developer.android.com/reference/androidx/leanback/widget/SearchEditText
- 3: https://developer.android.com/reference/kotlin/androidx/leanback/app/SearchSupportFragment
- 4: fix(tv): keep focus on search field instead of jumping to results grid streamyfin/streamyfin#1637
- 5: https://github.com/callstackincubator/agent-skills/blob/main/skills/react-native-tv-best-practices/references/focus-management.md
- 6: https://medium.com/@keiver.dev/stop-fighting-tvos-focus-use-swiftui-searchable-from-react-native-expo-module-5971bb0fbdf5
- 7: https://github.com/keiver/expo-tvos-search/tree/v1.2.3
- 8: https://github.com/keiver/expo-tvos-search/blob/guard-focus-restauration/CHANGELOG.md
🌐 Web query:
github Silo-Server silo-android TvSearchScreen.kt searchScreenHasFocus
💡 Result:
In the Silo-Server/silo-android repository, TvSearchScreen.kt is located in the androidTvApp module [1][2], specifically within the ui/screens/search/ directory. While there is no standard public API or widely documented variable named searchScreenHasFocus explicitly associated with that specific file in the repository's main branch, focus management in Compose for TV applications—including those developed with Kotlin Multiplatform like Silo—typically relies on mechanisms such as FocusRequester, focusProperties, and tracking focus state via onFocusChanged or similar modifiers [3][4][5]. Projects in this ecosystem often use shared UI patterns to manage D-pad navigation [1][6]. Logic related to screen focus in search interfaces generally involves: 1. Tracking whether the search field or search results are currently active using state variables [3]. 2. Using FocusRequester to manually shift focus to search results when a query is submitted [5][7]. 3. Configuring navigation boundaries or focus groups to ensure the D-pad behaves correctly when moving between the search bar and the results list [4]. If you are investigating focus issues in this specific codebase, you may find that such state is handled by view models or custom focus-management utilities rather than a single global variable [4]. I recommend searching for usages of FocusRequester or onFocusChanged within that file to locate the specific implementation of focus tracking [3][5].
Citations:
- 1: https://github.com/Silo-Server/silo-android
- 2: Silo-Server:3f6b7fc...Silo-Server:20e0f8c
- 3: https://github.com/android/compose-samples/blob/main/Jetsnack/app/src/main/java/com/example/jetsnack/ui/home/search/Search.kt
- 4: https://github.com/iFan6oy/tv-focus-navigation-kit
- 5: https://github.com/android/compose-samples/blob/main/Jetcaster/tv/src/main/java/com/example/jetcaster/tv/ui/JetcasterApp.kt
- 6: https://siloserver.org/
- 7: kiduyu-klaus/KiduyuTv_final@39d14cc
🌐 Web query:
https://raw.githubusercontent.com/Silo-Server/silo-android/main/androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/search/TvSearchScreen.kt
💡 Result:
The file TvSearchScreen.kt in the Silo-Server/silo-android repository is a component of the native Android TV client for the Silo self-hosted media server [1][2]. The Silo project is a media streaming platform designed for self-hosting that utilizes a Go-based backend and provides native clients for various platforms, including Android phone and Android TV [1][3][4]. The silo-android repository is structured as a Kotlin Multiplatform project [1][5]. Within this repository, the androidTvApp module contains the UI and logic specifically tailored for the "10-foot" TV experience, utilizing Compose for TV (androidx.tv) [1]. TvSearchScreen.kt is responsible for the search interface within the TV application [1]. Consistent with the broader design of the Silo Android TV client—which features D-pad-first navigation, tvOS-aligned top-menu shells, and media-type-aware browsing—this screen provides a search experience that is scoped by media type, debounced, and paginated [1][6]. As part of a Compose-based architecture, it integrates with the app's shared business logic core and uses Koin for dependency injection [1]. The search functionality is intended to allow users to quickly find media across their server libraries, contrasting with manual browsing through large catalogs [1][6].
Citations:
- 1: https://github.com/Silo-Server/silo-android
- 2: https://github.com/Silo-Server
- 3: https://siloserver.org/
- 4: https://github.com/Silo-Server/silo-server
- 5: https://github.com/Silo-Server/silo-android/blob/main/AGENTS.md
- 6: https://github.com/brettpetch/silo-android
🌐 Web query:
"TvFocusAcquisitionBudgetMillis" "TvSearchReturnSettleMillis"
💡 Result:
The terms "TvFocusAcquisitionBudgetMillis" and "TvSearchReturnSettleMillis" do not appear in official Android developer documentation, public Android source code, or widely recognized TV development libraries [1][2][3][4][5][6]. It is highly likely that these are not public API constants, but rather internal implementation-specific variables, custom constants from a third-party framework, or misremembered/proprietary identifiers. Given the naming conventions: - "TvFocusAcquisitionBudgetMillis" suggests a mechanism for managing focus transitions or the time allotted for acquiring focus state in a TV application environment. - "TvSearchReturnSettleMillis" suggests a timing threshold related to search query execution, debouncing, or result display settling. If you encountered these in a specific codebase, they are likely local constants used to tune focus animation timing, debounce user input during search, or handle navigation state transitions within that specific app's architecture [7][8][9]. If you are looking for standardized ways to handle these tasks in Android TV development, you should refer to standard Android framework classes like TVFocusGuideView, SearchFragment, or Jetpack TV components, which handle focus and search timing through established, documented patterns [10][3][11][9].
Citations:
- 1: https://android.googlesource.com/platform/packages/apps/TV/+/master/src/com/android/tv/search/TvProviderSearch.java
- 2: https://android.googlesource.com/platform/packages/apps/TV/+/d12bfa398c56027290a9e6e4fd14f635458ec581/src/com/android/tv/search/LocalSearchProvider.java
- 3: https://developer.android.com/design/ui/tv/guides/styles/focus-system
- 4: https://android.googlesource.com/platform/frameworks/base/+/6baed9e3a272b09e87f15801a389d7714d0b051f/packages/SystemUI/src/com/android/systemui/recents/tv/animations/ViewFocusAnimator.java
- 5: https://android.googlesource.com/platform/packages/apps/TV/+/d12bfa398c56027290a9e6e4fd14f635458ec581/src/com/android/tv/search/SearchInterface.java
- 6: https://android.googlesource.com/platform/packages/apps/TV/+/7e330bd191e3618d718a9ea2cfacb7e2fc3a3f7a/src/com/android/tv/search/LocalSearchProvider.java
- 7: https://github.com/iFan6oy/tv-focus-navigation-kit
- 8: https://github.com/AmazonAppDev/vega-tv-interfaces-sample/blob/main/src/services/focusGuide/README.md
- 9: https://developer.android.com/training/tv/discovery/in-app-search
- 10: https://github.com/callstackincubator/agent-skills/blob/main/skills/react-native-tv-best-practices/references/focus-management.md
- 11: https://developer.android.com/jetpack/androidx/releases/tv?hl=id
🌐 Web query:
Android Jetpack Compose FocusState hasFocus isFocused descendant onFocusChanged documentation
💡 Result:
In Jetpack Compose, FocusState is an interface that provides information about the focus state of a composable component [1][2]. It is accessed through focus-related modifiers like Modifier.onFocusChanged or Modifier.onFocusEvent [1][3]. Key properties of FocusState include: isFocused: A Boolean that indicates whether the specific component to which the modifier is attached is currently focused [1][4]. hasFocus: A Boolean that checks if the component itself OR any of its descendant (child) components are focused [1][4]. This is the primary mechanism for detecting if focus is held within a specific part of the UI hierarchy [4][5]. Modifier.onFocusChanged is used to observe these states [6][7]. When added to a component, the callback is invoked whenever the focus state of the associated focus target changes [6][7]. It is best practice to place the onFocusChanged modifier before the focusable or focusTarget modifier in the modifier chain to ensure it correctly observes the focus events [6][8][9]. Example Usage: Box( Modifier.onFocusChanged { focusState -> val isFocused = focusState.isFocused val hasFocus = focusState.hasFocus // Handle focus state changes }.focusable) { /* content */ } For scenarios where you need to be notified every time a focus event occurs—even if the overall state has not changed—the onFocusEvent modifier can be used instead [6][7].
Citations:
- 1: https://developer.android.com/reference/kotlin/androidx/compose/ui/focus/FocusState
- 2: https://composables.com/jetpack-compose/androidx.compose.ui/ui/interfaces/FocusState/api
- 3: https://composables.com/docs/androidx.compose.ui/ui/interfaces/FocusState
- 4: https://developer.android.com/develop/ui/compose/touch-input/focus/react-to-focus
- 5: https://developer.android.google.cn/develop/ui/compose/touch-input/focus/react-to-focus
- 6: https://developer.android.com/reference/kotlin/androidx/compose/ui/focus/onFocusChanged.modifier
- 7: https://developer.android.com/reference/kotlin/androidx/compose/ui/focus/package-summary
- 8: https://developer.android.com/develop/ui/compose/touch-input/focus/change-focus-behavior
- 9: https://developer.android.com/reference/kotlin/androidx/compose/ui/focus/FocusRequesterModifier
Track each requested Search target instead of screen-wide focus.
searchScreenHasFocus uses FocusState.hasFocus, which is true when any descendant is focused. A retry can therefore succeed while the requested target is not focused. Track each target, including the restored card ID, before completing focus requests. Handle the return-observation timeout explicitly.
📍 Affects 1 file
androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/search/TvSearchScreen.kt#L467-L470(this comment)androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/search/TvSearchScreen.kt#L245-L250androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/search/TvSearchScreen.kt#L376-L404androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/search/TvSearchScreen.kt#L419-L424androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/search/TvSearchScreen.kt#L442-L453
🤖 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/search/TvSearchScreen.kt`
around lines 467 - 470, Replace screen-wide focus tracking with per-target focus
state in TvSearchScreen.kt: update the focus modifier at lines 467-470 and the
target request/restore logic at lines 245-250, 376-404, 419-424, and 442-453 to
track the requested field, chip, result, request row, feedback action, and
restored card ID before completing each request. Ensure completion only occurs
when the specific requested target is focused, and handle return-observation
timeouts explicitly.
| var crashRowHasFocus by remember { mutableStateOf(false) } | ||
| LaunchedEffect(state.consent) { | ||
| val target = initialTvDiagnosticsCrashFocus(state.consent) | ||
| // Relocation, not acquisition: the page is already focusable, so a | ||
| // miss just leaves focus wherever the route transition put it. | ||
| repeat(TvFrameRelocationMaxAttempts) { | ||
| withFrameNanos { } | ||
| when ( | ||
| tvDiagnosticsCrashFocusRequestResult( | ||
| runCatching { crashFocusRequesters.getValue(target).requestFocus() }, | ||
| ) | ||
| ) { | ||
| TvDiagnosticsCrashFocusRequestResult.FOCUSED -> return@LaunchedEffect | ||
| TvDiagnosticsCrashFocusRequestResult.RETRY -> Unit | ||
| } | ||
| } | ||
| // tvDiagnosticsCrashFocusRequestResult mapped a Result, so "did not | ||
| // throw" counted as FOCUSED and the loop stopped on acceptance rather | ||
| // than on arrival. | ||
| requestFocusUntilObserved( | ||
| maxAttempts = TvFrameRelocationMaxAttempts, | ||
| awaitAttempt = { withFrameNanos { } }, | ||
| requestFocus = crashFocusRequesters.getValue(target)::requestFocus, | ||
| isFocused = { crashRowHasFocus }, | ||
| ) | ||
| } | ||
| val model = tvDiagnosticsScreenModel(state) | ||
| fun Modifier.crashFocusControl(current: TvDiagnosticsCrashFocus): Modifier = | ||
| focusRequester(crashFocusRequesters.getValue(current)) | ||
| .onFocusChanged { crashRowHasFocus = it.isFocused || crashRowHasFocus } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
file='androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/settings/diagnostics/TvDiagnosticsSettingsScreen.kt'
printf '%s\n' '--- outline ---'
ast-grep outline "$file" || true
printf '%s\n' '--- relevant source ---'
sed -n '1,180p' "$file"
printf '%s\n' '--- focus symbols and enum usages ---'
rg -n -C 3 'TvDiagnosticsCrashFocus|crashFocusControl|requestFocusUntilObserved|crashRowHasFocus|focusedCrashRow' androidTvAppRepository: Silo-Server/silo-android
Length of output: 1980
🏁 Script executed:
#!/bin/bash
set -e
base='https://api.github.com/repos/Silo-Server/silo-android'
file='androidTvApp/src/androidMain/kotlin/org/siloserver/silo/tv/ui/screens/settings/diagnostics/TvDiagnosticsSettingsScreen.kt'
printf '%s\n' '--- repository metadata ---'
curl -fsSL "$base" | jq '{default_branch, pushed_at}'
printf '%s\n' '--- target file ---'
curl -fsSL "$base/contents/$file" | jq -r '.content' | tr -d '\n' | base64 -d | sed -n '1,180p'
printf '%s\n' '--- repository search ---'
curl -fsSLG "$base/code/search" \
--data-urlencode 'q=TvDiagnosticsCrashFocus repo:Silo-Server/silo-android' |
jq -r '.items[]? | [.path, .html_url] | `@tsv`'Repository: Silo-Server/silo-android
Length of output: 1980
🌐 Web query:
site:github.com/Silo-Server/silo-android "TvDiagnosticsSettingsScreen.kt" "crashRowHasFocus"
💡 Result:
There is no public record of a file named TvDiagnosticsSettingsScreen.kt or a variable named crashRowHasFocus in the identified codebase. The search results for the Silo-Server/silo-android project [1] indicate that the project includes Android TV diagnostics features, including local reports for crashes and focus events [1], but these specific identifiers do not appear in the available documentation or commit history [2][3][4][5]. The project does contain settings screens and focus-related fixes, such as addressing D-pad and menu focus behavior, but these are associated with different files and commit messages [2].
Citations:
- 1: https://github.com/Silo-Server/silo-android
- 2: Silo-Server:11e89b1...Silo-Server:20e0f8c
- 3: Silo-Server:3f6b7fc...Silo-Server:20e0f8c
- 4: Silo-Server:20e0f8c...Silo-Server:5dbadc2
- 5: 5dbadc2
Track the focused diagnostics row by identity.
crashRowHasFocus remains true after focus loss. A later consent change can therefore stop relocation before the new target receives focus. Store the focused TvDiagnosticsCrashFocus value and compare it with target.
🤖 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/diagnostics/TvDiagnosticsSettingsScreen.kt`
around lines 75 - 93, Replace the sticky crashRowHasFocus boolean with state
tracking the currently focused TvDiagnosticsCrashFocus identity. Update
crashFocusControl to assign the row’s current value on focus changes, and make
requestFocusUntilObserved in the LaunchedEffect for state.consent succeed only
when the tracked value equals target.
Silo-Server#202 removed the two TvDiagnosticsPromptScreen claims this baseline was holding open, so the ratchet reports 0 and the constant follows it. Every TV screen is now free of silent focus claims, and any new one fails the build. Verified: :androidTvApp:testDebugUnitTest 976 tests, 0 failures. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ad path Three review findings, all re-confirmed on the new main. Back could strand focus nowhere. On a loading or empty rail the content group has no focusable child, so claimContentFocus returned false, the panel it had just made invisible could no longer take focus, and the D-pad went dead. The claim's return value was being treated as proof of arrival, which is exactly what Silo-Server#208's ratchet exists to stop. moveFocusToContent now uses requestFocusUntilObserved against contentHasFocus, and when content genuinely has nothing to focus the shell puts focus back on the bar with dwell suppressed and reports the failed handoff. The shell dismantles the previous focus owner, so the shell owes a real successor — a loading state should not have to invent a focusable control to satisfy shell navigation. A dwell PREVIEW was routed as an entered cascade. Resting on a tab long enough to open the preview, without pressing Down, left focus on the bar but openPanel non-null; Back then closed it AND threw the viewer into content from a menu they were still browsing, costing them the trip back up to reach Home. tvShellBackAction now takes panelEntered and returns ClosePanelPreview, which dismisses the preview and moves nothing. barFocusFromPanelClose / MoveFocusToContent were unreachable from production: the flag is set only by closePanel(true), whose sole production caller was TvCascadeSelector.onClose, and the selector never invokes it — Back is centralised in the shell, deliberately, and more so under Android 16 callback ordering. Deleted rather than wired up; adding a competing Back handler inside an always-composed selector would risk double consumption. closePanel now takes no argument and never moves focus. Mutation-checked: removing the preview branch fails both new routing tests. androidTvApp 996, all green, debug APK assembles. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…exit (#204) * fix(tv): cascade reachability and Back out of the chrome Carried onto the telemetry branch so testers exercise the fixes, not just the instrumentation. Three changes, all in the top-bar focus model: - menuFocusTarget was doing two jobs: naming which bar element to land on, and marking that element's dwell preview suppressed. An ordinary content-to-bar Up carries a target too, so every trip up from content armed the Back-close suppression and left that tab unable to reopen its own cascade. Only closePanel() sets the new menuFocusSuppressesDwell flag. - An earlier attempt expired the suppression on a 600 ms timer. That fixed the symptom and broke the cause: a Back-close would reopen the panel once the timer elapsed. Removed, now that suppression is armed only where it belongs. - Back from a bar that holds focus solely because a cascade was just dismissed now returns to content instead of walking the viewer along the bar to Home. Gated on barFocusFromPanelClose, which clears as soon as focus leaves the bar, so an ordinary bar Back keeps the QA back-stack model. Verified: :androidTvApp:assembleDebug, :androidTvApp:lintDebug. Behavioural and timing-dependent, so not unit tested; the existing tvShellBackAction tests still compile and still assert the unchanged MenuBack path via the new parameter's default. Unproven on a device beyond the Shield — which is why it ships on the build that now reports what the focus model actually did. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> (cherry picked from commit e6e738a4919b34c60c45dc557528fee9c9131ac8) * fix(tv): Back out of a cascade returns to content Back inside a cascade closed the panel and parked focus on the tab above it, so leaving the chrome took three presses and the middle one navigated Home. "Back doesn't exit the menus" was an accurate description of the model. onBack() now closes the panel without claiming focus and the shell moves focus to content in the same press. The panel's own close action still returns to the bar, which is where that belongs. TvShellFocusStateTest asserted the old contract (bar nudged on a panel Back); updated to assert the opposite, since not claiming focus is the point. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> (cherry picked from commit 62d980e13b00c71aacee7628ebdf6e5f77c2f66b) * fix(tv): stop Back ping-ponging so Home and exit stay reachable Confirmed from a trace rather than inferred. Back from content climbed to the bar, the tab's cascade opened by dwell on arrival, and the next Back saw an open panel and closed it straight back to content: back root_panel close -> content focus back root_panel request -> menu focused -> root_panel preview back root_panel close -> content focus Two presses, no progress, MenuBack never reached — so Home and exit were unreachable by Back at all. Mine: when suppression was narrowed to closePanel() only, it was also stripped from the Back-from-content path, where it was doing real work. The distinction that matters is not which call site but why focus arrived: Up from content = browsing -> the cascade should open Back from content = leaving -> it must not Both carry a target; only the intent differs. MoveFocusToMenu now suppresses. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> (cherry picked from commit b44e1d8f6405eea588a3d1bffc67d3525d7bd938) * fix(tv): observe the content handoff, split preview Back, drop the dead path Three review findings, all re-confirmed on the new main. Back could strand focus nowhere. On a loading or empty rail the content group has no focusable child, so claimContentFocus returned false, the panel it had just made invisible could no longer take focus, and the D-pad went dead. The claim's return value was being treated as proof of arrival, which is exactly what #208's ratchet exists to stop. moveFocusToContent now uses requestFocusUntilObserved against contentHasFocus, and when content genuinely has nothing to focus the shell puts focus back on the bar with dwell suppressed and reports the failed handoff. The shell dismantles the previous focus owner, so the shell owes a real successor — a loading state should not have to invent a focusable control to satisfy shell navigation. A dwell PREVIEW was routed as an entered cascade. Resting on a tab long enough to open the preview, without pressing Down, left focus on the bar but openPanel non-null; Back then closed it AND threw the viewer into content from a menu they were still browsing, costing them the trip back up to reach Home. tvShellBackAction now takes panelEntered and returns ClosePanelPreview, which dismisses the preview and moves nothing. barFocusFromPanelClose / MoveFocusToContent were unreachable from production: the flag is set only by closePanel(true), whose sole production caller was TvCascadeSelector.onClose, and the selector never invokes it — Back is centralised in the shell, deliberately, and more so under Android 16 callback ordering. Deleted rather than wired up; adding a competing Back handler inside an always-composed selector would risk double consumption. closePanel now takes no argument and never moves focus. Mutation-checked: removing the preview branch fails both new routing tests. androidTvApp 996, all green, debug APK assembles. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(tv): route Back on observed panel focus, not entry intent Follow-up review of the previous commit found four issues with the fixes themselves. panelEntersFocus is INTENT, set before the selector's asynchronous focus request runs. An empty panel, an unattached requester or a silently failed claim all leave focus on the bar while that flag says otherwise, so Back classified them as entered and threw the viewer into content from a bar they never left. Routing now asks panelHasFocus, reported by the selector as its rows and pills gain and lose focus. That signal already existed and was wired to a no-op comment in the shell — the same class of mistake the branch is fixing elsewhere. The observed handoff could yank focus off content that had just taken it: the final attempt inspects focus in the same frame it requested it, so an accepted claim reporting asynchronously looked like failure. It now waits one more frame and re-checks before falling back to the bar. The bar fallback passed no target, so menuFocusSuppressesDwell had no button to suppress and the tab it focused could reopen its preview a moment later. It now passes the selected target. Also corrects comments in three files that still described the deleted panel-close focus behaviour. Mutation-checked earlier work still holds; two existing tests had to be updated because they asserted on intent, which is exactly the bug. androidTvApp 998, all green, debug APK assembles. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: rxwatcher <rxwatcher@users.noreply.github.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…r#208 invalidated Pre-merge gate caught three internal contradictions, and checking them against main showed the problem was larger than arithmetic. The adoption section was the note's central argument: 8 adopters against 44 direct callers, 18%, 107 runCatching occurrences, therefore "enforcement first, migration second, shell model third". Measured on main at 7245c0f that is no longer true. Silo-Server#208's sweep of 78 sites and its ratchet have taken adoption to 34 files against 24 with a raw requestFocus() — 9 of which use both — so 69%, with 15 genuine hold-outs and exactly ONE runCatching still wrapping a requestFocus. So the note now says that plainly and retracts its own ordering: enforcement exists and adoption followed it, and what remains is the part a sweep cannot do — the shell still infers ownership from twelve flags, and Silo-Server#204's panelHasFocus is the newest instance of a type distinction expressed as a boolean pair. Also corrected: the monthly table (22 and 48, not 26 and 49) and the 90-day split (49 fix, 7 feat of 81) now state their method so they can be re-derived; the field list was missing panelHasFocus and menuFocusSuppressesDwell, which contradicted the Silo-Server#204 paragraph immediately below it; and 49 was being used for three different quantities in the metrics section. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… the old order The previous commit retracted the enforcement-first ordering in one section and left three others prescribing it: Enforcement still proposed adding the source test, Scope still listed the gate as in-scope and ordered migration before the shell model, and Out still said the observed-focus policy's problem is that it is not used — which contradicts the 69% adoption documented two sections above. Enforcement now records that Silo-Server#208 shipped exactly the proposed gate and that this note proposes none. Scope leads with the shell ownership model, since that is the part a sweep cannot do, and lists the gate as out. Out drops the not-used claim. The adoption, hold-out and runCatching figures now state the greps that produce them, as the commit-subject counts already did. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Round 3 found the retraction still contradicted itself and that most figures could not be re-derived. 'Focus entry is a shell concern' asks for a restorer-placement source test, which read as a contradiction of 'no new enforcement'. They are different checks: Silo-Server#208's ratchet catches silent focus claims, the restorer rule is about where a focusRestorer may live in the composition tree. Both statements now say which check they mean. Every empirical figure now carries the command that produces it, and the churn tables are re-measured rather than inherited: file churn 14/9/7/7/6 and directory churn 126/34/14/16, both counted as focus-SUBJECT commits since 2026-05-13. The month table's method now shows a concrete dated window instead of an elided --since, and the runCatching figures state their grep. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 participant
Why
requestFocus()throws when its node has not attached yet, rather than returning false. SorunCatching { requestFocus() }does not handle that failure — it hides it. Focus goes nowhere, no exception surfaces, nothing is logged, and a leanback app has no touch fallback to recover with.That is cause #1 of
2026-08-04-whole-application-focus-hardening-design.md— "a focus request executing without exception is treated as focus acquisition" — and it is not a backlog being worked off. It is still the majority of the code:requestFocus()directlyrunCatching { … requestFocus() … }sites remain in TV screens18% adoption. It is also the exact mechanism behind #199 (content focus entry found nothing to focus) and #202 (the crash-report prompt was unreachable, so crash reports have never been sendable from a television).
Meanwhile focus fixes are accelerating, not converging:
main49 of the last 90 days' 83 are
fix:, against 9feat:.A better rule that 18% of the code follows is worth less than the existing rule made impossible to violate. So this makes the next instance fail the build.
What it does
Walks
ui/screens/, countsrunCatchingoccurrences whose following 220 characters containrequestFocus(, and asserts the count.It does not fix the 78 existing sites — it stops the 79th, while they are migrated in churn order (player 10, detail 8, settings 7, recommendations 7, calendar 6, auth 6, library 6, search 6 …).
Equality rather than
<=, on purpose. A<=ratchet leaves slack that the next silent claim quietly fills. Migrating a site means loweringBASELINEin the same commit — and the failure message tells you that, with the current site list.Source tests that read files by path and assert on structure are an existing convention in this repo (
TvPictureInPictureSourceTest,TvHudPickerFocusWiringSourceTest, and others), so this is not a new mechanism.Verification
Both directions, because a ratchet that cannot fail is decoration:
runCatching { requester.requestFocus() }into a screen → BUILD FAILED with the explanatory messageTest-only; no production code changes.
Summary by CodeRabbit
Bug Fixes
Tests