feat: improve on-device coach quality and orbiting shell FAB - #102
Conversation
Raise local model output budgets and add continuation when responses truncate mid-sentence. Use tier-aware prompts, structured output format, priority-aware context truncation, and skip agentic tool loops on constrained devices. Refactor shell FAB to orbit menu items in an upper-right arc, lift it above the coach input bar, and inset the chat text field from the left. Co-authored-by: Youri Bontekoe <YKDBontekoe@users.noreply.github.com>
|
Warning Review limit reached
Next review available in: 51 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughCoach chat now uses tier-aware prompt and response handling, including continuation for truncated Gemma output and conditional agentic tool loops. Floating navigation and coach input receive inset-aware layout updates, with supporting tests and refreshed code-map metadata. ChangesCoach inference and UI
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CoachChat
participant IsolateAiCoachRepository
participant AiIsolateEntrypoint
participant GemmaRuntime
CoachChat->>IsolateAiCoachRepository: submit tier-aware coach prompt
IsolateAiCoachRepository->>AiIsolateEntrypoint: send AiChatRequest with output budget
AiIsolateEntrypoint->>GemmaRuntime: stream response
GemmaRuntime-->>AiIsolateEntrypoint: response chunks
AiIsolateEntrypoint-->>CoachChat: forward response chunks
AiIsolateEntrypoint->>GemmaRuntime: request continuation when response appears truncated
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
Co-authored-by: Youri Bontekoe <YKDBontekoe@users.noreply.github.com>
Co-authored-by: Youri Bontekoe <YKDBontekoe@users.noreply.github.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: 1
🧹 Nitpick comments (3)
test/features/coach_chat/coach_chat_agentic_tool_test.dart (1)
40-42: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a test for the constrained-tier tool-loop-disabled path.
All tests override
gemmaInferenceTierProvidertoGemmaInferenceTier.full, which enables the agentic tool loop. The PR objective explicitly adds "skipped agentic tool loops on constrained devices," but no test verifies thatGemmaInferenceTier.constraineddisables the loop. A modelTOOL_CALLresponse on a constrained device should degrade to a direct answer without executing tools.🧪 Suggested constrained-tier test
test('skips tool loop on constrained tier and answers directly', () async { final fakeAi = _ScriptedAgenticAiCoachRepository( scriptedTurns: [ 'TOOL_CALL: {"name":"get_recent_runs","arguments":{"limit":2}}', ], ); final prefs = await SharedPreferences.getInstance(); final container = ProviderContainer( overrides: [ sharedPreferencesProvider.overrideWithValue(prefs), healthRepositoryProvider.overrideWithValue(_FakeHealthRepository()), chatAiCoachRepositoryProvider.overrideWithValue(fakeAi), gemmaInferenceTierProvider.overrideWith( (ref) async => GemmaInferenceTier.constrained, ), ], ); await container .read(coachConversationsProvider.notifier) .ensureActiveConversation(); await container.read(coachChatProvider.future); addTearDown(container.dispose); await container .read(coachChatProvider.notifier) .sendMessage('How is my training?'); final assistant = container.read(coachChatProvider).value!.last; // Tool loop is disabled on constrained tier — no tool steps should execute. expect(assistant.toolSteps, isNull); });🤖 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 `@test/features/coach_chat/coach_chat_agentic_tool_test.dart` around lines 40 - 42, Add a test in the coach chat agentic tool tests that overrides gemmaInferenceTierProvider to GemmaInferenceTier.constrained, supplies a scripted TOOL_CALL response, sends a user message, and verifies the assistant returns directly without tool steps or tool execution. Keep the existing full-tier tests unchanged and dispose the ProviderContainer after the test.test/domain/utils/coach_prompt_truncator_test.dart (1)
1-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd edge-case tests for uncovered branches.
The test file covers the happy paths but misses several branches in
truncateCoachPromptandcoachResponseLooksTruncated:
truncateCoachPromptwhen the question marker is absent (falls back to raw substring).truncateCoachPromptwhen the question block itself exceedsmaxChars(returns truncated question).coachResponseLooksTruncatedwith…(Unicode ellipsis) — the only reachable check on line 43 of the truncator.These are simple additions that would lock in the fallback behavior.
🧪 Suggested additional tests
test('truncates to maxChars when question marker is absent', () { final prompt = 'x' * 3000; final truncated = truncateCoachPrompt(prompt); expect(truncated.length, lessThanOrEqualTo(GemmaInferenceLimits.maxPromptCharacters)); expect(truncated, isNot(contains('Person'))); }); test('truncates question block itself when it exceeds maxChars', () { final longQuestion = 'Person’s question: ' + 'x' * 3000; final truncated = truncateCoachPrompt(longQuestion); expect(truncated.length, lessThanOrEqualTo(GemmaInferenceLimits.maxPromptCharacters)); }); test('returns false for Unicode ellipsis ending', () { expect( coachResponseLooksTruncated( 'Your readiness is low because ' * 30 + '…', maxOutputTokens: 256, ), isFalse, ); });🤖 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 `@test/domain/utils/coach_prompt_truncator_test.dart` around lines 1 - 46, Add edge-case tests in the existing truncateCoachPrompt and coachResponseLooksTruncated groups: verify prompts without the question marker use raw max-length truncation, oversized question blocks are truncated to GemmaInferenceLimits.maxPromptCharacters, and a response ending with the Unicode ellipsis (… ) is not classified as truncated. Preserve the existing happy-path tests.lib/shared/providers/gemma_tier_provider.dart (1)
1-8: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider generating this provider with
@riverpodfor convention consistency.As per coding guidelines, "generate providers with
@riverpodplus build_runner" and "use AsyncNotifierProvider for async state." This hand-writtenFutureProviderworks correctly but deviates from the project's provider generation convention. For a simple read-only probe this may be acceptable, but if consistency matters, wrapping it as an@riverpod-generatedAsyncNotifierProviderwould align with the rest of the codebase.🤖 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 `@lib/shared/providers/gemma_tier_provider.dart` around lines 1 - 8, Replace the hand-written gemmaInferenceTierProvider FutureProvider with an `@riverpod-generated` provider using the project’s AsyncNotifier convention, while preserving GemmaRuntimeTier.resolve() as the async value source. Add the required generator annotation/base implementation and ensure generated provider output is produced through build_runner.Source: Coding guidelines
🤖 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 `@lib/features/coach_chat/providers/coach_chat_provider.dart`:
- Around line 246-252: The cached gemmaInferenceTierProvider can make
enableToolLoop use a stale tier; invalidate it before reading at the inference
call site in lib/features/coach_chat/providers/coach_chat_provider.dart lines
246-252, then await the refreshed value before computing enableToolLoop. No
direct change is required in lib/shared/providers/gemma_tier_provider.dart lines
1-8; its existing FutureProvider behavior supports re-resolution after
invalidation.
---
Nitpick comments:
In `@lib/shared/providers/gemma_tier_provider.dart`:
- Around line 1-8: Replace the hand-written gemmaInferenceTierProvider
FutureProvider with an `@riverpod-generated` provider using the project’s
AsyncNotifier convention, while preserving GemmaRuntimeTier.resolve() as the
async value source. Add the required generator annotation/base implementation
and ensure generated provider output is produced through build_runner.
In `@test/domain/utils/coach_prompt_truncator_test.dart`:
- Around line 1-46: Add edge-case tests in the existing truncateCoachPrompt and
coachResponseLooksTruncated groups: verify prompts without the question marker
use raw max-length truncation, oversized question blocks are truncated to
GemmaInferenceLimits.maxPromptCharacters, and a response ending with the Unicode
ellipsis (… ) is not classified as truncated. Preserve the existing happy-path
tests.
In `@test/features/coach_chat/coach_chat_agentic_tool_test.dart`:
- Around line 40-42: Add a test in the coach chat agentic tool tests that
overrides gemmaInferenceTierProvider to GemmaInferenceTier.constrained, supplies
a scripted TOOL_CALL response, sends a user message, and verifies the assistant
returns directly without tool steps or tool execution. Keep the existing
full-tier tests unchanged and dispose the ProviderContainer after the test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 39eda044-423c-4a04-8fc0-a55e10505a48
📒 Files selected for processing (19)
CODEMAP.mdlib/app/shell_page.dartlib/core/theme/layout.dartlib/domain/utils/coach_context_formatter.dartlib/domain/utils/coach_prompt_truncator.dartlib/domain/utils/gemma_inference_limits.dartlib/features/coach_chat/presentation/pages/coach_chat_page.dartlib/features/coach_chat/presentation/widgets/chat_input_bar.dartlib/features/coach_chat/providers/coach_chat_provider.dartlib/features/coach_chat/providers/coach_chat_provider.g.dartlib/infrastructure/ai/gemma/ai_isolate_entrypoint.dartlib/infrastructure/ai/gemma/ai_isolate_messages.dartlib/infrastructure/ai/gemma/coach_prompt_builder.dartlib/infrastructure/ai/gemma/gemma_inference_session.dartlib/infrastructure/ai/isolate_ai_coach_repository.dartlib/shared/providers/gemma_tier_provider.dartlib/shared/widgets/kynos_floating_nav.darttest/domain/utils/coach_prompt_truncator_test.darttest/features/coach_chat/coach_chat_agentic_tool_test.dart
Invalidate gemmaInferenceTierProvider on each coach message so thermal throttling cannot leave the agentic loop enabled on a constrained tier. Add a constrained-tier regression test. Co-authored-by: Youri Bontekoe <YKDBontekoe@users.noreply.github.com>
Co-authored-by: Youri Bontekoe <YKDBontekoe@users.noreply.github.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/features/coach_chat/coach_chat_agentic_tool_test.dart (1)
40-42: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConstrained-tier test logic is correct; container setup duplicates
buildContainer.The new constrained test correctly validates the tier-gated tool-loop path: with
GemmaInferenceTier.constrained,enableToolLoopisfalse,maxStepsbecomes0, theTOOL_CALLis stripped (leaving empty content), and the resultingStateErrorproduceshasError: truewith a single captured user message. All assertions align with the provider logic incoach_chat_provider.dart.However, lines 115–130 duplicate the entire
buildContainersetup with only the tier value differing. Consider parameterizingbuildContainerto accept an optional tier (defaulting toGemmaInferenceTier.full) to eliminate the duplication and keep future override changes in one place.♻️ Proposed refactor: parameterize buildContainer
Future<ProviderContainer> buildContainer( _ScriptedAgenticAiCoachRepository fakeAi, { - GemmaInferenceTier tier = GemmaInferenceTier.full, + GemmaInferenceTier tier = GemmaInferenceTier.full, }) async { final prefs = await SharedPreferences.getInstance(); final container = ProviderContainer( overrides: [ sharedPreferencesProvider.overrideWithValue(prefs), healthRepositoryProvider.overrideWithValue(_FakeHealthRepository()), chatAiCoachRepositoryProvider.overrideWithValue(fakeAi), gemmaInferenceTierProvider.overrideWith( - (ref) async => GemmaInferenceTier.full, + (ref) async => tier, ), ], );Then the constrained test becomes:
- final prefs = await SharedPreferences.getInstance(); - final container = ProviderContainer( - overrides: [ - sharedPreferencesProvider.overrideWithValue(prefs), - healthRepositoryProvider.overrideWithValue(_FakeHealthRepository()), - chatAiCoachRepositoryProvider.overrideWithValue(fakeAi), - gemmaInferenceTierProvider.overrideWith( - (ref) async => GemmaInferenceTier.constrained, - ), - ], - ); - addTearDown(container.dispose); - await container - .read(coachConversationsProvider.notifier) - .ensureActiveConversation(); - await container.read(coachChatProvider.future); + final container = await buildContainer( + fakeAi, + tier: GemmaInferenceTier.constrained, + ); + addTearDown(container.dispose);Also applies to: 109-142
🤖 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 `@test/features/coach_chat/coach_chat_agentic_tool_test.dart` around lines 40 - 42, Parameterize the existing buildContainer helper to accept an optional GemmaInferenceTier argument defaulting to GemmaInferenceTier.full, while keeping its shared provider overrides and setup unchanged. Update the constrained-tier test to pass GemmaInferenceTier.constrained instead of duplicating the container construction, and preserve the existing full-tier callers through the default.
🤖 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.
Nitpick comments:
In `@test/features/coach_chat/coach_chat_agentic_tool_test.dart`:
- Around line 40-42: Parameterize the existing buildContainer helper to accept
an optional GemmaInferenceTier argument defaulting to GemmaInferenceTier.full,
while keeping its shared provider overrides and setup unchanged. Update the
constrained-tier test to pass GemmaInferenceTier.constrained instead of
duplicating the container construction, and preserve the existing full-tier
callers through the default.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d7cf752d-b9c4-4dca-ba28-8847b1a3d25d
📒 Files selected for processing (2)
lib/features/coach_chat/providers/coach_chat_provider.darttest/features/coach_chat/coach_chat_agentic_tool_test.dart
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/features/coach_chat/providers/coach_chat_provider.dart
Co-authored-by: Youri Bontekoe <YKDBontekoe@users.noreply.github.com>
# [1.22.0](v1.21.0...v1.22.0) (2026-07-13) ### Features * improve on-device coach quality and orbiting shell FAB ([#102](#102)) ([f9d4c87](f9d4c87))
2 participants
Summary
Addresses two coach UX issues: weak/truncated on-device model replies and awkward shell FAB placement/interaction.
On-device coach quality
SIGNALS / ANSWER / ACTIONscaffolding helps small models stay completeShell FAB
bottomInsetValidation
flutter analyze— cleanflutter test— 257 passingflutter build web— succeedscoach-fab-orbit-demo.mp4
To show artifacts inline, enable in settings.
Summary by CodeRabbit
New Features
Bug Fixes
Tests