Skip to content

Implement retry button on failed LLM prompt#166

Open
alextopalova wants to merge 2 commits into
StanfordBDHG:mainfrom
alextopalova:retry-failed-prompt
Open

Implement retry button on failed LLM prompt#166
alextopalova wants to merge 2 commits into
StanfordBDHG:mainfrom
alextopalova:retry-failed-prompt

Conversation

@alextopalova
Copy link
Copy Markdown

Implement retry button on failed LLM prompt

♻️ Current situation & Problem

Issue #114
When an LLM prompt fails (e.g. due to a network error or timeout), the user has no way to retry without restarting their session or re-navigating to the chat.

⚙️ Release Notes

  • Added a Retry button to the error alert shown when an LLM prompt fails in the user study chat
  • When tapped, the retry action strips any incomplete assistant output from the session context and re-triggers response generation automatically

📚 Documentation

  • retryLastPrompt() is documented inline with a doc comment explaining the context-cleanup behavior before re-triggering generation
  • The viewStateAlert(state:onRetry:) View extension replaces the default .viewStateAlert modifier in UserStudyChatView to support the retry callback
  • A localization entry for "Retry" has been added to Localizable.xcstrings

✅ Testing

The UI retry flow was manually verified:

  • Simulated an API key issue by deforming the key
  • Error alert presents with both Retry and Dismiss options
  • Tapping Retry clears the incomplete assistant message, resets session state to .ready, and re-triggers generateAssistantResponse()
  • Tapping Dismiss returns the view to idle without retrying

Code of Conduct & Contributing Guidelines

By creating and submitting this pull request, you agree to follow our Code of Conduct and Contributing Guidelines:

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 5, 2026

📝 Walkthrough

Walkthrough

This PR adds error recovery functionality by introducing a retry mechanism for failed LLM prompts. When the LLM session enters an error state, users can now dismiss the error and retry the last prompt, which removes incomplete context and regenerates the assistant response.

Changes

Error Recovery with Retry Capability

Layer / File(s) Summary
Retry Logic
LLMonFHIR/FHIRInterpretation/UserStudy/UserStudyChatViewModel.swift
retryLastPrompt() method removes the last incomplete context entry, resets session state to .ready, and asynchronously triggers assistant generation again.
Error Alert UI
LLMonFHIR/FHIRInterpretation/UserStudy/UserStudyChatView.swift
New View.viewStateAlert(state:onRetry:) extension displays an alert when viewState is .error, with "Retry" and "Dismiss" buttons; dismiss resets state to .idle, and "Retry" invokes the callback.
View Wiring
LLMonFHIR/FHIRInterpretation/UserStudy/UserStudyChatView.swift
onChange(of: model.llmSession.state) modifier now uses the new viewStateAlert helper, wiring the retry button to model.retryLastPrompt().
Localization
LLMonFHIR/Resources/Localizable.xcstrings
New "Retry" localization string entry with description for the retry button.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: adding a retry button to handle failed LLM prompts, which directly matches the primary purpose of the changeset.
Description check ✅ Passed The description is well-structured and comprehensive, covering the problem statement, release notes, documentation, and manual testing verification, all directly related to the retry functionality implementation.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
LLMonFHIR/FHIRInterpretation/UserStudy/UserStudyChatView.swift (1)

141-163: 💤 Low value

Move viewStateAlert(state:onRetry:) to a dedicated utility file.

The extension targets the module-level View protocol but lives inside a view-specific file. Placing it in its own file (e.g., View+ViewStateAlert.swift) improves discoverability and makes the intent clear — especially since the existing SpeziViews viewStateAlert(state:) is the peer being extended.

🤖 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 `@LLMonFHIR/FHIRInterpretation/UserStudy/UserStudyChatView.swift` around lines
141 - 163, Move the extension View containing viewStateAlert(state:onRetry:) out
of UserStudyChatView.swift into its own file (e.g., View+ViewStateAlert.swift);
ensure the new file imports SwiftUI, declares the same extension View and the
viewStateAlert(state: Binding<ViewState>, onRetry: `@escaping` () -> Void) method
unchanged, and keep the same access level so callers still resolve, then remove
the original extension from UserStudyChatView.swift to avoid duplicate symbols
and improve discoverability relative to the existing SpeziViews
viewStateAlert(state:).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@LLMonFHIR/FHIRInterpretation/UserStudy/UserStudyChatView.swift`:
- Around line 141-163: Move the extension View containing
viewStateAlert(state:onRetry:) out of UserStudyChatView.swift into its own file
(e.g., View+ViewStateAlert.swift); ensure the new file imports SwiftUI, declares
the same extension View and the viewStateAlert(state: Binding<ViewState>,
onRetry: `@escaping` () -> Void) method unchanged, and keep the same access level
so callers still resolve, then remove the original extension from
UserStudyChatView.swift to avoid duplicate symbols and improve discoverability
relative to the existing SpeziViews viewStateAlert(state:).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c1af981c-680d-44b1-a274-e9fcf26cfdc2

📥 Commits

Reviewing files that changed from the base of the PR and between b1de795 and a062281.

📒 Files selected for processing (3)
  • LLMonFHIR/FHIRInterpretation/UserStudy/UserStudyChatView.swift
  • LLMonFHIR/FHIRInterpretation/UserStudy/UserStudyChatViewModel.swift
  • LLMonFHIR/Resources/Localizable.xcstrings

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant