-
Notifications
You must be signed in to change notification settings - Fork 110
Improve frontend behavior when AI agent is unavailable #134
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Improve frontend behavior when AI agent is unavailable #134
Conversation
📝 WalkthroughWalkthroughThis PR adds Expo configuration support and implements agent availability detection in ChatScreen. New dependencies (react-dom, react-native-web, Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Frontend/src/Screens/ChatScreen.jsx (1)
222-240: Missing fallback to local model when agent is unavailable.The PR objectives explicitly state: "Auto-switch chat behavior to a model-only mode when the agent is unavailable: bypass RAG/agent flows (no /agent or /agent/context calls) and use generateResponse() with the local GGUF model."
However, when
agentAvailableisfalse, the code shows an alert and returns (lines 168-174, 346-354) rather than falling back to the local model.🔎 Proposed fallback logic
try { let response = null; let result = null; - // Initialize RAG service - try { - await ragService.initialize(); - } catch (e) { - console.warn("Agent initialization skipped or failed"); - setAgentAvailable(false); - throw new Error("Agent unavailable"); - } - - - - // Set user context - conversationContext.setUserContext(context); - - // Check toggle mode first - if (useRAGMode) { + if (agentAvailable && useRAGMode) { + // Initialize RAG service only if agent is available + try { + await ragService.initialize(); + } catch (e) { + console.warn("Agent initialization failed, falling back to local model"); + setAgentAvailable(false); + // Fall through to local model below + } + } + + if (agentAvailable && useRAGMode) { + // Set user context + conversationContext.setUserContext(context); + // RAG Mode (Robot) - Process structured commands if (conversationContext.hasPendingFollowUp()) { console.log('🤖 Processing follow-up response with RAG...'); result = await conversationContext.processFollowUpResponse(userInput, ragService); } else { console.log('🤖 Processing new query with RAG...'); result = await ragService.processQuery(userInput, context); } } else { - // Model Mode (Phone) - Use backend model for general chat - console.log('📞 Processing with backend model...'); + // Local Model Mode - Use local GGUF model + console.log('📞 Processing with local model (agent unavailable)...'); response = await generateResponse(updatedConversation); result = { message: response, intent: 'general_chat', action: null }; }This allows the chat to continue functioning with the local model when the agent is unavailable, as specified in the objectives.
🧹 Nitpick comments (3)
Frontend/src/Screens/ChatScreen.jsx (3)
28-28: Remove unnecessary whitespace-only changes.These lines contain only whitespace modifications that don't add value and make the diff harder to review. Consider reverting these formatting changes unless they're part of a broader code formatting initiative.
Also applies to: 44-44, 161-161, 251-251, 304-304, 322-322, 335-335, 339-339, 342-342, 374-374, 376-376, 378-378, 416-416, 433-433, 444-444, 449-449, 454-454, 465-465
904-934: Markdown style changes appear to be formatting only.The changes to
createMarkdownStylesare primarily formatting (adding/removing whitespace). Unless there's a specific reason for reformatting, these changes add noise to the PR focused on agent availability.Consider reverting these formatting changes or applying them in a separate PR dedicated to code formatting.
22-22: Consider managing agent availability in AgentContext instead of local state.The PR objectives mention "Detect agent availability in the frontend (AgentContext.jsx / ChatScreen.jsx)". Since you're already using
useAgentContextand it hasinitializeContextandisInitialized, it would be more maintainable to:
- Add
agentAvailablestate to AgentContext- Have AgentContext perform the health check
- Consume it here via
const { agentAvailable } = useAgentContext()This would:
- Centralize agent state management
- Allow other components to react to agent availability
- Avoid duplicate health checks if multiple components need this information
- Better align with the PR objectives
Example implementation in AgentContext.jsx:
const [agentAvailable, setAgentAvailable] = useState(false); useEffect(() => { const checkHealth = async () => { try { const response = await fetch(`${BASE_URL}/health`); const data = await response.json(); setAgentAvailable(data.agent_initialized === true); } catch (error) { setAgentAvailable(false); } }; checkHealth(); }, []); // Return agentAvailable in the context valueAlso applies to: 182-189
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
Frontend/.expo/README.mdFrontend/.expo/devices.jsonFrontend/package.jsonFrontend/src/Screens/ChatScreen.jsx
🧰 Additional context used
🧬 Code graph analysis (1)
Frontend/src/Screens/ChatScreen.jsx (4)
Frontend/src/services/RAGService.js (2)
ragService(3664-3664)ragService(3664-3664)Frontend/src/context/AgentContext.jsx (2)
refreshContext(68-94)error(29-29)Frontend/src/services/ConversationContext.js (2)
conversationContext(384-384)conversationContext(384-384)Frontend/src/theme/ThemeContext.js (1)
theme(63-63)
🔇 Additional comments (1)
Frontend/src/Screens/ChatScreen.jsx (1)
674-684: Send button opacity and disabled state are correct.The send button correctly reflects both
agentAvailableandisGeneratingstates. Once the proactive health check is implemented, this will work as intended from the initial render.Minor suggestion: You could simplify the disabled condition for clarity:
disabled={!agentAvailable || isGenerating}matches the opacity condition perfectly.
| { | ||
| "devices": [] | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not commit the .expo folder.
The Frontend/.expo/README.md file added in this same PR explicitly states: "No, you should not share the '.expo' folder" and "Upon project creation, the '.expo' folder is already added to your '.gitignore' file." This contradicts the inclusion of these files in the PR.
Remove the .expo folder from this PR and ensure it's listed in .gitignore.
🤖 Prompt for AI Agents
In Frontend/.expo/devices.json around lines 1 to 3, the committed .expo folder
should not be included in the repo; remove the .expo directory from the PR and
ensure it is ignored: delete the .expo files from the commit (use git rm
--cached -r Frontend/.expo or remove and re-commit without them), add or confirm
Frontend/.gitignore contains an entry for /.expo or .expo/ (and commit that
change), and verify the .expo folder is no longer tracked (git status) before
pushing the updated branch.
| > Why do I have a folder named ".expo" in my project? | ||
| The ".expo" folder is created when an Expo project is started using "expo start" command. | ||
| > What do the files contain? | ||
| - "devices.json": contains information about devices that have recently opened this project. This is used to populate the "Development sessions" list in your development builds. | ||
| - "settings.json": contains the server configuration that is used to serve the application manifest. | ||
| > Should I commit the ".expo" folder? | ||
| No, you should not share the ".expo" folder. It does not contain any information that is relevant for other developers working on the project, it is specific to your machine. | ||
| Upon project creation, the ".expo" folder is already added to your ".gitignore" file. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this file from the PR.
This README explicitly documents that the .expo folder "should not be shared" and "is already added to your .gitignore file" (line 7-8), yet you're committing it to the repository. This is inconsistent with Expo best practices.
🤖 Prompt for AI Agents
In Frontend/.expo/README.md lines 1-8: this file documents that the .expo folder
should not be shared yet the folder is being committed; remove the .expo folder
and its README from the PR, ensure .expo is listed in Frontend/.gitignore (or
the repo root .gitignore), and untrack any already committed .expo files by
running git rm -r --cached Frontend/.expo (or remove tracked files via your Git
client), then commit the removal and push; do not add any .expo content back
into the repo.
| "react-dom": "18.3.1", | ||
| "react-native-web": "~0.19.13", | ||
| "@expo/metro-runtime": "~4.0.1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for usage of the newly added dependencies in Frontend directory
echo "=== Searching for react-dom imports ==="
rg -n "from ['\"]react-dom['\"]|import.*react-dom|require\(['\"]react-dom['\"]\)" Frontend/ --type=js --type=jsx --type=ts --type=tsx 2>/dev/null || echo "No react-dom imports found"
echo -e "\n=== Searching for react-native-web imports ==="
rg -n "from ['\"]react-native-web['\"]|import.*react-native-web|require\(['\"]react-native-web['\"]\)" Frontend/ --type=js --type=jsx --type=ts --type=tsx 2>/dev/null || echo "No react-native-web imports found"
echo -e "\n=== Searching for @expo/metro-runtime imports ==="
rg -n "from ['\"]@expo/metro-runtime['\"]|import.*@expo/metro-runtime|require\(['\"]@expo/metro-runtime['\"]\)" Frontend/ --type=js --type=jsx --type=ts --type=tsx 2>/dev/null || echo "No @expo/metro-runtime imports found"
echo -e "\n=== Checking Frontend/package.json for context ==="
cat Frontend/package.json 2>/dev/null | head -60Repository: AOSSIE-Org/BabyNest
Length of output: 2516
🏁 Script executed:
#!/bin/bash
# Check for build configuration files that might use these packages
echo "=== Looking for metro/webpack/build config files ==="
fd -e "metro.config.js" -e "webpack.config.js" -e "expo.json" -e "app.json" -e "babel.config.js"
echo -e "\n=== Checking if there's a web subdirectory or web-specific project ==="
find . -maxdepth 2 -type d \( -name "web" -o -name "Web" -o -name "www" \) 2>/dev/null
echo -e "\n=== Checking repo root structure ==="
ls -la | head -20
echo -e "\n=== Checking for any Expo or web configuration in Frontend ==="
cat Frontend/babel.config.js 2>/dev/null || echo "No babel.config.js"Repository: AOSSIE-Org/BabyNest
Length of output: 1527
🏁 Script executed:
#!/bin/bash
# Check git history to see if these dependencies were newly added or already present
echo "=== Checking git history for these dependencies ==="
git log --oneline -20 -- Frontend/package.json
echo -e "\n=== Checking if these packages were added recently ==="
git diff HEAD~1 Frontend/package.json 2>/dev/null | grep -A 2 -B 2 "react-dom\|react-native-web\|@expo/metro-runtime" || echo "Unable to compare with previous commit"
echo -e "\n=== Checking current branch/PR info ==="
git statusRepository: AOSSIE-Org/BabyNest
Length of output: 581
🏁 Script executed:
#!/bin/bash
# Get the detailed diff of the most recent commit for package.json
echo "=== Checking the most recent commit diff for Frontend/package.json ==="
git show f0b5245:Frontend/package.json 2>/dev/null | tail -20
echo -e "\n=== Checking the actual diff of what changed ==="
git show f0b5245 -- Frontend/package.json 2>/dev/null | head -100Repository: AOSSIE-Org/BabyNest
Length of output: 3753
Remove unused Expo web dependencies from package.json.
The dependencies react-dom, react-native-web, and @expo/metro-runtime added in this PR are not imported anywhere in the codebase and are incompatible with this React Native mobile-only project. There is no web build configuration (no metro.config.js, webpack.config.js, or expo.json), no web platform targets, and no justification for these packages. Since the PR is focused on improving behavior when the AI agent is unavailable, these unrelated dependencies should be removed.
🤖 Prompt for AI Agents
In Frontend/package.json around lines 46 to 48, remove the unused Expo/web
dependencies "react-dom", "react-native-web", and "@expo/metro-runtime" because
they are not imported anywhere, incompatible with this mobile-only React Native
project, and there is no web build config; update package.json by deleting these
three entries and then run npm/yarn install to update the lockfile and ensure no
residual references remain.
| import { conversationContext } from '../services/ConversationContext'; | ||
| import { conversationContext } from '../services/ConversationContext'; | ||
| export default function ChatScreen() { | ||
| const [agentAvailable, setAgentAvailable] = useState(true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agent availability should be checked proactively, not initialized optimistically.
Initializing agentAvailable to true means the UI will allow user input and message sending even when the agent is actually unavailable. The PR objectives require detecting agent availability using /health or error responses from /agent/context on initialization.
🔎 Recommended approach
- Add a proactive health check in
useEffecton component mount:
+useEffect(() => {
+ const checkAgentAvailability = async () => {
+ try {
+ const response = await fetch(`${BASE_URL}/health`);
+ const data = await response.json();
+ setAgentAvailable(data.agent_initialized === true);
+ } catch (error) {
+ console.warn('Agent health check failed:', error);
+ setAgentAvailable(false);
+ }
+ };
+ checkAgentAvailability();
+}, []);- Initialize state to
false(pessimistic) ornull(unknown) until the health check completes:
-const [agentAvailable, setAgentAvailable] = useState(true);
+const [agentAvailable, setAgentAvailable] = useState(false);This ensures the UI correctly reflects the agent's state from the start.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In Frontend/src/Screens/ChatScreen.jsx around line 19, agentAvailable is
optimistically initialized to true which allows UI interaction before confirming
the agent is reachable; change initialization to a pessimistic value (false or
null) and add a useEffect hook that runs on mount to call the agent health
endpoint (/health or /agent/context), update agentAvailable based on the
response, handle network/errors by setting it to false, and ensure the UI
(input/send) is disabled while the check is pending or when agentAvailable is
false.
| if (!agentAvailable) { | ||
| Alert.alert( | ||
| "AI Unavailable", | ||
| "The AI assistant is currently disabled. Offline features are still accessible." | ||
| ); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Logic flaw: This pre-check can never trigger on the first message.
The guard if (!agentAvailable) at the beginning of handleSendMessage will never execute on the first message because:
agentAvailableis initialized totrue(line 19)- It's only set to
falseat line 212, which happens AFTER this pre-check runs - The user can type and click send before the backend availability is verified
The current flow is reactive (detects unavailability after a failed send attempt) rather than proactive (preventing the attempt in the first place). Combine this fix with the proactive health check suggested in the previous comment.
🤖 Prompt for AI Agents
In Frontend/src/Screens/ChatScreen.jsx around lines 168 to 174, the pre-check if
(!agentAvailable) can never block the very first send because agentAvailable is
initialized true and only flipped later; fix by making availability proactive:
initialize agentAvailable to null or false, run an agent health check on
component mount (or before first send) to set agentAvailable appropriately, and
modify handleSendMessage to await or re-check availability (or call the health
check) before attempting to send so the guard can actually prevent sending when
the backend is down.
| try { | ||
| await ragService.initialize(); | ||
| } catch (e) { | ||
| console.warn("Agent initialization skipped or failed"); | ||
| setAgentAvailable(false); | ||
| throw new Error("Agent unavailable"); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This reactive detection should be supplemented with proactive initialization.
While catching ragService.initialize() failures is good for handling runtime degradation, it doesn't prevent the first failed attempt. The user will:
- Type a message
- Click send
- Wait for the request to fail
- See an error alert
- Only then have the UI disable
The PR objectives state: "Detect agent availability in the frontend... using /health or error responses from /agent/context and treat agent_initialized === false... as an 'agent unavailable' state." This should happen on mount, not during the first message send. See previous comments for the proactive health check approach.
| // Refresh context after successful command execution | ||
| if (result.success) { | ||
| await refreshContext(); | ||
| } | ||
| } else { | ||
| await refreshContext(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Context refresh after successful command may fail if agent becomes unavailable.
After a successful command execution, refreshContext() is called without checking if the agent is still available. If the agent becomes unavailable between the command execution and the context refresh, this could throw an unhandled error.
Add error handling around the context refresh:
if (result.success) {
- await refreshContext();
+ try {
+ await refreshContext();
+ } catch (error) {
+ console.warn('Failed to refresh context:', error);
+ // Context refresh is non-critical, continue without alerting user
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Refresh context after successful command execution | |
| if (result.success) { | |
| await refreshContext(); | |
| } | |
| } else { | |
| await refreshContext(); | |
| } | |
| // Refresh context after successful command execution | |
| if (result.success) { | |
| try { | |
| await refreshContext(); | |
| } catch (error) { | |
| console.warn('Failed to refresh context:', error); | |
| // Context refresh is non-critical, continue without alerting user | |
| } | |
| } |
🤖 Prompt for AI Agents
In Frontend/src/Screens/ChatScreen.jsx around lines 305 to 308, the call to
refreshContext() after a successful command can throw if the agent becomes
unavailable; wrap the await refreshContext() in a try/catch (or check agent
availability before calling) so any error is caught and handled gracefully, log
or surface a non-fatal warning/error (don't let it crash the UI), and optionally
avoid calling refreshContext when the agent status indicates it is unavailable.
| editable={agentAvailable} | ||
| placeholder={ | ||
| agentAvailable | ||
| ? "Ask me to book appointments, track health..." | ||
| : "AI assistant is currently unavailable" | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Input editable state has the same initialization issue.
Since agentAvailable is initialized to true, the input field will be enabled on mount even if the backend agent is actually unavailable. This allows the user to type before availability is confirmed.
After implementing the proactive health check (see earlier comments), consider also adding a loading state to show a different placeholder while the health check is in progress:
placeholder={
agentAvailable === null
? "Checking agent status..."
: agentAvailable
? "Ask me to book appointments, track health..."
: "AI assistant is currently unavailable"
}🤖 Prompt for AI Agents
In Frontend/src/Screens/ChatScreen.jsx around lines 665 to 670, the input's
editable prop and placeholder rely on agentAvailable being initialized to true
which enables typing before backend availability is confirmed; update the
component to treat agentAvailable as tri-state (null = loading, true =
available, false = unavailable), set initial agentAvailable to null, and change
editable to only be true when agentAvailable === true; also switch the
placeholder to show a "Checking agent status..." message when agentAvailable is
null, the normal prompt when true, and the unavailable message when false so the
UI reflects the proactive health check state.
|
@charuljain02 , i raised the issue for asking maintainers to assign me. |
|
Hi! @sharma-sugurthi Thank you so much for taking the time to explain the workflow to me. I'm actually quite new to this and I was just really eager to contribute and gain some hands-on experience, so I didn't realize I was stepping outside the project's etiquette. |
|
We dont want SKIP_AGENT_INIT flag |
Okay, in my beginning I was also exactly like you. Its nice to letting you know in this project.. |
Noted. Will go as you say... |
…E-Org#134) - Add SKIP_AGENT_INIT environment variable support in backend - Implement health check endpoint for agent availability detection - Add agent availability banner in chat screen UI - Modify RAG service to gracefully handle agent unavailability - Update documentation with agent availability behavior - Fix schema file path resolution in database module - Fix duplicate keys in theme context When agent is disabled, app shows clear warning and falls back to local model while maintaining structured command functionality.
🛠 Fix: Handle AI agent unavailable state gracefully
What changed
SKIP_AGENT_INIT)Why
Previously, the UI allowed interaction even when the AI agent was not initialized, leading to a confusing user experience.
This change ensures clear feedback and prevents invalid actions.
UI Behavior
AI Available
AI Unavailable
Related Issue
Closes #126
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.