Facelift changes and model loading UX fix#1
Merged
priyavratuniyal merged 21 commits intomainfrom Mar 26, 2026
Merged
Conversation
Required for the Manrope + Inter dual-font system specified in the Koshika design spec.
Add success/warning/info semantic colors, textPrimary/Secondary/Muted aliases, surfaceContainerHighest, onErrorContainer, onTertiaryContainer, and 10 health category colors with a categoryColor() lookup helper.
…and elevation tokens Adds KoshikaTypography (Manrope headlines + Inter body with 15 Material styles and 6 custom health styles), KoshikaSpacing (8dp grid), KoshikaRadius (sm through pill), KoshikaElevation (tinted BoxShadow presets), and pre-built card decorations and button styles.
…it scheme, wire fonts Switch to light-only theme (ThemeMode.light), replace the seed-generated ColorScheme with an explicit one mapped to AppColors tokens, wire Manrope + Inter via KoshikaTypography.textTheme, set card elevation to 0, and add pill/outlined button theme defaults.
KoshikaCard provides the standard card container with 24px radius, asymmetric editorial padding, and no-border design. StatusBadge maps BiomarkerFlag to semantic pill badges with icons. IconContainer wraps icons in colored rounded-rect backgrounds for list items.
Adds ShimmerScope (shared AnimationController), ShimmerBox, ShimmerLine, and ShimmerCircle for skeleton loading states. Uses a horizontal gradient sweep on a 1500ms linear repeat cycle.
SimpleTrendLinePainter is a lightweight CustomPainter for compact sparklines in dashboard category cards. Haptics wraps HapticFeedback with light, selection, and heavy static methods.
Replace hardcoded colors, typography, spacing, and radii with KoshikaTypography, KoshikaSpacing, KoshikaRadius, and KoshikaDecorations. Hero card uses gradient decoration and heroMetric typography. Category trend cards now use health category colors. Insights card uses tertiaryContainer (insight) decoration. Section headers use sectionHeader style. Border.all removed from insights card (no-line rule).
Replace hardcoded Colors.green/amber/orange/red/grey with AppColors semantic tokens. Remove Dividers and Border.all (no-line rule). Use KoshikaTypography, KoshikaDecorations, KoshikaRadius, and KoshikaSpacing.
Replace CircularProgressIndicator with shimmer skeleton loading. Use heroMetric typography for the main biomarker value and StatusBadge pill for flag display. Add LATEST RESULT and REFERENCE RANGE metric labels. History list uses alternating row backgrounds instead of Divider separators. All spacing, radii, and colors use design system tokens.
…design system Replace hardcoded Colors.green/red/orange with AppColors.success/error/warning. Use AppColors.secondary for chart line color. Replace Card with Container using KoshikaDecorations.card. Remove dead isDark branch in gauge painter. Zone labels use ALL-CAPS with design system-aligned styling.
…ip bubbles AI bubbles get 4px left accent strip and KOSHIKA INTELLIGENCE label. Chat input bar uses BackdropFilter for glassmorphic effect with privacy label. All state views (download, loading, ready, error, empty) migrated to design system tokens. Removed borders per no-line rule.
Reports list uses KoshikaCard containers instead of Card widgets. Report detail uses metricLabel for category headers, removes Dividers. Settings replaces ListTiles with custom SettingsRow using IconContainer, model tiles use KoshikaDecorations.card with switch expressions, and Dividers replaced with spacing per no-line rule.
Onboarding pages use design system typography and spacing. Action button gets gradient pill decoration with medium elevation shadow. Page indicator dots use AppColors tokens. Icon circles use primary at 15% opacity for a lighter, editorial feel.
…cons Custom bottom nav replaces NavigationBar with glassmorphic backdrop blur (white @ 85%), animated icon transitions between outlined and filled states, and primaryContainer pill indicator behind selected item. Uses extendBody for content-behind-nav effect.
GemmaService and EmbeddingService now call installModel/installEmbedder in initialize() when model files already exist on disk. This registers the model as "active" with the flutter_gemma SDK so that getActiveModel/ getActiveEmbedder succeeds without requiring the user to re-download. Previously, the services only checked isModelInstalled but never called install(), causing "No active model set" errors on app re-open. Chat screen now auto-loads the model on entry when status is ready, removing the manual "Load Model" tap requirement on every launch. Session restore on initState removed — chat always starts fresh. Raw exception strings replaced with user-friendly messages across SplashScreen and SettingsScreen; raw errors go to debugPrint only.
Remove extendBody from HomeScreen — glassmorphic nav bar was obscuring scrollable content on screens that need full height. Set centerTitle: false in AppBarTheme so titles align left per the editorial design spec. Replace Row with Wrap in dashboard hero card stats so the three stat items wrap to a second line on narrow screens instead of overflowing.
After services initialize, kick off loadModel() for both GemmaService and EmbeddingService if their status is ready (i.e. already downloaded). Calls are unawaited so navigation proceeds immediately while models warm up in the background, eliminating the manual Load tap requirement.
…e/koshika-facelift
📝 WalkthroughWalkthroughAdds a Koshika design system, migrates many screens/widgets to those tokens, switches the app to a fixed light theme with a glassmorphic bottom nav, adds shimmer/status/haptic utilities, and performs non-blocking background model re-load/re-install attempts during service initialization. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
- Wrap single-line if bodies in curly braces per lint rules - Resolves curly_braces_in_flow_control_structures warnings in dashboard_screen.dart and app_colors.dart
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/main.dart (1)
27-31:⚠️ Potential issue | 🟠 MajorRemove duplicate
FlutterGemma.initialize()call inGemmaService.initialize().
FlutterGemma.initialize(maxDownloadRetries: 5)is called twice:
- In
main()at line 29 (awaited, beforerunApp())- In
gemma_service.dartline 71 (not awaited, insideinitialize())The first initialization in
main()is sufficient and completes before the app starts. The second call ingemmaService.initialize()is redundant. Remove theFlutterGemma.initialize()call fromgemmaService.initialize()to avoid double initialization of the SDK.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/main.dart` around lines 27 - 31, Remove the redundant SDK initialization: delete the FlutterGemma.initialize(...) call from GemmaService.initialize() so the SDK is only initialized once in main() where Future<void> main() awaits FlutterGemma.initialize(maxDownloadRetries: 5) before runApp; keep the initialization in main() and ensure GemmaService.initialize() no longer calls or attempts to re-initialize FlutterGemma.initialize().
🧹 Nitpick comments (7)
lib/widgets/dashboard_summary_card.dart (2)
50-65: Consider using design tokens instead ofColors.white.Lines 54 and 63 use
Colors.whitedirectly, which is inconsistent with the design-system-driven approach used elsewhere in this PR. Consider using an appropriateAppColorstoken (e.g.,AppColors.onPrimaryContaineror a dedicated surface color) to maintain consistency and ease future theme adjustments.♻️ Suggested token usage
Text( 'Health Overview', style: theme.textTheme.titleMedium?.copyWith( fontWeight: FontWeight.bold, - color: Colors.white, + color: AppColors.onPrimaryContainer, ), ), ], ), const SizedBox(height: KoshikaSpacing.base), Text( '$totalTracked Biomarkers Tracked', style: KoshikaTypography.sectionHeader.copyWith( - color: Colors.white, + color: AppColors.onPrimaryContainer, ), ),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/widgets/dashboard_summary_card.dart` around lines 50 - 65, Replace the hardcoded Colors.white usages in the Text styles with the appropriate design token from your design system (e.g., AppColors.onPrimaryContainer) to keep theming consistent: update the color passed into theme.textTheme.titleMedium?.copyWith(...) and KoshikaTypography.sectionHeader.copyWith(...) to use the AppColors token, and add the necessary import for AppColors at the top of dashboard_summary_card.dart; ensure both Text style usages (the "Health Overview" title and the '$totalTracked Biomarkers Tracked' subtitle) use the same chosen token.
161-166: Consider using design-system typography for the label.Line 166 uses an inline
TextStyle(fontSize: 12, color: color)while the count on line 163 usesKoshikaTypography.statusText. For consistency, consider using a design-system typography token for the label as well.♻️ Suggested refactor
Text( '$count', style: KoshikaTypography.statusText.copyWith(color: color), ), const SizedBox(width: KoshikaSpacing.xs), - Text(label, style: TextStyle(fontSize: 12, color: color)), + Text(label, style: KoshikaTypography.statusText.copyWith(color: color)), ],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/widgets/dashboard_summary_card.dart` around lines 161 - 166, The label Text currently uses an inline TextStyle(fontSize: 12, color: color); replace that inline style with the design-system typography token from KoshikaTypography to match the count's usage (e.g., use an appropriate token such as KoshikaTypography.label, KoshikaTypography.bodySmall, or the project’s equivalent) and apply the color via copyWith(color: color); update the Text widget that renders label so it uses KoshikaTypography.<token>.copyWith(color: color) instead of the inline TextStyle.lib/screens/onboarding_screen.dart (1)
161-168: Hardcoded color in gradient breaks design system consistency.The gradient uses a hardcoded teal color
Color(0xFF0D9488)(Line 164) alongsideAppColors.primary. This circumvents the design system that the PR establishes. Consider adding this color toAppColors(e.g.,AppColors.primaryGradientEndorAppColors.teal) for consistency and easier future theming.🎨 Extract gradient color to design system
In
lib/theme/app_colors.dart, add:static const Color primaryGradientEnd = Color(0xFF0D9488);Then update the gradient:
child: DecoratedBox( decoration: BoxDecoration( gradient: const LinearGradient( - colors: [AppColors.primary, Color(0xFF0D9488)], + colors: [AppColors.primary, AppColors.primaryGradientEnd], ), borderRadius: KoshikaRadius.pill, boxShadow: KoshikaElevation.medium, ),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/screens/onboarding_screen.dart` around lines 161 - 168, The gradient in the DecoratedBox currently uses a hardcoded Color(0xFF0D9488) next to AppColors.primary; add a named constant to the design system (e.g., in AppColors add static const Color primaryGradientEnd = Color(0xFF0D9488)) and update the gradient colors list in onboarding_screen.dart to use AppColors.primaryGradientEnd instead of the hardcoded value so the gradient follows the AppColors theme (look for DecoratedBox and KoshikaRadius.pill to locate the usage).lib/widgets/trend_line_painter.dart (1)
79-83: List reference comparison inshouldRepaintmay not behave as expected.The comparison
values != oldDelegate.valuescompares list references, not contents. This means:
- If a new list with identical values is passed, it will trigger a repaint (slightly inefficient but safe)
- If the same list instance is mutated in place, changes won't be detected (unlikely usage pattern but risky)
For a lightweight sparkline painter, this is acceptable, but worth noting for maintainability.
🔧 Optional: Use deep equality for more precise repaint control
+import 'package:flutter/foundation.dart'; + `@override` bool shouldRepaint(SimpleTrendLinePainter oldDelegate) => - values != oldDelegate.values || + !listEquals(values, oldDelegate.values) || lineColor != oldDelegate.lineColor || lineWidth != oldDelegate.lineWidth || showFill != oldDelegate.showFill;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/widgets/trend_line_painter.dart` around lines 79 - 83, The shouldRepaint implementation currently compares list references (values != oldDelegate.values) which misses in-place mutations and overtriggers for equivalent new lists; update SimpleTrendLinePainter.shouldRepaint to compare list contents instead (e.g., use Flutter's listEquals from foundation or manually compare lengths and elements) and keep the existing comparisons for lineColor, lineWidth, and showFill; ensure you reference values, shouldRepaint, and SimpleTrendLinePainter when making the change.lib/screens/reports_screen.dart (1)
381-390: Silent exception swallowing during file deletion.The catch block at Lines 383-388 silently ignores all exceptions when deleting the PDF file. While this prevents crashes, it could hide issues like permission problems or disk errors that might be worth logging.
🔧 Consider logging the error for debugging
onDismissed: (_) { objectbox.deleteReport(report.id); try { final file = File(report.pdfPath); if (file.existsSync()) { file.deleteSync(); } - } catch (_) {} + } catch (e) { + debugPrint('Failed to delete PDF file: $e'); + } setState(() {}); },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/screens/reports_screen.dart` around lines 381 - 390, The onDismissed handler currently swallows all exceptions during PDF deletion; update the catch to capture the exception (and optionally the stack) and log it instead of ignoring it so permission/disk errors are visible—specifically modify the try/catch around File(report.pdfPath).existsSync()/deleteSync() in the onDismissed block (which also calls objectbox.deleteReport(report.id) and setState()) to log the error via your app logger or debugPrint including contextual info (report.id and report.pdfPath) and consider showing a non-blocking user-facing message if deletion fails.lib/screens/settings_screen.dart (1)
454-582: Extract the shared model-status card before these drift.
_ModelStatusTileand_EmbeddingModelTilenow duplicate the same layout, status mapping, progress/error rendering, and action row. A single configurable widget would keep future model-state/UI changes from diverging.Also applies to: 605-744
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/screens/settings_screen.dart` around lines 454 - 582, The two widgets _ModelStatusTile and _EmbeddingModelTile duplicate the same UI and status logic (layout, status icon/label/color getters, progress/error rendering, and action row); refactor by extracting a single reusable widget (e.g., ModelStatusCard) that accepts a ModelInfo (or the existing modelInfo), callbacks (onDownload/onLoad/onUnload) and optional config (labels/icons if needed), then replace _ModelStatusTile and _EmbeddingModelTile with thin wrappers that build ModelStatusCard; move the shared getters (_statusIcon, _statusLabel, _statusColor), progress/error rendering and action-row rendering into ModelStatusCard so changes apply in one place while retaining modelInfo-based behavior.lib/theme/koshika_design_system.dart (1)
162-190: Nit: Comment says "8dp base grid" but values include non-8dp multiples.Values like
xs=4,md=12,lg=20,contentPadding=20don't follow an 8dp grid. Consider updating the comment to "4dp base grid" or "4dp/8dp hybrid grid" for accuracy.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/theme/koshika_design_system.dart` around lines 162 - 190, The comment "8dp base grid" is inaccurate since values in KoshikaSpacing (e.g., xs, sm, md, lg, xl, xxl, xxxl and screen/content paddings like contentPadding and cardPaddingAsymmetric) use 4dp increments and non-8 multiples; update the header comment to reflect the actual spacing system (for example "4dp base grid" or "4dp/8dp hybrid grid") so it accurately documents KoshikaSpacing and its constants.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/screens/biomarker_detail_screen.dart`:
- Around line 75-117: The skeleton inside ShimmerScope is a tall Column that can
overflow; make it scrollable by wrapping the Column (the child of Padding) in a
scrollable widget such as SingleChildScrollView (or ListView) so the
ShimmerScope + Padding remain but the inner content can scroll; update the
widget tree around the Column (affecting ShimmerScope, the Padding's child, and
all ShimmerBox/ShimmerLine widgets) to use a SingleChildScrollView (optionally
with physics: AlwaysScrollableScrollPhysics and a Column inside) so the loading
skeleton no longer causes RenderFlex overflow on small/landscape viewports.
In `@lib/screens/settings_screen.dart`:
- Around line 387-430: The interactive row currently uses GestureDetector;
replace it with a Material widget wrapping an InkWell to enable keyboard focus,
hover and ripple feedback: wrap the Container's content in Material (color:
AppColors.surfaceContainerLowest, borderRadius: KoshikaRadius.lg) and move the
onTap to an InkWell (set borderRadius to KoshikaRadius.lg and keep
splash/highlight behavior), keeping padding/margin and child Row intact; ensure
any null checks for trailing/subtitle remain and that semantics/focus are
preserved by providing InkWell's onTap (and optionally focusNode/hoverColor if
needed).
In `@lib/widgets/biomarker_trend_chart.dart`:
- Around line 127-131: The dot color sets dotColor based on (result.flag ==
BiomarkerFlag.normal || result.flag == BiomarkerFlag.unknown) so the tooltip's
flag color logic must match that rule; update the tooltip color branch (the code
computing the tooltip flag color, e.g., tooltipFlagColor or similar) to treat
BiomarkerFlag.unknown the same as BiomarkerFlag.normal (use AppColors.success
for both) instead of rendering unknown as AppColors.error, ensuring consistency
between the dotColor and tooltip display for result.flag values.
In `@lib/widgets/chat_message_bubble.dart`:
- Around line 46-69: The left accent Container currently renders as a separate
short strip (with only minHeight: 40) and therefore doesn't span multi-line
bubbles; instead, remove that separate accent Container used when (!isUser &&
!isError) and add a left border to the bubble Container's BoxDecoration (the
Container with padding and decoration named bubbleColor) by setting
decoration.border = Border(left: BorderSide(color: AppColors.primary, width:
4)); ensure you only apply this border when (!isUser && !isError) and keep the
existing borderRadius and bubbleColor intact so the accent aligns with the full
bubble height for multiline messages.
In `@lib/widgets/shimmer_loading.dart`:
- Around line 48-53: The current of(BuildContext context) method relies on an
assert that is stripped in release builds, causing a null-check crash when the
_ShimmerInherited is missing; change it to explicitly check for null and throw a
FlutterError with a clear message (e.g., "ShimmerScope not found in widget
tree") instead of using assert so release builds fail fast and provide a helpful
error; update the method that references _ShimmerInherited and returns
inherited!.controller to perform a null check and throw the FlutterError when
inherited is null.
In `@pubspec.yaml`:
- Around line 23-25: The pubspec.yaml currently pins the google_fonts dependency
to ^6.2.1; update the dependency declaration for google_fonts to the current
stable version (e.g., ^8.0.2) in pubspec.yaml, run flutter pub get and the test
suite to catch any breaking API changes, and if you must keep ^6.2.1 instead,
add a brief comment in pubspec.yaml explaining the compatibility reason;
reference the google_fonts entry when making the change.
---
Outside diff comments:
In `@lib/main.dart`:
- Around line 27-31: Remove the redundant SDK initialization: delete the
FlutterGemma.initialize(...) call from GemmaService.initialize() so the SDK is
only initialized once in main() where Future<void> main() awaits
FlutterGemma.initialize(maxDownloadRetries: 5) before runApp; keep the
initialization in main() and ensure GemmaService.initialize() no longer calls or
attempts to re-initialize FlutterGemma.initialize().
---
Nitpick comments:
In `@lib/screens/onboarding_screen.dart`:
- Around line 161-168: The gradient in the DecoratedBox currently uses a
hardcoded Color(0xFF0D9488) next to AppColors.primary; add a named constant to
the design system (e.g., in AppColors add static const Color primaryGradientEnd
= Color(0xFF0D9488)) and update the gradient colors list in
onboarding_screen.dart to use AppColors.primaryGradientEnd instead of the
hardcoded value so the gradient follows the AppColors theme (look for
DecoratedBox and KoshikaRadius.pill to locate the usage).
In `@lib/screens/reports_screen.dart`:
- Around line 381-390: The onDismissed handler currently swallows all exceptions
during PDF deletion; update the catch to capture the exception (and optionally
the stack) and log it instead of ignoring it so permission/disk errors are
visible—specifically modify the try/catch around
File(report.pdfPath).existsSync()/deleteSync() in the onDismissed block (which
also calls objectbox.deleteReport(report.id) and setState()) to log the error
via your app logger or debugPrint including contextual info (report.id and
report.pdfPath) and consider showing a non-blocking user-facing message if
deletion fails.
In `@lib/screens/settings_screen.dart`:
- Around line 454-582: The two widgets _ModelStatusTile and _EmbeddingModelTile
duplicate the same UI and status logic (layout, status icon/label/color getters,
progress/error rendering, and action row); refactor by extracting a single
reusable widget (e.g., ModelStatusCard) that accepts a ModelInfo (or the
existing modelInfo), callbacks (onDownload/onLoad/onUnload) and optional config
(labels/icons if needed), then replace _ModelStatusTile and _EmbeddingModelTile
with thin wrappers that build ModelStatusCard; move the shared getters
(_statusIcon, _statusLabel, _statusColor), progress/error rendering and
action-row rendering into ModelStatusCard so changes apply in one place while
retaining modelInfo-based behavior.
In `@lib/theme/koshika_design_system.dart`:
- Around line 162-190: The comment "8dp base grid" is inaccurate since values in
KoshikaSpacing (e.g., xs, sm, md, lg, xl, xxl, xxxl and screen/content paddings
like contentPadding and cardPaddingAsymmetric) use 4dp increments and non-8
multiples; update the header comment to reflect the actual spacing system (for
example "4dp base grid" or "4dp/8dp hybrid grid") so it accurately documents
KoshikaSpacing and its constants.
In `@lib/widgets/dashboard_summary_card.dart`:
- Around line 50-65: Replace the hardcoded Colors.white usages in the Text
styles with the appropriate design token from your design system (e.g.,
AppColors.onPrimaryContainer) to keep theming consistent: update the color
passed into theme.textTheme.titleMedium?.copyWith(...) and
KoshikaTypography.sectionHeader.copyWith(...) to use the AppColors token, and
add the necessary import for AppColors at the top of
dashboard_summary_card.dart; ensure both Text style usages (the "Health
Overview" title and the '$totalTracked Biomarkers Tracked' subtitle) use the
same chosen token.
- Around line 161-166: The label Text currently uses an inline
TextStyle(fontSize: 12, color: color); replace that inline style with the
design-system typography token from KoshikaTypography to match the count's usage
(e.g., use an appropriate token such as KoshikaTypography.label,
KoshikaTypography.bodySmall, or the project’s equivalent) and apply the color
via copyWith(color: color); update the Text widget that renders label so it uses
KoshikaTypography.<token>.copyWith(color: color) instead of the inline
TextStyle.
In `@lib/widgets/trend_line_painter.dart`:
- Around line 79-83: The shouldRepaint implementation currently compares list
references (values != oldDelegate.values) which misses in-place mutations and
overtriggers for equivalent new lists; update
SimpleTrendLinePainter.shouldRepaint to compare list contents instead (e.g., use
Flutter's listEquals from foundation or manually compare lengths and elements)
and keep the existing comparisons for lineColor, lineWidth, and showFill; ensure
you reference values, shouldRepaint, and SimpleTrendLinePainter when making the
change.
🪄 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
Run ID: eb8e9bfb-34ed-4b24-82ff-72bc2f904cc4
⛔ Files ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (25)
lib/main.dartlib/screens/biomarker_detail_screen.dartlib/screens/chat_screen.dartlib/screens/dashboard_screen.dartlib/screens/onboarding_screen.dartlib/screens/report_detail_screen.dartlib/screens/reports_screen.dartlib/screens/settings_screen.dartlib/screens/splash_screen.dartlib/services/embedding_service.dartlib/services/gemma_service.dartlib/services/haptic_feedback_service.dartlib/theme/app_colors.dartlib/theme/koshika_design_system.dartlib/widgets/biomarker_trend_chart.dartlib/widgets/chat_message_bubble.dartlib/widgets/dashboard_summary_card.dartlib/widgets/flag_badge.dartlib/widgets/icon_container.dartlib/widgets/koshika_card.dartlib/widgets/reference_range_gauge.dartlib/widgets/shimmer_loading.dartlib/widgets/status_badge.dartlib/widgets/trend_line_painter.dartpubspec.yaml
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/screens/dashboard_screen.dart (1)
829-843:⚠️ Potential issue | 🟡 MinorLatest mini-bar highlight can be wrong for duplicate heights.
On Line 830–832,
barHeights.indexOf(h)uses first-match lookup, so if values repeat, the true last bar may not be marked as latest.Suggested fix
- children: barHeights.map((h) { - final isLatest = - barHeights.last == h && - barHeights.indexOf(h) == barHeights.length - 1; + children: barHeights.asMap().entries.map((entry) { + final index = entry.key; + final h = entry.value; + final isLatest = index == barHeights.length - 1; return Expanded(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/screens/dashboard_screen.dart` around lines 829 - 843, The logic that sets isLatest uses barHeights.indexOf(h), which returns the first match and fails when heights repeat; change the iteration to preserve the index and use that index to detect the last bar (e.g., iterate via barHeights.asMap().entries or List.generate so you have both index and value) and set isLatest to (index == barHeights.length - 1) — update the mapping closure where barHeights.map((h) { ... }) is used so it references the index instead of indexOf(h).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/screens/dashboard_screen.dart`:
- Around line 223-231: The comparator currently returns 0 for non-critical rows
causing unstable ordering; update the comparator (the block comparing a.flag and
b.flag) so when neither side is critical it falls back to a deterministic
secondary key: first compare severity (e.g., a.severity vs b.severity, using
compareTo or numeric subtraction so higher severity sorts earlier), and if
severities are equal then compare a.name.toLowerCase() with b.name.toLowerCase()
for a stable alphabetical tiebreaker. Ensure the comparator returns the integer
result of the severity comparison or the name compareTo instead of 0.
---
Outside diff comments:
In `@lib/screens/dashboard_screen.dart`:
- Around line 829-843: The logic that sets isLatest uses barHeights.indexOf(h),
which returns the first match and fails when heights repeat; change the
iteration to preserve the index and use that index to detect the last bar (e.g.,
iterate via barHeights.asMap().entries or List.generate so you have both index
and value) and set isLatest to (index == barHeights.length - 1) — update the
mapping closure where barHeights.map((h) { ... }) is used so it references the
index instead of indexOf(h).
🪄 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
Run ID: 43f95833-b050-4506-a903-a44428661c5b
⛔ Files ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
lib/screens/dashboard_screen.dartlib/theme/app_colors.dart
- wrap shimmer skeleton in ListView to prevent overflow on landscape/small screens - add secondary name sort to make out-of-range list order deterministic - align unknown flag tooltip color with dot color (both now green) - replace GestureDetector with Material+InkWell in settings rows for ripple and a11y - use left BorderSide on chat bubble instead of separate accent strip widget
There was a problem hiding this comment.
♻️ Duplicate comments (2)
lib/screens/dashboard_screen.dart (1)
222-231:⚠️ Potential issue | 🟠 MajorKeep severity ordering before the name tiebreaker.
Lines 290-310 only render
outOfRangeResults.take(3). With the current comparator, aborderlineitem can displace ahigh/lowone purely because its name sorts earlier.📌 Suggested change
+ int severityRank(BiomarkerFlag flag) { + switch (flag) { + case BiomarkerFlag.critical: + return 0; + case BiomarkerFlag.high: + case BiomarkerFlag.low: + return 1; + case BiomarkerFlag.borderline: + return 2; + case BiomarkerFlag.normal: + case BiomarkerFlag.unknown: + return 3; + } + } + outOfRangeResults.sort((a, b) { - if (a.flag == BiomarkerFlag.critical && - b.flag != BiomarkerFlag.critical) { - return -1; - } - if (b.flag == BiomarkerFlag.critical && - a.flag != BiomarkerFlag.critical) { - return 1; - } - return a.displayName.compareTo(b.displayName); + final bySeverity = severityRank(a.flag).compareTo(severityRank(b.flag)); + if (bySeverity != 0) return bySeverity; + return a.displayName.toLowerCase().compareTo( + b.displayName.toLowerCase(), + ); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/screens/dashboard_screen.dart` around lines 222 - 231, The comparator passed to outOfRangeResults.sort currently mixes severity and name ordering and allows name to override severity; change the comparator in the outOfRangeResults.sort block so it first maps a.flag and b.flag to a severity rank (e.g., critical > high/low > borderline > normal) and compares those ranks, and only if the ranks are equal fall back to comparing a.displayName.compareTo(b.displayName); reference the comparator used on outOfRangeResults and the BiomarkerFlag enum/constants when implementing the severity-to-rank mapping.lib/widgets/biomarker_trend_chart.dart (1)
127-131:⚠️ Potential issue | 🟠 MajorDon't render
unknownas a healthy result.
BiomarkerFlag.unknownis indeterminate, but both branches color it withAppColors.success. That makes the chart show an unknown value as normal, whilelib/widgets/status_badge.dartandlib/widgets/flag_badge.dartrender it neutrally.🎯 Suggested change
- final dotColor = - (result.flag == BiomarkerFlag.normal || - result.flag == BiomarkerFlag.unknown) - ? AppColors.success - : AppColors.error; + final dotColor = result.flag == BiomarkerFlag.normal + ? AppColors.success + : result.flag == BiomarkerFlag.unknown + ? AppColors.textMuted + : AppColors.error; … - final flagColor = - (result.flag == BiomarkerFlag.normal || - result.flag == BiomarkerFlag.unknown) - ? AppColors.success - : AppColors.error; + final flagColor = result.flag == BiomarkerFlag.normal + ? AppColors.success + : result.flag == BiomarkerFlag.unknown + ? AppColors.textMuted + : AppColors.error;Also applies to: 243-247
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/widgets/biomarker_trend_chart.dart` around lines 127 - 131, The dotColor logic in biomarker_trend_chart currently treats BiomarkerFlag.unknown as success; update the conditional in the dotColor assignment (and the similar block at the other occurrence) to only map BiomarkerFlag.normal to AppColors.success, map BiomarkerFlag.unknown to the neutral color used by StatusBadge/FlagBadge (e.g., AppColors.neutral), and map other non-normal flags to AppColors.error so unknown values render neutrally instead of as healthy.
🧹 Nitpick comments (2)
lib/screens/settings_screen.dart (1)
560-584: Consider extracting shared status logic to reduce duplication.The
_statusIcon,_statusLabel, and_statusColorgetters are identical between_ModelStatusTileand_EmbeddingModelTile. This duplication could be consolidated.♻️ Suggested approach: extract to a mixin or extension
// Add this extension near the top of the file or in a shared location extension ModelStatusVisuals on ModelStatus { IconData get icon => switch (this) { ModelStatus.notDownloaded => Icons.cloud_download_outlined, ModelStatus.downloading => Icons.downloading, ModelStatus.ready => Icons.check_circle_outline, ModelStatus.loading => Icons.hourglass_top, ModelStatus.loaded => Icons.check_circle, ModelStatus.error => Icons.error_outline, }; String get label => switch (this) { ModelStatus.notDownloaded => 'Not Downloaded', ModelStatus.downloading => 'Downloading...', ModelStatus.ready => 'Ready', ModelStatus.loading => 'Loading...', ModelStatus.loaded => 'Active', ModelStatus.error => 'Error', }; Color get color => switch (this) { ModelStatus.notDownloaded => AppColors.onSurfaceVariant, ModelStatus.downloading || ModelStatus.loading => AppColors.statusBusy, ModelStatus.ready => AppColors.statusReady, ModelStatus.loaded => AppColors.statusActive, ModelStatus.error => AppColors.error, }; }Then in both tiles, replace:
- final statusColor = _statusColor; + final statusColor = modelInfo.status.color; ... - Icon(_statusIcon, color: statusColor, size: 20), + Icon(modelInfo.status.icon, color: statusColor, size: 20), ... - _statusLabel, + modelInfo.status.label,Also applies to: 722-746
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/screens/settings_screen.dart` around lines 560 - 584, Extract the duplicated status logic from _statusIcon, _statusLabel, and _statusColor into a single extension or mixin on ModelStatus (e.g. ModelStatusVisuals) that exposes icon, label, and color getters; update both _ModelStatusTile and _EmbeddingModelTile to call modelInfo.status.icon / .label / .color instead of the local getters to remove duplication and keep behavior identical.lib/widgets/chat_message_bubble.dart (1)
68-68: Consider localizing hardcoded labels.
'KOSHIKA INTELLIGENCE'and'Error'are user-facing strings; if l10n is used in the app, move these to localization keys.Also applies to: 90-90
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/widgets/chat_message_bubble.dart` at line 68, Replace the hardcoded user-facing strings in the ChatMessageBubble widget (the label "KOSHIKA INTELLIGENCE" and the "Error" text) with localized resources: locate the occurrences in the ChatMessageBubble class/build method and use the app's localization accessor (e.g., AppLocalizations.of(context).koshikaIntelligence or context.l10n.koshikaIntelligence) instead of the literal strings, add corresponding keys/entries to the ARB/i18n resource files, and update any imports to reference the localization class so the UI reads the localized values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@lib/screens/dashboard_screen.dart`:
- Around line 222-231: The comparator passed to outOfRangeResults.sort currently
mixes severity and name ordering and allows name to override severity; change
the comparator in the outOfRangeResults.sort block so it first maps a.flag and
b.flag to a severity rank (e.g., critical > high/low > borderline > normal) and
compares those ranks, and only if the ranks are equal fall back to comparing
a.displayName.compareTo(b.displayName); reference the comparator used on
outOfRangeResults and the BiomarkerFlag enum/constants when implementing the
severity-to-rank mapping.
In `@lib/widgets/biomarker_trend_chart.dart`:
- Around line 127-131: The dotColor logic in biomarker_trend_chart currently
treats BiomarkerFlag.unknown as success; update the conditional in the dotColor
assignment (and the similar block at the other occurrence) to only map
BiomarkerFlag.normal to AppColors.success, map BiomarkerFlag.unknown to the
neutral color used by StatusBadge/FlagBadge (e.g., AppColors.neutral), and map
other non-normal flags to AppColors.error so unknown values render neutrally
instead of as healthy.
---
Nitpick comments:
In `@lib/screens/settings_screen.dart`:
- Around line 560-584: Extract the duplicated status logic from _statusIcon,
_statusLabel, and _statusColor into a single extension or mixin on ModelStatus
(e.g. ModelStatusVisuals) that exposes icon, label, and color getters; update
both _ModelStatusTile and _EmbeddingModelTile to call modelInfo.status.icon /
.label / .color instead of the local getters to remove duplication and keep
behavior identical.
In `@lib/widgets/chat_message_bubble.dart`:
- Line 68: Replace the hardcoded user-facing strings in the ChatMessageBubble
widget (the label "KOSHIKA INTELLIGENCE" and the "Error" text) with localized
resources: locate the occurrences in the ChatMessageBubble class/build method
and use the app's localization accessor (e.g.,
AppLocalizations.of(context).koshikaIntelligence or
context.l10n.koshikaIntelligence) instead of the literal strings, add
corresponding keys/entries to the ARB/i18n resource files, and update any
imports to reference the localization class so the UI reads the localized
values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7698b18e-dc14-41a0-8fd7-729bd66cf558
📒 Files selected for processing (5)
lib/screens/biomarker_detail_screen.dartlib/screens/dashboard_screen.dartlib/screens/settings_screen.dartlib/widgets/biomarker_trend_chart.dartlib/widgets/chat_message_bubble.dart
This was referenced Mar 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Visual & Design Updates
Improvements