feat: badge inference profiles by real usage, not a provider-slot heuristic#3570
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
📝 WalkthroughWalkthroughInference profile badges now indicate profiles referenced by categories or agents, while provider detail pages list every profile using that provider’s models. Supporting utilities, Riverpod state, UI components, tests, localization keys, documentation, and 0.9.1066 release metadata were updated. ChangesInference profile usage
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ProviderDetailPage
participant ProfileUsage
participant ProfilesUsingProviderSection
ProviderDetailPage->>ProfileUsage: resolve profilesUsingProviderModels
ProfileUsage-->>ProviderDetailPage: return matching profiles
ProviderDetailPage->>ProfilesUsingProviderSection: pass matching profiles
ProfilesUsingProviderSection->>ProfilesUsingProviderSection: render profile cards
Possibly related PRs
Suggested reviewers: 🚥 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 |
…ristic The Profiles tab badged a profile ACTIVE when any of its model slots pointed at a model owned by a configured provider, preferring the isDefault flag. isDefault is never user-settable — only seed templates set it, and the profile form just carries it forward — so connecting a provider instantly badged a freshly seeded profile with no user action able to grant or revoke it. The winner was also order-dependent (first match in getConfigsByType order), and nothing read the result at inference time: real routing is a category's defaultProfileId, a task's profileId, or an agent's AgentInferenceSetup. A profile no request would ever use could be badged while the profile every category used showed nothing. The badge now reads 'In use' and is driven by profileIdsInUse(): a profile is badged iff a category default or an agent setup references it. Disabled agent setups are excluded (they run no inference) and typed setups shadow the legacy config.profileId chain. Tasks are deliberately not scanned — TaskData.profileId is copied from the category at creation, so it duplicates a signal already counted, and scanning every task to render a settings badge is not worth the query. The provider detail page asked a different, legitimate question under the same misleading label. Its section becomes 'Profiles using this provider' and lists every profile referencing the provider's models instead of picking an arbitrary winner — what would break if the provider were removed. Deletes pickActiveProfileForProvider and activeProfileIdsForProviders along with active_profile.dart; modelByProfileSlotId moves to the new profile_usage util.
a1dddc1 to
5809e8e
Compare
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (98.41%) is below the target coverage (99.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #3570 +/- ##
==========================================
- Coverage 99.22% 99.22% -0.01%
==========================================
Files 1780 1781 +1
Lines 130324 130336 +12
==========================================
+ Hits 129311 129322 +11
- Misses 1013 1014 +1
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:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
test/features/ai/ui/settings/widgets/v2/ai_settings_cards_model_test.dart (1)
365-373: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRename the stale “Active” test title.
The test now verifies
isInUseand the “In use” badge, but its title still says “Active”, which makes failures and test reports misleading.🤖 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/ai/ui/settings/widgets/v2/ai_settings_cards_model_test.dart` around lines 365 - 373, Rename the test title in the AiProfileCard widget test from “Active” to “In use” so it accurately describes the isInUse state and badge being verified; leave the test behavior unchanged.test/features/ai/ui/settings/ai_settings_page_test.dart (1)
484-507: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMake the badge assertion distinguish routing from
isDefault.Because
profile-1is still the default profile, a regression that badges defaults instead of consumingprofileIdsInUseProvidercould pass this test. Make the in-use profile non-default, make another profile default but absent from the override set, and assert the badge within each card rather than globally.🤖 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/ai/ui/settings/ai_settings_page_test.dart` around lines 484 - 507, Update the profile fixtures in the test around buildProfile so the in-use profile-1 is not default, while profile-2 is default but excluded from profileIdsInUseProvider. Scope the “In use” assertions to each corresponding AiProfileCard, verifying only profile-1 is badged and profile-2 is not.test/features/ai/ui/settings/provider/ai_provider_detail_page_test.dart (1)
440-466: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd a second matching profile to prove “every profile”.
This fixture contains only
defaultProfileand asserts one card, so it cannot detect a regression where the section renders only one matching profile. Add another profile referencinggemini-proand assert both cards/names are rendered.🤖 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/ai/ui/settings/provider/ai_provider_detail_page_test.dart` around lines 440 - 466, Extend the test around buildProfile and AiProfileCard to include a second non-default profile whose thinking slot references gemini-pro, include it in pumpWith profiles, and assert both profile names/cards render so the section verifies every matching profile.
🤖 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/l10n/app_cs.arb`:
- Around line 874-876: Re-sort the localization ARB files so
aiProviderDetailProfilesUsingTitle appears alphabetically between
aiProviderDetailPageTitle and aiProviderDetailRemoveButton. Apply this in
lib/l10n/app_cs.arb at lines 874-876, lib/l10n/app_da.arb at lines 1035-1036,
and lib/l10n/app_it.arb at lines 1035-1036, then run make sort_arb_files and
verify all locale files, including app_en.arb, remain correctly sorted.
In `@lib/l10n/app_nl.arb`:
- Line 1034: Re-sort the aiProviderDetailProfilesUsingTitle entry alphabetically
in all affected ARB files: lib/l10n/app_nl.arb:1034-1034,
lib/l10n/app_pt.arb:1034-1034, and lib/l10n/app_ro.arb:867-867. Place it after
aiProviderDetailPageTitle and before aiProviderDetailRemoveButton, preferably by
running make sort_arb_files.
---
Outside diff comments:
In `@test/features/ai/ui/settings/ai_settings_page_test.dart`:
- Around line 484-507: Update the profile fixtures in the test around
buildProfile so the in-use profile-1 is not default, while profile-2 is default
but excluded from profileIdsInUseProvider. Scope the “In use” assertions to each
corresponding AiProfileCard, verifying only profile-1 is badged and profile-2 is
not.
In `@test/features/ai/ui/settings/provider/ai_provider_detail_page_test.dart`:
- Around line 440-466: Extend the test around buildProfile and AiProfileCard to
include a second non-default profile whose thinking slot references gemini-pro,
include it in pumpWith profiles, and assert both profile names/cards render so
the section verifies every matching profile.
In `@test/features/ai/ui/settings/widgets/v2/ai_settings_cards_model_test.dart`:
- Around line 365-373: Rename the test title in the AiProfileCard widget test
from “Active” to “In use” so it accurately describes the isInUse state and badge
being verified; leave the test behavior unchanged.
🪄 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: e0bf9bb2-8cc1-443f-8512-4b4862543ffc
📒 Files selected for processing (42)
CHANGELOG.mdflatpak/com.matthiasn.lotti.metainfo.xmllib/features/ai/state/profile_usage_provider.dartlib/features/ai/ui/settings/README.mdlib/features/ai/ui/settings/ai_settings_page.dartlib/features/ai/ui/settings/ai_settings_tab_builders.dartlib/features/ai/ui/settings/provider/ai_provider_detail_page.dartlib/features/ai/ui/settings/provider/ai_provider_detail_widgets.dartlib/features/ai/ui/settings/provider/ai_provider_models_section.dartlib/features/ai/ui/settings/util/active_profile.dartlib/features/ai/ui/settings/util/profile_usage.dartlib/features/ai/ui/settings/widgets/v2/ai_profile_card.dartlib/l10n/app_cs.arblib/l10n/app_da.arblib/l10n/app_de.arblib/l10n/app_en.arblib/l10n/app_es.arblib/l10n/app_fr.arblib/l10n/app_it.arblib/l10n/app_localizations.dartlib/l10n/app_localizations_cs.dartlib/l10n/app_localizations_da.dartlib/l10n/app_localizations_de.dartlib/l10n/app_localizations_en.dartlib/l10n/app_localizations_es.dartlib/l10n/app_localizations_fr.dartlib/l10n/app_localizations_it.dartlib/l10n/app_localizations_nl.dartlib/l10n/app_localizations_pt.dartlib/l10n/app_localizations_ro.dartlib/l10n/app_localizations_sv.dartlib/l10n/app_nl.arblib/l10n/app_pt.arblib/l10n/app_ro.arblib/l10n/app_sv.arbpubspec.yamltest/features/ai/state/profile_usage_provider_test.darttest/features/ai/ui/settings/ai_settings_page_test.darttest/features/ai/ui/settings/provider/ai_provider_detail_page_test.darttest/features/ai/ui/settings/util/active_profile_test.darttest/features/ai/ui/settings/util/profile_usage_test.darttest/features/ai/ui/settings/widgets/v2/ai_settings_cards_model_test.dart
💤 Files with no reviewable changes (2)
- lib/features/ai/ui/settings/util/active_profile.dart
- test/features/ai/ui/settings/util/active_profile_test.dart
| "aiProviderDetailProfilesUsingTitle": "Profily využívající tohoto poskytovatele", | ||
| "aiProviderDetailAddModelButton": "Přidat model", | ||
| "aiProviderDetailApiKeyLabel": "API klíč", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Re-sort ARB files after the aiProviderDetailActiveProfileTitle → aiProviderDetailProfilesUsingTitle rename. In all three locales, the renamed key now sorts before aiProviderDetailAddModelButton, which is alphabetically wrong ("AddModelButton" < "ProfilesUsingTitle"); it belongs between aiProviderDetailPageTitle and aiProviderDetailRemoveButton. This is likely present in the other locale files (and possibly app_en.arb) that weren't part of this review batch as well.
lib/l10n/app_cs.arb#L874-L876: moveaiProviderDetailProfilesUsingTitleto its correct alphabetical position (betweenaiProviderDetailPageTitleandaiProviderDetailRemoveButton).lib/l10n/app_da.arb#L1035-L1036: same fix.lib/l10n/app_it.arb#L1035-L1036: same fix.
Run make sort_arb_files to fix this and verify all 11 locale files (plus app_en.arb) are correctly sorted.
As per coding guidelines: "Keep localization ARB files sorted by running make sort_arb_files after adding labels."
📍 Affects 3 files
lib/l10n/app_cs.arb#L874-L876(this comment)lib/l10n/app_da.arb#L1035-L1036lib/l10n/app_it.arb#L1035-L1036
🤖 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/l10n/app_cs.arb` around lines 874 - 876, Re-sort the localization ARB
files so aiProviderDetailProfilesUsingTitle appears alphabetically between
aiProviderDetailPageTitle and aiProviderDetailRemoveButton. Apply this in
lib/l10n/app_cs.arb at lines 874-876, lib/l10n/app_da.arb at lines 1035-1036,
and lib/l10n/app_it.arb at lines 1035-1036, then run make sort_arb_files and
verify all locale files, including app_en.arb, remain correctly sorted.
Source: Coding guidelines
| "aiProviderConnectStepConnect": "Verbinden", | ||
| "aiProviderConnectStepReview": "Evaluatie", | ||
| "aiProviderDetailActiveProfileTitle": "Actief profiel", | ||
| "aiProviderDetailProfilesUsingTitle": "Profielen die deze provider gebruiken", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Renamed ARB key aiProviderDetailProfilesUsingTitle is out of alphabetical order in all three locale files. The key kept its old position (from the removed aiProviderDetailActiveProfileTitle), placed right before aiProviderDetailAddModelButton, instead of sorting after aiProviderDetailPageTitle and before aiProviderDetailRemoveButton as its new name requires. This is harmless for gen-l10n (JSON map) but violates the project's ARB-sort convention.
lib/l10n/app_nl.arb#L1034-L1034: move theaiProviderDetailProfilesUsingTitleentry to sort afteraiProviderDetailPageTitleand beforeaiProviderDetailRemoveButton(or simply re-runmake sort_arb_files).lib/l10n/app_pt.arb#L1034-L1034: same reordering fix.lib/l10n/app_ro.arb#L867-L867: same reordering fix.
Based on learnings, this relies on the path instruction: "**/*.arb: Keep localization ARB files sorted by running make sort_arb_files after adding labels."
📍 Affects 3 files
lib/l10n/app_nl.arb#L1034-L1034(this comment)lib/l10n/app_pt.arb#L1034-L1034lib/l10n/app_ro.arb#L867-L867
🤖 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/l10n/app_nl.arb` at line 1034, Re-sort the
aiProviderDetailProfilesUsingTitle entry alphabetically in all affected ARB
files: lib/l10n/app_nl.arb:1034-1034, lib/l10n/app_pt.arb:1034-1034, and
lib/l10n/app_ro.arb:867-867. Place it after aiProviderDetailPageTitle and before
aiProviderDetailRemoveButton, preferably by running make sort_arb_files.
Source: Coding guidelines
1 participant
Summary
Second piece of the inference-profile audit (after #3568): the ACTIVE badge becomes a statement about real usage instead of a heuristic nobody could influence.
profileIdsInUse()— a profile is in use iff a category default or an agent inference setup references itprofileIdsInUseProviderpickActiveProfileForProvider/activeProfileIdsForProviders;modelByProfileSlotIdmoves to the newutil/profile_usage.dartWhy
This is the bug that started the audit: "profiles are shown as active even though there is no clear mechanism to set them as active."
pickActiveProfileForProviderbadged the first profile with a model slot owned by a configured provider, preferringisDefault. Three problems:isDefaultis not user-settable. Only seed templates set it (profile_seeding_service.dart); the profile form only carries the existing value forward. So connecting a provider seeded a profile that was immediately badged, and no UI could grant or revoke the badge.getConfigsByTypeorder won — that order is a cache, not a sort.CategoryDefinition.defaultProfileId,TaskData.profileId, orAgentInferenceSetup. A profile no request would ever use could be badged while the profile every category used showed nothing.Definition of "in use"
Included: category defaults, and agent setups (
baseProfileId, plus the legacyconfig.profileIdchain for agents predating typed setups). Disabled setups are excluded — they run no inference, so a profile they still name is not routing anything.Excluded, deliberately: tasks.
TaskData.profileIdis copied from the owning category when the task is created, so a task reference duplicates a category reference already counted — and scanning every task to render a settings badge trades a real query cost for no new signal. An agent-level override is the case that genuinely diverges, and that is covered. Documented in the function's doc comment.Provider detail page
That section answered a different and legitimate question — "which profiles depend on this provider?" — under a label claiming something else. Rather than silently repurposing it, it is retitled and now lists every matching profile instead of an arbitrary winner, which is what you want before deleting a provider. Its cards render unbadged: referencing a provider's model is not the same as something routing through the profile.
Verification
fvm flutter analyze lib test— no issues foundfvm flutter test test/features/ai/— 4336 passedfvm flutter test test/features/ai/ui/settings/— 817 passedprofile_usage.dartandprofile_usage_provider.dartat 100% line coverage (verified againstcoverage/lcov.info, not estimated)Four existing tests encoded the old semantics and were updated rather than deleted — the Profiles-tab badge test now asserts the badge follows a reference, not
isDefault.Still open from the audit
lotti3-8f7.4— seeded profiles, models, and automation reviving after deletion (seedDefaultson every provider save,backfillNewModels, theupgradeExistingskill backfill, and no tombstones so a synced peer resurrects the row).Summary by CodeRabbit
New Features
Bug Fixes