feat: onboarding turns on automatic inference for the areas it creates#3574
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughOnboarding-created categories now enable automatic inference, while ordinary creation remains unset. Reused categories preserve explicit opt-outs. Reconnecting providers clears seeded-profile tombstones so bundled profiles can be restored. Tests, documentation, release metadata, and versioning were updated. ChangesOnboarding setup behavior
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant OnboardingWelcomeModal
participant CategoryRepository
participant OnboardingApiKeyPanel
participant SeedTombstoneStore
participant ProviderSetup
User->>OnboardingWelcomeModal: Continue onboarding
OnboardingWelcomeModal->>CategoryRepository: Create or update category automation setting
CategoryRepository->>CategoryRepository: Persist automaticInferenceEnabled
User->>OnboardingApiKeyPanel: Connect provider
OnboardingApiKeyPanel->>SeedTombstoneStore: Clear revived profile tombstone
OnboardingApiKeyPanel->>ProviderSetup: Run FTUE setup
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3574 +/- ##
=======================================
Coverage 99.14% 99.14%
=======================================
Files 1782 1783 +1
Lines 130431 130441 +10
=======================================
+ Hits 129319 129329 +10
Misses 1112 1112
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Automatic transcription and image analysis are opt-in per category and default to off. Onboarding, though, binds a seeded profile and Laura to every area it creates and then has the user record a first capture — so the flow demonstrated speech becoming a task while the same gesture the next day would do nothing until the user found the category switch. Onboarding's first capture itself is unaffected either way: it runs through CaptureController -> AudioTranscriptionService, an explicit user action that never passed through ProfileAutomationService. What breaks without this is everything after onboarding, where task-linked recordings go through RecorderController -> AutomaticPromptTrigger -> tryTranscribe, which is gated. Connecting a provider, choosing areas, and recording on purpose is the consent the gate asks for, so areas created in that flow start with automation on. Every other creation path still leaves the preference null (off). Reuse is the one case where the user may already have answered: onboarding fills in a null but leaves an explicit false alone, so the flow cannot become a back door around the switch.
Follow-up to the seed tombstones merged in #3573, which introduced this gap. Onboarding binds each area to a well-known seeded profile id unconditionally. Once deletions are remembered, a user who had deleted that bundled profile and later re-ran onboarding got a category pointing at a row seeding now refuses to recreate: _resolveModel returns noModel and the first task degrades to title-only. Onboarding is re-runnable from the maintenance page and the onboarding settings panel, so this is reachable rather than theoretical. The key step now clears that provider's tombstone before running the FTUE setup, so the seeding it triggers can recreate the profile. Deliberately setting the provider up again is the 'bring it back' signal the store's forget was built for; every other path still honors the deletion. Exposes the store as seedTombstoneStoreProvider rather than reaching into getIt from a widget, which also lets tests substitute one. Also fixes a defect in the tombstone serialization from that same PR: the mutation queue was a single static, so it was shared across every database in the isolate. In tests one stalled mutation against a discarded database wedged every later write, and the onboarding flow hung mid-connect. The queue is now keyed by settings database, which is both what the serialization actually needs and test-safe.
f669a93 to
896509a
Compare
|
Rebased onto main now that #3573 has merged, renumbered to What #3573 broke. Onboarding binds each area to a well-known seeded profile id unconditionally ( A defect in #3573 itself, found while testing this. The tombstone mutation queue was a single static shared across every database in the isolate. In tests, one stalled mutation against a discarded database wedged every later write — the onboarding flow hung mid-connect, which is how I found it. The queue is now keyed by settings database, which is both what the serialization actually needs (two different databases have nothing to serialize against) and test-safe. Worth knowing that the version merged in #3573 has that flaw. Also exposes the store as Verification: |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/features/onboarding/ui/onboarding_animation_gallery_page_test.dart (1)
6-17: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRestore the
Overrideimports used by mock helpers.Both tests type
extraOverridesasList<Override>and expand provider overrides before running them. Addimport 'package:flutter_riverpod/misc.dart' show Override;so the test files resolve before the analyzer is run.
test/features/onboarding/ui/onboarding_animation_gallery_page_test.darttest/features/onboarding/ui/widgets/onboarding_api_key_panel_test.dart🤖 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/onboarding/ui/onboarding_animation_gallery_page_test.dart` around lines 6 - 17, Add the flutter_riverpod misc import exposing Override in both test/features/onboarding/ui/onboarding_animation_gallery_page_test.dart (lines 6-17) and test/features/onboarding/ui/widgets/onboarding_api_key_panel_test.dart (lines 10-22), so their extraOverrides declarations and provider override expansion resolve correctly.Sources: Coding guidelines, Learnings
🤖 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.
Outside diff comments:
In `@test/features/onboarding/ui/onboarding_animation_gallery_page_test.dart`:
- Around line 6-17: Add the flutter_riverpod misc import exposing Override in
both test/features/onboarding/ui/onboarding_animation_gallery_page_test.dart
(lines 6-17) and
test/features/onboarding/ui/widgets/onboarding_api_key_panel_test.dart (lines
10-22), so their extraOverrides declarations and provider override expansion
resolve correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 750564b3-c312-42ad-a702-fa98c722b5b5
📒 Files selected for processing (15)
CHANGELOG.mdflatpak/com.matthiasn.lotti.metainfo.xmllib/features/ai/state/seed_tombstone_provider.dartlib/features/ai/util/seed_tombstone_store.dartlib/features/categories/README.mdlib/features/categories/repository/categories_repository.dartlib/features/onboarding/ui/onboarding_welcome_modal.dartlib/features/onboarding/ui/widgets/onboarding_api_key_panel.dartpubspec.yamltest/features/categories/repository/categories_repository_test.darttest/features/categories/test_utils.darttest/features/onboarding/ui/onboarding_animation_gallery_page_test.darttest/features/onboarding/ui/onboarding_welcome_modal_test.darttest/features/onboarding/ui/widgets/onboarding_api_key_panel_test.darttest/mocks/mocks.dart
🚧 Files skipped from review as they are similar to previous changes (4)
- test/features/categories/repository/categories_repository_test.dart
- test/features/categories/test_utils.dart
- test/features/onboarding/ui/onboarding_welcome_modal_test.dart
- lib/features/onboarding/ui/onboarding_welcome_modal.dart
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 `@lib/features/categories/README.md`:
- Around line 32-37: Update the consent wording in the categories documentation
to match the implementation: provider connection/onboarding category setup
triggers automaticInferenceEnabled: true when profileId is present, before any
first-capture recording is verified. Do not describe recording a first capture
as part of the consent boundary unless the onboarding implementation is changed
to enforce that condition.
In `@test/features/onboarding/ui/onboarding_animation_gallery_page_test.dart`:
- Line 6: Add the explicit Override import from flutter_riverpod/misc.dart to
both onboarding_animation_gallery_page_test.dart (lines 6-6) and
onboarding_api_key_panel_test.dart (lines 10-13), so their List<Override> helper
signatures resolve correctly.
In `@test/features/onboarding/ui/onboarding_welcome_modal_test.dart`:
- Around line 89-91: Track whether setUp registered the SettingsDb mock,
distinguishing it from an existing suite-provided registration, and have the
teardown around the SettingsDb cleanup unregister only when this test owns that
registration. Preserve existing registrations for later tests.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b5a1693f-7a11-4035-94da-04efc7255fb8
📒 Files selected for processing (15)
CHANGELOG.mdflatpak/com.matthiasn.lotti.metainfo.xmllib/features/ai/state/seed_tombstone_provider.dartlib/features/ai/util/seed_tombstone_store.dartlib/features/categories/README.mdlib/features/categories/repository/categories_repository.dartlib/features/onboarding/ui/onboarding_welcome_modal.dartlib/features/onboarding/ui/widgets/onboarding_api_key_panel.dartpubspec.yamltest/features/categories/repository/categories_repository_test.darttest/features/categories/test_utils.darttest/features/onboarding/ui/onboarding_animation_gallery_page_test.darttest/features/onboarding/ui/onboarding_welcome_modal_test.darttest/features/onboarding/ui/widgets/onboarding_api_key_panel_test.darttest/mocks/mocks.dart
…teardown Review follow-up. The README and the code comment both said the consent was 'connecting a provider and recording a first capture', but the flag is written at the category step, before onStartCapture hands control to the capture. Reworded to the gesture that actually triggers it — connecting a provider and choosing those areas — rather than one that happens afterwards. The welcome-modal test preserved an existing SettingsDb registration but its tearDown removed it unconditionally, which would have deleted a suite-provided registration for every later test. It now tracks whether this setUp owns the registration and only unregisters its own.
1 participant
Summary
Follow-up to #3568: areas created during onboarding start with automatic inference on. Every other creation path still leaves it off.
CategoryRepository.createCategorygains an optionalautomaticInferenceEnabledOnboardingWelcomeModal._continuepassestruefor areas it creates, and fills in anullon reused areas — but never overwrites an explicitfalseWhy
#3568 made automatic transcription and image analysis an explicit per-category opt-in, defaulting to off everywhere. That left onboarding in an incoherent state, which I only spotted when tracing the onboarding path end to end:
onboarding_welcome_modal.dart:527binds each area to a seeded profile (onboardingSeededProfileId) plus Laura as the task-agent templateOnboarding was teaching a behaviour the app then withheld until the user found the switch.
What is and isn't affected — verified, not assumed
Onboarding's own capture never depended on the gate.
onboarding_first_task_step.dart:126usescaptureControllerProvider, andcapture_controller.dart:105transcribes throughAudioTranscriptionServicedirectly. That is an explicit user action and never passed throughProfileAutomationService. So the onboarding "aha" worked before this change and works after.What was broken is everything after onboarding. Task-linked recordings go
recorder_controller.dart:401→AutomaticPromptTrigger→tryTranscribe(automatic_prompt_trigger.dart:46), which is gated. That is the path this change fixes.Why this isn't the implicit enablement #3568 removed
The audit's objection was that selecting a profile silently enabled automation — a side effect of a configuration choice, with seeded profiles arriving pre-automated. Onboarding is different in kind: the user connects a provider, picks areas, and records a capture, all deliberately, in a flow that is explicitly about setting up AI. That is consent in context.
The boundary is kept narrow on purpose:
true; the category create modal and every other caller leave it nullnull. An existingfalseis the user having switched automation off, and onboarding leaves it standing — otherwise the flow would be a back door around the switchVerification
fvm flutter analyze lib test— no issues foundfvm flutter test test/features/onboarding/ test/features/categories/— 716 passedCategoryTestUtils.createTestCategorygained the field so the opt-out fixture is expressibleNote on version numbering
This branch is cut from main and claims
0.9.1067, the same number as the open #3573. Whichever merges second needs a rebase and a renumber — flagging it so it is not a surprise.Summary by CodeRabbit