fix: resolve UI overflow and migrate to structured medical insights screen#32
Open
Shweta-281 wants to merge 12 commits intoAOSSIE-Org:mainfrom
Open
fix: resolve UI overflow and migrate to structured medical insights screen#32Shweta-281 wants to merge 12 commits intoAOSSIE-Org:mainfrom
Shweta-281 wants to merge 12 commits intoAOSSIE-Org:mainfrom
Conversation
github-actions
bot
added
bug
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 7 minutes and 21 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughRemoved example API keys from Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as TranscriptionScreen
participant Controller as TranscriptionController
participant Deepgram as DeepgramService
participant DeepgramAPI as Deepgram (external)
participant Gemini as GeminiService
participant ChatbotAPI as Gemini (external)
participant InsightsScreen as MedicalInsightsScreen
User->>UI: Tap record / submit
UI->>Controller: start recording / submit file
Controller->>Deepgram: transcribe(recordingPath)
Deepgram->>DeepgramAPI: POST audio (with retries & 30s timeout)
DeepgramAPI-->>Deepgram: JSON response
Deepgram-->>Controller: transcript (or "No speech detected")
Controller->>Gemini: generateInsights(transcript)
Gemini->>ChatbotAPI: prompt requesting JSON
ChatbotAPI-->>Gemini: text containing JSON
Gemini-->>Controller: MedicalInsights model (or error)
Controller->>UI: update state / enable navigation
User->>UI: Open "Medical Insights"
UI->>InsightsScreen: navigate with symptoms and medicines
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![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
🧹 Nitpick comments (4)
lib/features/transcription/data/gemini_service.dart (2)
34-44:_extractJsonmay extract invalid JSON when response contains multiple objects or prose.Using
indexOf('{')andlastIndexOf('}')captures everything between the first{and last}, which may include non-JSON content if the AI includes explanatory text or multiple objects.Consider also stripping markdown code fences that LLMs often add despite instructions:
♻️ Suggested improvement
String _extractJson(String response) { + // Strip markdown code fences + var cleaned = response.replaceAll(RegExp(r'```json\s*'), '').replaceAll(RegExp(r'```\s*'), ''); + - final start = response.indexOf('{'); - final end = response.lastIndexOf('}'); + final start = cleaned.indexOf('{'); + final end = cleaned.lastIndexOf('}'); if (start != -1 && end != -1) { - return response.substring(start, end + 1); + return cleaned.substring(start, end + 1); } throw Exception('Invalid JSON format from AI'); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/features/transcription/data/gemini_service.dart` around lines 34 - 44, _extractJson currently slices from the first '{' to the last '}' of the raw response which can capture non-JSON prose or multiple objects; update _extractJson to first remove markdown code fences and language markers (e.g., strip leading/following triple backticks and any language like "json") and any surrounding whitespace into a cleaned string, then compute start = cleaned.indexOf('{') and end = cleaned.lastIndexOf('}'), and return cleaned.substring(start, end+1) when valid; keep throwing the same Exception when no valid braces are found.
29-31: Preserve original exception details for easier debugging.The generic error message discards the original exception, making it harder to diagnose parsing failures in production.
♻️ Include original error
} catch (e) { - throw Exception('Failed to parse Gemini JSON response'); + throw Exception('Failed to parse Gemini JSON response: $e'); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/features/transcription/data/gemini_service.dart` around lines 29 - 31, The catch block in gemini_service.dart currently throws a generic Exception('Failed to parse Gemini JSON response') and discards the original error; update that catch (e) handler to include the original exception details (and optionally the stack trace) in the thrown error or log so callers can diagnose parsing issues—e.g., append e.toString() (and stackTrace) to the message or rethrow the original error after wrapping with context so the original exception information is preserved when parsing the Gemini JSON response.lib/features/transcription/domain/transcription_model.dart (1)
12-20:copyWithcannot resetinsightstonullonce set.The pattern
insights ?? this.insightsmeans callers cannot explicitly clearinsightsback tonull. If this is needed (e.g., when starting a new recording session), consider using a sentinel or wrapper pattern:♻️ Optional: Allow explicit null assignment
TranscriptionModel copyWith({ String? rawTranscript, - MedicalInsights? insights, + MedicalInsights? Function()? insights, }) { return TranscriptionModel( rawTranscript: rawTranscript ?? this.rawTranscript, - insights: insights ?? this.insights, + insights: insights != null ? insights() : this.insights, ); }Usage:
copyWith(insights: () => null)to clear,copyWith(insights: () => newValue)to set.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/features/transcription/domain/transcription_model.dart` around lines 12 - 20, The copyWith implementation for TranscriptionModel prevents explicitly clearing insights because it uses `insights ?? this.insights`; change copyWith to accept a nullable wrapper or sentinel (for example accept `MedicalInsights? Function()? insights` or an `Optional<T>`-style parameter) so callers can pass a closure/sentinel that returns null to clear the field; update the TranscriptionModel.copyWith signature and internal logic to check whether the wrapper is provided and use its result (including explicit null) instead of falling back to this.insights, ensuring callers can both set a new MedicalInsights or explicitly reset it to null.lib/features/transcription/presentation/transcription_screen.dart (1)
276-289: Disabled button icon color remains purple.When
isEnabledis false, the icon retainsColors.deepPurplewhile the background changes toColors.white24. This may create visual inconsistency. Consider dimming the icon when disabled:♻️ Optional styling fix
child: ElevatedButton.icon( - icon: Icon(icon, color: Colors.deepPurple), + icon: Icon(icon, color: isEnabled ? Colors.deepPurple : Colors.grey), label: Text( title, - style: const TextStyle(fontSize: 16, color: Colors.black87), + style: TextStyle(fontSize: 16, color: isEnabled ? Colors.black87 : Colors.grey), ),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/features/transcription/presentation/transcription_screen.dart` around lines 276 - 289, The icon color stays deep purple when the button is disabled — update the ElevatedButton.icon construction so the Icon's color is conditional on isEnabled (e.g., use Colors.deepPurple when isEnabled is true and a dimmed color like Colors.deepPurple.withOpacity(0.4) or Colors.grey when false); modify the Icon(...) used in the ElevatedButton.icon and keep the existing isEnabled/onPressed logic so the visual state matches the disabled background.
🤖 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/features/transcription/presentation/transcription_screen.dart`:
- Around line 209-223: The enabled state for the Medical Insights navigation
button currently only checks controller.medicines; update the predicate passed
to _buildNavigationButton so the button is enabled when either
controller.symptoms or controller.medicines is non-empty and
controller.isProcessing is false (i.e., replace the single-list check with a
logical OR of controller.symptoms.isNotEmpty || controller.medicines.isNotEmpty
while still gating by !controller.isProcessing); keep the navigation target as
MedicalInsightsScreen(symptoms: controller.symptoms, medicines:
controller.medicines).
In `@lib/main.dart`:
- Around line 11-15: The .env load currently swallows failures (dotenv.load) and
only logs via debugPrint, causing downstream "missing API key" errors; implement
a retry with exponential backoff (e.g., 3 attempts with backoff delays) around
dotenv.load, and if all attempts fail set a module-level boolean flag (e.g.,
envLoadFailed or envLoaded = false) and call a controller-facing notifier (e.g.,
Controller.notifyEnvLoadFailure() or expose EnvState.envLoadFailed) so the UI
can show a snackbar or other user-visible warning; also keep the existing
debugPrint but include the final error details when marking the flag/issuing the
notification.
In `@pubspec.yaml`:
- Around line 67-68: Remove the `.env` entry from the pubspec `assets:` block
(remove the line `- .env`) and stop bundling secrets into the app; instead add
`.env` to `.flutterignore` or implement conditional build rules to exclude it
from release builds, and move DEEPGRAM_API_KEY and GEMINI_API_KEY usage to a
backend proxy that your Flutter app calls (do not include keys in client-side
URLs like `?key=$apiKey`); if you must store anything client-side temporarily,
switch to using `flutter_secure_storage` to write keys into platform secure
storage at runtime, but prefer the backend-proxy approach for production.
---
Nitpick comments:
In `@lib/features/transcription/data/gemini_service.dart`:
- Around line 34-44: _extractJson currently slices from the first '{' to the
last '}' of the raw response which can capture non-JSON prose or multiple
objects; update _extractJson to first remove markdown code fences and language
markers (e.g., strip leading/following triple backticks and any language like
"json") and any surrounding whitespace into a cleaned string, then compute start
= cleaned.indexOf('{') and end = cleaned.lastIndexOf('}'), and return
cleaned.substring(start, end+1) when valid; keep throwing the same Exception
when no valid braces are found.
- Around line 29-31: The catch block in gemini_service.dart currently throws a
generic Exception('Failed to parse Gemini JSON response') and discards the
original error; update that catch (e) handler to include the original exception
details (and optionally the stack trace) in the thrown error or log so callers
can diagnose parsing issues—e.g., append e.toString() (and stackTrace) to the
message or rethrow the original error after wrapping with context so the
original exception information is preserved when parsing the Gemini JSON
response.
In `@lib/features/transcription/domain/transcription_model.dart`:
- Around line 12-20: The copyWith implementation for TranscriptionModel prevents
explicitly clearing insights because it uses `insights ?? this.insights`; change
copyWith to accept a nullable wrapper or sentinel (for example accept
`MedicalInsights? Function()? insights` or an `Optional<T>`-style parameter) so
callers can pass a closure/sentinel that returns null to clear the field; update
the TranscriptionModel.copyWith signature and internal logic to check whether
the wrapper is provided and use its result (including explicit null) instead of
falling back to this.insights, ensuring callers can both set a new
MedicalInsights or explicitly reset it to null.
In `@lib/features/transcription/presentation/transcription_screen.dart`:
- Around line 276-289: The icon color stays deep purple when the button is
disabled — update the ElevatedButton.icon construction so the Icon's color is
conditional on isEnabled (e.g., use Colors.deepPurple when isEnabled is true and
a dimmed color like Colors.deepPurple.withOpacity(0.4) or Colors.grey when
false); modify the Icon(...) used in the ElevatedButton.icon and keep the
existing isEnabled/onPressed logic so the visual state matches the disabled
background.
🪄 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: f204d3c9-2c62-42d7-9391-fc21e6807245
⛔ Files ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
.env.examplelib/features/transcription/data/deepgram_service.dartlib/features/transcription/data/gemini_service.dartlib/features/transcription/domain/medical_insights.dartlib/features/transcription/domain/transcription_model.dartlib/features/transcription/presentation/transcription_controller.dartlib/features/transcription/presentation/transcription_screen.dartlib/main.dartlib/screens/medical_insights_screen.dartpubspec.yaml
💤 Files with no reviewable changes (1)
- .env.example
Author
|
Hi @SharkyBytes This PR fixes a UI overflow issue in the Transcription Screen and updates the navigation to support the new structured AI response model. Would love your feedback, especially on the new Medical Insights screen |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/features/transcription/presentation/transcription_screen.dart`:
- Around line 230-242: The _statusText function currently omits
TranscriptionState.done so successful runs fall through to the default message;
update the switch inside _statusText(TranscriptionState state) to add a case
TranscriptionState.done that returns the same header copy used by the details
(e.g., "Ready to view results"), keeping the existing default return ('Tap the
mic to begin') for idle states so done is handled explicitly and consistent with
_statusDetailText.
- Around line 157-165: The empty-silence UI currently only checks
controller.transcription.isEmpty but the controller sets a sentinel string for
no-audio results; update the condition that shows the "No speech detected"
Center to also check for that sentinel (mirror the same string/sentinel used in
TranscriptionController for the no-audio path) so the UI displays when
controller.state == TranscriptionState.done and controller.transcription equals
the controller's no-audio sentinel (or isEmpty); reference controller.state,
TranscriptionState.done and controller.transcription when making the change and
consider adding a dedicated getter on TranscriptionController later.
- Around line 185-199: Remove the legacy "Prescription" navigation button and
its surrounding spacing from the transcription UI: delete the
_buildNavigationButton call that uses the label 'Prescription', icon
Icons.medication, the controller.prescription check, and the Navigator.push to
PrescriptionScreen(prescription: controller.prescription), and also remove the
adjacent SizedBox if it becomes redundant; this ensures the deprecated
PrescriptionScreen route (and the duplicated controller.prescription aliasing
summary) is no longer exposed in transcription_screen.dart.
🪄 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: e33eecaf-fa85-4554-9c31-a5c711b0b107
📒 Files selected for processing (4)
lib/features/transcription/presentation/transcription_controller.dartlib/features/transcription/presentation/transcription_screen.dartlib/main.dartpubspec.yaml
💤 Files with no reviewable changes (1)
- pubspec.yaml
lib/features/transcription/presentation/transcription_screen.dart
Outdated
Show resolved
Hide resolved
lib/features/transcription/presentation/transcription_screen.dart
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/features/transcription/presentation/transcription_screen.dart`:
- Around line 293-299: The _statusDetailText function references
TranscriptionState.summarizing which doesn't exist; update that branch to a
valid enum value: either replace `.summarizing` with `.processing` if
summarization is part of the processing stage, or with `.transcribing` if you
intended a separate transcription stage, and adjust the returned string
accordingly (keep function name _statusDetailText and the surrounding branches
intact).
- Around line 206-217: The instantiation of MedicalInsightsScreen is using the
wrong constructor argument: it currently passes controller.summary (a String)
but the widget requires two List<String> args named symptoms and medicines;
update the Navigator push to construct MedicalInsightsScreen(symptoms:
controller.symptoms, medicines: controller.medicines) so the types match and the
correct controller getters are used (refer to MedicalInsightsScreen,
controller.symptoms, controller.medicines).
🪄 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: a6024a97-868a-4307-afa7-0338a7238b2b
📒 Files selected for processing (1)
lib/features/transcription/presentation/transcription_screen.dart
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
lib/features/transcription/presentation/transcription_screen.dart (1)
205-219:⚠️ Potential issue | 🟡 MinorMedical Insights button enabled condition mismatches its data.
Line 209 checks
controller.summary.isNotEmptybut the screen displayssymptomsandmedicines. This creates two failure modes:
- If AI extracts symptoms/medicines but summary is empty, the button remains disabled.
- If summary exists but both lists are empty, users navigate to an empty insights screen.
The condition should check the actual data being displayed:
🐛 Proposed fix
_buildNavigationButton( context, 'Medical Insights', Icons.analytics, - controller.summary.isNotEmpty && !controller.isProcessing, + (controller.symptoms.isNotEmpty || controller.medicines.isNotEmpty) && !controller.isProcessing, () => Navigator.push( context, MaterialPageRoute( builder: (_) => MedicalInsightsScreen( symptoms: controller.symptoms, medicines: controller.medicines, ), ), ), ),This issue is related to a prior review comment that flagged a similar mismatch.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/features/transcription/presentation/transcription_screen.dart` around lines 205 - 219, The button enabling condition currently uses controller.summary.isNotEmpty but the MedicalInsightsScreen is built with controller.symptoms and controller.medicines; update the _buildNavigationButton call so its enabled predicate checks the actual data: use (controller.symptoms.isNotEmpty || controller.medicines.isNotEmpty) combined with !controller.isProcessing, ensuring navigation only occurs when at least one of symptoms or medicines has data before pushing MedicalInsightsScreen.
🤖 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/features/transcription/presentation/transcription_screen.dart`:
- Around line 155-164: The empty-state condition mismatches the sentinel string:
update the check in the widget that compares controller.transcription to the
sentinel so it matches TranscriptionController's value exactly; specifically
change the comparison against "No speech detected." to "No speech detected"
where the TranscriptionState.done branch checks controller.transcription so the
empty state renders correctly when TranscriptionController returns the sentinel.
---
Duplicate comments:
In `@lib/features/transcription/presentation/transcription_screen.dart`:
- Around line 205-219: The button enabling condition currently uses
controller.summary.isNotEmpty but the MedicalInsightsScreen is built with
controller.symptoms and controller.medicines; update the _buildNavigationButton
call so its enabled predicate checks the actual data: use
(controller.symptoms.isNotEmpty || controller.medicines.isNotEmpty) combined
with !controller.isProcessing, ensuring navigation only occurs when at least one
of symptoms or medicines has data before pushing MedicalInsightsScreen.
🪄 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: 9ac4e696-dec4-4a2f-9014-6260abbbb686
📒 Files selected for processing (1)
lib/features/transcription/presentation/transcription_screen.dart
lib/features/transcription/presentation/transcription_screen.dart
Outdated
Show resolved
Hide resolved
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.
Overview
This PR fixes UI issues in the Transcription Screen and updates the navigation flow to align with the new structured AI response system.
Fixes
1. RenderFlex Overflow
ExpandedandTextOverflow.ellipsisto ensure proper layout on all screen sizes2. Removed Deprecated Field Usage
controller.prescriptionwith structured data (medicines)Improvements
3. Introduced Medical Insights Screen
MedicalInsightsScreento display:4. UX Enhancements
Impact
🔗 Related Issue
Closes #31
✅ Checklist
We encourage contributors to use AI tools responsibly when creating Pull Requests. While AI can be a valuable aid, it is essential to ensure that your contributions meet the task requirements, build successfully, include relevant tests, and pass all linters. Submissions that do not meet these standards may be closed without warning to maintain the quality and integrity of the project. Please take the time to understand the changes you are proposing and their impact.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores