Add structured /health endpoint with dependency checks - #76
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe service adds a structured ChangesHealth check integration
Environment cleanup
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Sequence Diagram(s)sequenceDiagram
participant KeepAwakeWorkflow
participant RenderService
participant HealthEndpoint
participant GeminiConfiguration
KeepAwakeWorkflow->>RenderService: GET /health
RenderService->>HealthEndpoint: route request
HealthEndpoint->>GeminiConfiguration: check API key configuration
GeminiConfiguration-->>HealthEndpoint: return health status
HealthEndpoint-->>KeepAwakeWorkflow: JSON response with 200 or 503
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 2
🤖 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.
Inline comments:
In `@main.py`:
- Line 752: Remove the extra whitespace before the colon in the "status" key
within the affected mapping, preserving the existing key and value while making
the syntax flake8 E203-compliant.
- Around line 747-750: Update the health endpoint’s deep handling around the
checks["gemini_api_reachable"] assignment: either implement a bounded Gemini
reachability probe that marks the check failed and returns a non-200 health
response when unavailable, or remove the deep parameter and its skipped check
until real behavior exists. Ensure deep=true changes the health verdict rather
than returning 200 with a skipped check.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
main.py (2)
448-453: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winApply the requested language to normal
/chatgeneration.Line 453 normalizes the language, but the non-streaming prompt never adds
LANGUAGE_INSTRUCTIONSorresponse_language(unlike the streaming path)./chatcan therefore return a response in the wrong language while reporting the requested language in its payload.Proposed fix
system_context = ISLAMIC_CONTEXT + HADITH_ADAB_CONTEXT + system_context += LANGUAGE_INSTRUCTIONS + if effective_language: + system_context += f"\nresponse_language: {effective_language}" + else: + system_context += "\nresponse_language: auto (respond in the user's language)" if is_fiqh:🤖 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 `@main.py` around lines 448 - 453, Update the non-streaming /chat generation flow after effective_language is computed in the classification block to apply LANGUAGE_INSTRUCTIONS and pass response_language, matching the existing streaming path. Ensure generated responses use the requested normalized language while preserving the reported language payload.
502-509: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDo not reuse cached responses across explicit language requests.
The semantic-cache lookup is keyed only by prompt embedding, so a cached English response can be returned to a later Arabic request for the same prompt. Line 508 then labels that response as Arabic. Partition the cache by language, or bypass it whenever a supported language was explicitly requested.
Minimal safe fix
is_cacheable = ( is_new_chat and request.context is None + and effective_language is None and tafsir_context is None and zakat_context is None and SEMANTIC_CACHE_ENABLED🤖 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 `@main.py` around lines 502 - 509, Update the semantic-cache path around the ChatResponse construction to prevent reuse of cached responses when the caller explicitly requests a supported language. Bypass the cache for explicit language requests, or include the requested language in the cache key, while preserving cached responses for requests without an explicit language.
🤖 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.
Outside diff comments:
In `@main.py`:
- Around line 448-453: Update the non-streaming /chat generation flow after
effective_language is computed in the classification block to apply
LANGUAGE_INSTRUCTIONS and pass response_language, matching the existing
streaming path. Ensure generated responses use the requested normalized language
while preserving the reported language payload.
- Around line 502-509: Update the semantic-cache path around the ChatResponse
construction to prevent reuse of cached responses when the caller explicitly
requests a supported language. Bypass the cache for explicit language requests,
or include the requested language in the cache key, while preserving cached
responses for requests without an explicit language.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 7a222172-d2ce-4991-93ad-4b07f518276e
📒 Files selected for processing (3)
README.mdmain.pyrender.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- render.yaml
- README.md
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
main.py (4)
121-128: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winBound
promptandcontextinChatRequest. These fields are plainstrs, so a single oversized request can bloat memory and drive unnecessary retrieval/Gemini work in both chat endpoints. Addmin_length/max_lengthconstraints at the Pydantic boundary.🤖 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 `@main.py` around lines 121 - 128, Update the ChatRequest model to constrain prompt and context with appropriate Pydantic min_length and max_length bounds, while preserving their existing optional/required behavior and ensuring both chat endpoints receive validated input.Source: Path instructions
127-128: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftBind memory access to an authenticated principal.
The caller controls
user_id, which is then used to read and write memory. The public/memory/{user_id}routes also allow profile retrieval and deletion without authorization; the existing TODO explicitly acknowledges this exposure. Derive the identifier from authentication, or reject mismatches before any memory operation.Also applies to: 488-490, 715-719, 989-1013
🤖 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 `@main.py` around lines 127 - 128, Update the memory request handling around the user_id field and the public /memory/{user_id} routes to require an authenticated principal, derive the effective user identifier from that principal, and reject any caller-supplied mismatch before reading, writing, retrieving, or deleting memory. Apply the same authorization check to the referenced memory operation paths and remove the exposed unauthenticated access acknowledged by the existing TODO.
127-127: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winImport
Fieldfrom Pydantic
ChatRequest.user_idusesField(...), butmain.pyonly importsBaseModel. That leavesFieldundefined and breaks module import/CI.🤖 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 `@main.py` at line 127, Update the Pydantic import in main.py to include Field alongside BaseModel so the ChatRequest.user_id declaration resolves correctly and the module imports successfully.Sources: Path instructions, Linters/SAST tools
716-720: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winKeep a strong reference to these background tasks (main.py:716-720, 729-736). Dropping the
Taskobject means the memory extraction and history summarization jobs can be collected before they finish. Store them in a set and remove them in a done callback, or use a managed background-task facility.🤖 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 `@main.py` around lines 716 - 720, Update the background task creation around _extract_and_update_memory and the corresponding history summarization task to retain each asyncio.Task in a managed set, then remove it via a done callback after completion. Ensure both jobs remain strongly referenced until they finish.Sources: Path instructions, Linters/SAST tools
🤖 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.
Outside diff comments:
In `@main.py`:
- Around line 121-128: Update the ChatRequest model to constrain prompt and
context with appropriate Pydantic min_length and max_length bounds, while
preserving their existing optional/required behavior and ensuring both chat
endpoints receive validated input.
- Around line 127-128: Update the memory request handling around the user_id
field and the public /memory/{user_id} routes to require an authenticated
principal, derive the effective user identifier from that principal, and reject
any caller-supplied mismatch before reading, writing, retrieving, or deleting
memory. Apply the same authorization check to the referenced memory operation
paths and remove the exposed unauthenticated access acknowledged by the existing
TODO.
- Line 127: Update the Pydantic import in main.py to include Field alongside
BaseModel so the ChatRequest.user_id declaration resolves correctly and the
module imports successfully.
- Around line 716-720: Update the background task creation around
_extract_and_update_memory and the corresponding history summarization task to
retain each asyncio.Task in a managed set, then remove it via a done callback
after completion. Ensure both jobs remain strongly referenced until they finish.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e204a0d7-309f-4141-814b-dffc42fb09af
📒 Files selected for processing (2)
README.mdmain.py
🚧 Files skipped from review as they are similar to previous changes (1)
- README.md
de0f175 to
b52305e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@README.md`:
- Around line 50-57: Resolve the conflict in the README API table by removing
all merge markers and preserving the intended entries for /ping and /health,
along with the existing /memory/{user_id} endpoints. Ensure the resulting table
is valid Markdown and reflects both API behaviors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
2 participants
Closes #20
Summary by CodeRabbit
GET /healthendpoint that returns JSON service status, with optional deeper readiness details via a query parameter./health, fail on HTTP errors, and retry with a per-request timeout.GET /ping(liveness) fromGET /health(health/readiness).