-
Notifications
You must be signed in to change notification settings - Fork 110
fix: add chromadb dependency and health endpoint #117
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?
fix: add chromadb dependency and health endpoint #117
Conversation
📝 WalkthroughWalkthroughAdded a new /health HTTP endpoint to the Flask backend that returns JSON with status and agent_initialized; changed app run to debug=False; expanded backend setup and quick-start documentation with step-by-step venv and run instructions pointing to http://127.0.0.1:5000. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
Backend/app.py (2)
39-42: Critical: Missing SKIP_AGENT_INIT implementation - agent initialization still blocks startup.The PR objectives from issue #100 require adding a SKIP_AGENT_INIT environment variable to make agent initialization non-blocking, allowing Flask to start promptly for API development. However, the agent is unconditionally initialized at module level, which will still cause the blocking behavior described in the linked issue.
Wrap agent initialization in a conditional check:
# Initialize agent with database path db_path = os.path.join(os.path.dirname(__file__), "db", "database.db") first_time_setup() # This needs to be called before initializing the agent -agent = get_agent(db_path) +# Skip agent initialization in dev mode if requested +agent = None +if os.environ.get('SKIP_AGENT_INIT') != '1': + agent = get_agent(db_path) +else: + print("⚠️ Agent initialization skipped (SKIP_AGENT_INIT=1)")
44-62: Add defensive checks for agent-dependent endpoints.Once SKIP_AGENT_INIT is implemented, agent-dependent endpoints like
/agentwill fail withAttributeErrorwhen the agent isNone. Add guard clauses to return appropriate errors.@app.route("/agent", methods=["POST"]) def run_agent(): + if agent is None: + return jsonify({"error": "Agent not initialized. Start without SKIP_AGENT_INIT=1"}), 503 + if not request.is_json: return jsonify({"error": "Invalid JSON format"}), 400Apply similar checks to all agent-dependent endpoints:
/agent/cache/status(line 66),/agent/context(line 90),/agent/tasks/recommendations(line 116),/agent/cache/stats(line 147), and/agent/cache/cleanup(line 170).Setup.md (1)
9-9: Contradictory Python version requirements for Flask backend.Line 9 specifies Python 3.8+, but line 97 specifies Python 3.12+ for the backend. Additionally, line 194 reverts to 3.8+. Clarify the actual minimum Python version required for the Flask backend and update all references consistently.
🧹 Nitpick comments (1)
Setup.md (1)
102-102: Add language specifiers to code blocks.Static analysis identified code blocks without language specifiers, which reduces syntax highlighting and readability.
Apply language specifiers to the code blocks:
- ``` + ```bash cd Backend ```- ``` + ```bash python -m venv .venv source .venv/bin/activate ```- ``` + ```bash pip install -r requirements.txt ```-``` +```bash python app.pyAlso applies to: 106-106, 111-111, 117-117 </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: CodeRabbit UI **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between a76d240fae2788a78479266ee26390d989f7bdfd and fac61d98aefad0a7dee95779e8719d7c76d29705. </details> <details> <summary>📒 Files selected for processing (2)</summary> * `Backend/app.py` (1 hunks) * `Setup.md` (1 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🪛 markdownlint-cli2 (0.18.1)</summary> <details> <summary>Setup.md</summary> 102-102: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 106-106: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 111-111: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 117-117: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 120-120: Bare URL used (MD034, no-bare-urls) </details> </details> </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Backend/app.py
Outdated
| @app.route('/health') | ||
| def health(): | ||
| return jsonify({ | ||
| 'status': 'ok', | ||
| 'agent_initialized': True # Since agent is always initialized in this version | ||
| }) |
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.
Critical: Health endpoint doesn't reflect actual agent initialization status.
The PR objectives require the health endpoint to indicate whether the agent is initialized, supporting a response like {"agent_initialized": false, "status": "ok"} when running with SKIP_AGENT_INIT=1. However, this endpoint hardcodes agent_initialized: True, which will be incorrect if the SKIP_AGENT_INIT feature is implemented.
The endpoint should check whether the agent was actually initialized:
+# Global agent variable
+agent = None
+
@app.route('/health')
def health():
return jsonify({
'status': 'ok',
- 'agent_initialized': True # Since agent is always initialized in this version
+ 'agent_initialized': agent is not None
})📝 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.
| @app.route('/health') | |
| def health(): | |
| return jsonify({ | |
| 'status': 'ok', | |
| 'agent_initialized': True # Since agent is always initialized in this version | |
| }) | |
| # Global agent variable | |
| agent = None | |
| @app.route('/health') | |
| def health(): | |
| return jsonify({ | |
| 'status': 'ok', | |
| 'agent_initialized': agent is not None | |
| }) |
🤖 Prompt for AI Agents
In Backend/app.py around lines 31-36, the health endpoint currently hardcodes
"agent_initialized: True" which must instead reflect runtime state; change the
route to read a module-level boolean (e.g., AGENT_INITIALIZED) that your startup
code sets when the agent is initialized and fall back to checking
os.environ.get('SKIP_AGENT_INIT') == '1' (negated) if the flag is missing;
return that boolean in the JSON response and ensure the variable is properly
declared/updated during app initialization so the endpoint always reports the
real agent initialization status.
| ## Backend Setup (Offline Flask API) | ||
| BabyNest includes an offline backend using Flask. | ||
|
|
||
| ### 1. Install Python Dependencies | ||
| sh | ||
| pip install flask flask-cors chromadb | ||
|
|
||
|
|
||
| ### 2. Run the Backend Locally | ||
| sh | ||
| cd Backend | ||
| BabyNest includes an offline backend using Flask with ChromaDB for vector search. | ||
|
|
||
| ### Prerequisites | ||
| - Python 3.12+ | ||
| - Virtual environment (venv) | ||
|
|
||
| ### Installation | ||
| 1. Navigate to the Backend directory: | ||
| ``` | ||
| cd Backend | ||
| ``` | ||
| 2. Create and activate a virtual environment: | ||
| ``` | ||
| python -m venv .venv | ||
| source .venv/bin/activate | ||
| ``` | ||
| 3. Install dependencies: | ||
| ``` | ||
| pip install -r requirements.txt | ||
| ``` | ||
|
|
||
| ### Running the Backend | ||
| Start the server with: | ||
| ``` | ||
| python app.py | ||
|
|
||
|
|
||
| This will start the API locally. | ||
| ``` | ||
| The server will run on http://127.0.0.1:5000. Check /health for status. | ||
|
|
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.
Document the SKIP_AGENT_INIT environment variable for fast dev mode.
The PR objectives require documenting "Full Mode" vs. "Fast Dev Mode" with instructions for using SKIP_AGENT_INIT=1, but this section only shows basic startup. Developers need to know how to skip agent initialization during development.
Add documentation for the development mode:
### Running the Backend
-Start the server with:
+Start the server:
+
+**Full Mode (with Agent):**python app.py
-The server will run on http://127.0.0.1:5000. Check /health for status.
+
+**Fast Dev Mode (skip Agent initialization):**
+```
+SKIP_AGENT_INIT=1 python app.py
+```
+
+The server will run on <http://127.0.0.1:5000>. Check `/health` for status.
This also resolves the bare URL warning by using angle brackets.
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
102-102: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
106-106: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
111-111: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
117-117: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
120-120: Bare URL used
(MD034, no-bare-urls)
|
@sharma-sugurthi there are merge conflicts in setup.md |
|
@bhavik-mangla I will solve those. |
|
@bhavik-mangla please review it. |
|
missing cd Backend in readme |
- 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
b6a3d82 to
a5674a1
Compare
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Backend/app.py (2)
31-35: Critical: Missing SKIP_AGENT_INIT implementation required by PR objectives.The PR objectives and linked issue #100 explicitly require implementing
SKIP_AGENT_INITto skip heavy vector-store setup during development ("Fast Dev Mode"). However, the agent is always initialized unconditionally here—there's no check for the environment variable. This means:
- The server will still hang during startup due to blocking ChromaDB initialization
- The
/healthendpoint will always reportagent_initialized: true- The "Fast Dev Mode" workflow documented in the PR objectives cannot function
🔎 Implement conditional agent initialization
# Initialize agent with database path db_path = os.path.join(os.path.dirname(__file__), "db", "database.db") first_time_setup() # This needs to be called before initializing the agent -agent = get_agent(db_path) +# Support skipping agent initialization for fast dev startup +if os.environ.get('SKIP_AGENT_INIT') == '1': + agent = None + print("⚠️ Agent initialization skipped (SKIP_AGENT_INIT=1). /agent endpoints will not work.") +else: + agent = get_agent(db_path)
37-56: Add guard for agent endpoints when agent is uninitialized.When
SKIP_AGENT_INIT=1is used,agentwill beNoneand callingagent.run()will raiseAttributeError. Protect the endpoint to return a clear error.🔎 Add agent initialization check
@app.route("/agent", methods=["POST"]) def run_agent(): + if agent is None: + return jsonify({"error": "Agent not initialized. Start without SKIP_AGENT_INIT=1 to enable agent features."}), 503 + if not request.is_json: return jsonify({"error": "Invalid JSON format"}), 400Apply similar guards to other agent-dependent endpoints (lines 59-175).
♻️ Duplicate comments (1)
Setup.md (1)
143-151: Document SKIP_AGENT_INIT environment variable per PR objectives.The PR objectives explicitly require documenting "Full Mode" vs. "Fast Dev Mode" with instructions for using
SKIP_AGENT_INIT=1to skip agent initialization. This allows developers to start the server quickly without waiting for ChromaDB setup. The current documentation only shows basic startup.🔎 Add Fast Dev Mode documentation
### Running the Backend -Start the server with: +Start the server: + +**Full Mode (with Agent):** +```bash +python app.py +``` + +**Fast Dev Mode (skip Agent initialization):** +```bash +SKIP_AGENT_INIT=1 python app.py +``` + +The server will run on <http://127.0.0.1:5000>. Check `/health` for status. +``` -``` -python app.py -``` - -This will start the API locally. - -The server will run on http://127.0.0.1:5000. Check `/health` for status.This also addresses markdown linting warnings by adding language specifiers and using angle brackets for the bare URL.
🧹 Nitpick comments (2)
Setup.md (1)
130-141: Add language specifiers to code blocks.Markdown linting tools recommend specifying languages for fenced code blocks to enable proper syntax highlighting.
🔎 Add language identifiers
1. Navigate to the Backend directory: - ``` + ```bash cd Backend ``` 2. Create and activate a virtual environment: - ``` + ```bash python -m venv .venv source .venv/bin/activate ``` 3. Install dependencies: - ``` + ```bash pip install -r requirements.txt ```Based on static analysis hints.
README.md (1)
33-37: Optional: Vary sentence structure for readability.Three consecutive sentences start with "For". Consider rewording line 37 to improve flow, e.g., "See Setup.md for detailed setup instructions."
Based on static analysis hints.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
Backend/app.pyBackend/requirements.txtREADME.mdSetup.md
🧰 Additional context used
🪛 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)
🪛 markdownlint-cli2 (0.18.1)
Setup.md
130-130: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
134-134: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
139-139: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
145-145: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
151-151: Bare URL used
(MD034, no-bare-urls)
🪛 Ruff (0.14.10)
Backend/app.py
193-193: Possible binding to all interfaces
(S104)
🔇 Additional comments (3)
Backend/app.py (2)
185-190: Health endpoint implementation looks good.The endpoint correctly checks
agent is not None, which will properly reportagent_initialized: falseonce theSKIP_AGENT_INITfeature (flagged above) is implemented.
193-193: Verify intended binding and debug setting.Two changes here:
debug=False— reasonable for production-like behavior, but confirm this is intentional (development typically usesdebug=Truefor auto-reload and better error pages)host='0.0.0.0'— binds to all network interfaces. If this backend is only for local development, considerhost='127.0.0.1'to reduce exposure.Based on static analysis hints.
README.md (1)
31-37: Quick Start section is clear and helpful.The backend setup command on line 35 includes
cd Backend, addressing the concern raised by bhavik-mangla. The reference to Setup.md provides a good path for users who need more detailed instructions.
Description
This PR addresses the following issues:
chromadbdependency to requirements.txt/healthendpoint to check server statusChanges
chromadbto Backend/requirements.txt/healthendpoint to Backend/app.pyTesting
SKIP_AGENT_INIT=1/healthendpoint returns expected responseFixes #100
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.