Fix #134: Improve frontend behavior when backend agent is disabled#143
Fix #134: Improve frontend behavior when backend agent is disabled#143sharma-sugurthi wants to merge 2 commits intoAOSSIE-Org:mainfrom
Conversation
- Add chromadb==0.5.23 to requirements.txt - Implement /health endpoint in app.py - Update Setup.md with backend setup instructions - Add Quick Start in README.md with cd Backend Fixes AOSSIE-Org#100
…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.
📝 WalkthroughWalkthroughBackend conditionally initializes an agent based on the SKIP_AGENT_INIT flag and adds a /health endpoint to report agent status. Frontend now performs health checks on mount and propagates agent availability through context, services, and components. The backend and frontend coordinate to display a banner when the agent is unavailable, with fallback to local GGUF model for conversation handling. Changes
Sequence DiagramssequenceDiagram
participant ChatScreen
participant AgentContext
participant Backend as Backend /health
participant RAGService
participant ConversationContext
Note over ChatScreen,ConversationContext: Health Check on Mount
ChatScreen->>ChatScreen: useEffect on mount
ChatScreen->>AgentContext: call checkHealth()
AgentContext->>Backend: GET /health
Backend-->>AgentContext: {agent_initialized: true/false}
AgentContext->>AgentContext: set agentAvailable, healthChecked
AgentContext-->>ChatScreen: updated state
ChatScreen->>ChatScreen: render Agent Banner if agent unavailable
Note over ChatScreen,ConversationContext: RAG Query with Agent Awareness
ChatScreen->>RAGService: processQuery(userQuery, context, agentAvailable)
alt agentAvailable is true
RAGService->>RAGService: executeAction with backend call
else agentAvailable is false
RAGService->>RAGService: handleGeneralChat with local GGUF fallback
end
RAGService-->>ChatScreen: response
Note over ChatScreen,ConversationContext: Follow-up Processing
ChatScreen->>ConversationContext: processFollowUpResponse(query, ragService, agentAvailable)
ConversationContext->>ConversationContext: validate extractedData
alt validation passes
ConversationContext->>RAGService: executeAction(..., agentAvailable)
else validation fails
ConversationContext-->>ChatScreen: structured error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ 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.
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)
Frontend/src/Screens/ChatScreen.jsx (1)
296-321: Fallback path ignores agent availability state.When
resultis falsy or not an object, this fallback logic calls${BASE_URL}/agentwithout checkingagentAvailable. If the agent is known to be unavailable (via health check), this will still attempt the backend call before falling back to the local model, causing unnecessary latency.🔎 Proposed fix to check agentAvailable before backend call
} else { // Fallback to general chat if RAG doesn't understand - try { + if (!agentAvailable) { + // Skip backend call when agent is unavailable + response = await generateResponse(updatedConversation); + } else { + try { const agentResponse = await fetch(`${BASE_URL}/agent`, { method: 'POST', headers: { 'Content-Type': 'application/json', }, body: JSON.stringify({ query: userInput, user_id: "default" }), }); if (agentResponse.ok) { const agentData = await agentResponse.json(); response = agentData.response; } else { throw new Error('Backend agent request failed'); } - } catch (backendError) { - console.warn('Backend agent failed, falling back to local model:', backendError.message); - // Fallback to local model if backend is unavailable - response = await generateResponse(updatedConversation); + } catch (backendError) { + console.warn('Backend agent failed, falling back to local model:', backendError.message); + // Fallback to local model if backend is unavailable + response = await generateResponse(updatedConversation); + } } }
🧹 Nitpick comments (3)
README.md (1)
31-37: Helpful addition for quick onboarding.The Quick Start section provides a clear entry point for new contributors.
Consider adding a note that the backend activation command (
source .venv/bin/activate) is for Linux/macOS; Windows users should use.venv\Scripts\activate.Frontend/src/services/ConversationContext.js (1)
117-138: Consider consolidating the redundant validation checks.The validation at lines 117-123 checks
!extractedData || typeof extractedData !== 'object', and then line 132 checks essentially the same condition again (extractedData && typeof extractedData === 'object' && extractedData !== null). The second check is always true if we reach that point.🔎 Proposed simplification
// Ensure extractedData is a valid object if (!extractedData || typeof extractedData !== 'object') { console.error('Invalid extractedData:', extractedData); return { success: false, message: `❌ I'm having trouble processing that information. Could you try again?` }; } // Merge with existing partial data (only update non-null values) const mergedData = { ...(this.pendingFollowUp.partialData || {}) }; // Only update fields that have actual values (not null/undefined) - // Double-check that extractedData is still valid before Object.entries - if (extractedData && typeof extractedData === 'object' && extractedData !== null) { - for (const [key, value] of Object.entries(extractedData)) { - if (value !== null && value !== undefined && value !== '') { - mergedData[key] = value; - } + for (const [key, value] of Object.entries(extractedData)) { + if (value !== null && value !== undefined && value !== '') { + mergedData[key] = value; } }Frontend/src/context/AgentContext.jsx (1)
70-92: Robust health check implementation.The function handles all paths correctly: successful response, non-ok HTTP status, and network errors. The
finallyblock ensureshealthCheckedis always set totrue.Consider adding a timeout to prevent the health check from hanging indefinitely if the backend is slow or unresponsive:
🔎 Optional: Add fetch timeout
const checkHealth = async () => { try { - const response = await fetch(`${BASE_URL}/health`, { - method: 'GET', - headers: { - 'Content-Type': 'application/json', - }, - }); + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 5000); + + const response = await fetch(`${BASE_URL}/health`, { + method: 'GET', + headers: { + 'Content-Type': 'application/json', + }, + signal: controller.signal, + }); + + clearTimeout(timeoutId); if (response.ok) {
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
Backend/app.pyBackend/db/db.pyBackend/requirements.txtFrontend/README.mdFrontend/src/Screens/ChatScreen.jsxFrontend/src/context/AgentContext.jsxFrontend/src/services/ConversationContext.jsFrontend/src/services/RAGService.jsFrontend/src/theme/ThemeContext.jsREADME.mdSetup.md
🧰 Additional context used
🧬 Code graph analysis (4)
Frontend/src/Screens/ChatScreen.jsx (2)
Frontend/src/context/AgentContext.jsx (7)
useAgentContext(18-24)useAgentContext(18-24)checkHealth(70-92)agentAvailable(32-32)context(19-19)context(27-27)healthChecked(33-33)Frontend/src/services/RAGService.js (2)
ragService(3672-3672)ragService(3672-3672)
Frontend/src/services/RAGService.js (1)
Frontend/src/context/AgentContext.jsx (1)
agentAvailable(32-32)
Frontend/src/services/ConversationContext.js (2)
Frontend/src/services/RAGService.js (2)
ragService(3672-3672)ragService(3672-3672)Frontend/src/context/AgentContext.jsx (1)
agentAvailable(32-32)
Backend/app.py (1)
Backend/agent/agent.py (2)
get_agent(97-102)run(37-61)
🪛 LanguageTool
README.md
[style] ~36-~36: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...-r requirements.txt && python app.py`. For detailed setup instructions, see [Setup...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 Ruff (0.14.10)
Backend/app.py
218-218: Possible binding to all interfaces
(S104)
🔇 Additional comments (22)
Backend/db/db.py (1)
6-6: LGTM!The absolute path resolution ensures
schema.sqlis correctly located relative to this module, regardless of the current working directory. This is a more robust approach than relying on relative paths from the execution context.Frontend/src/theme/ThemeContext.js (1)
44-44: LGTM - Duplicate key removal.This change removes a duplicate theme key as mentioned in the PR objectives.
Note: Pre-existing issue - the
text(line 36) andiconText(line 40) values in the dark theme are missing the#prefix for hex colors (should be#fff3b0). Consider addressing this in a follow-up.Frontend/README.md (1)
97-109: LGTM!Clear documentation of the agent availability feature, covering scenarios and user-visible behavior. This helps developers understand the graceful degradation behavior introduced in this PR.
Frontend/src/services/ConversationContext.js (2)
107-107: LGTM - Good use of default parameter.The
agentAvailableparameter defaults totrue, maintaining backward compatibility with existing callers while allowing the new health-aware flow to pass the actual availability state.
186-186: No action required. TheexecuteActionmethod inRAGService.js(line 1449) already accepts theagentAvailableparameter with the correct signature:async executeAction(intent, data, userContext, agentAvailable = true). The call at line 186 inConversationContext.jsproperly matches this signature.Setup.md (1)
143-151: LGTM - Clear backend setup instructions.The running instructions are clear and the
/healthendpoint reference aligns with the new health check feature, providing users with a way to verify the server status.Frontend/src/context/AgentContext.jsx (2)
32-33: LGTM - Good default state values.
agentAvailabledefaults totrue(optimistic assumption until checked), andhealthCheckedstarts asfalseto track whether the check has completed. This allows UI to distinguish between "not yet checked" and "checked and unavailable."
187-193: LGTM - Context API properly exposed.All three new health-related values (
agentAvailable,healthChecked,checkHealth) are correctly exposed in the context value for consumers like ChatScreen.Backend/app.py (4)
34-42: LGTM - Clean conditional initialization.The environment variable check is robust, handling multiple truthy values. The lazy import of
get_agentonly when initialization is not skipped avoids loading agent dependencies unnecessarily, which aligns with the "fast dev mode" goal.
46-48: LGTM - Consistent guard pattern.The 503 response is appropriate for "Service Unavailable" when the agent is not initialized. This pattern is consistently applied across all agent-related endpoints.
210-215: LGTM - Health endpoint provides clear status.The
/healthendpoint returns both overall status and agent initialization state, enabling the frontend to determine agent availability and display appropriate UI.
218-218: Binding to all interfaces (0.0.0.0) - verify this is intentional.Binding to
0.0.0.0allows the server to accept connections from any network interface, which is necessary for Docker/container deployments but exposes the service to the network. The static analysis tool flags this as a potential security consideration (S104).This is likely intentional for the use case. Ensure the deployment environment has appropriate network security controls (firewall, reverse proxy, etc.) when running in production.
Frontend/src/Screens/ChatScreen.jsx (5)
21-21: LGTM!The destructuring correctly integrates the new agent availability state (
agentAvailable,healthChecked) and thecheckHealthfunction from the context, aligning with the AgentContext API defined inFrontend/src/context/AgentContext.jsx.
165-168: LGTM!The mount-only health check is appropriate to determine agent availability before user interaction. The empty dependency array is intentional here since
checkHealthis a stable reference from context.
213-216: LGTM!Correctly propagates
agentAvailableto both the follow-up response handler and the RAG query processor, enabling the service layer to make informed decisions about backend calls.
527-535: LGTM!The banner implementation is well-structured:
- Conditionally renders only after health check completes (
healthChecked) and when agent is unavailable- Uses theme-aware colors with sensible fallbacks (
#fff3cd,#856404for warning styling)- Clear, informative message explaining the degraded mode to users
726-745: LGTM!Banner styles are appropriately defined with proper flexbox layout, spacing, and shadow properties for both iOS and Android platforms.
Frontend/src/services/RAGService.js (5)
633-633: LGTM!The default parameter
agentAvailable = trueensures backward compatibility with existing callers while allowing new callers to explicitly control agent availability behavior.
656-656: LGTM!Correctly propagates
agentAvailableto the action execution layer.
1449-1449: LGTM!Consistent signature update with default value to maintain backward compatibility.
1499-1499: LGTM!Correctly passes
agentAvailableto thehandleGeneralChatmethod, which is the only action handler that needs to be aware of agent availability for backend calls.
2357-2365: LGTM!The early return when
!agentAvailableis a clean implementation of graceful degradation. Returning a generic helpful response instead of attempting a backend call that will fail provides better UX and avoids unnecessary latency.
Changes Made
SKIP_AGENT_INITenvironment variable support/healthendpoint for agent availability detectionTesting
SKIP_AGENT_INIT=1- returns proper 503 responsesBehavior
When
SKIP_AGENT_INIT=1is set:Summary by CodeRabbit
New Features
Bug Fixes
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.