docs: populate OpenAPI metadata, examples, and error responses (#18) - #67
docs: populate OpenAPI metadata, examples, and error responses (#18)#67Fury03 wants to merge 2 commits into
Conversation
/docs is the primary integration reference for the backend and frontend
teams, but the app supplied only a title, so it showed untagged,
unsummarized routes with no field descriptions and only happy-path
schemas.
- App metadata: description covering session semantics and the response
envelope, version, contact and license, and a tag list with descriptions
covering every router (chat, health, tafsir, study, stellar,
scholar-review).
- Every route now carries a tag and a summary, with docstrings explaining
the semantics that are not obvious from the schema — above all that
omitting chat_id starts a session and passing it back continues one.
- /chat documents 200, 422 and 500 with example bodies, and ChatRequest
ships two worked request examples (start a session, continue one).
- DELETE /chat/{chat_id} and /ping declare typed response models, and the
delete route documents its idempotent "not found" case, which also
returns 200.
- Every field on ChatRequest, ChatResponse and Message has a description
and, where scalar, an example.
- README links /docs and adds a copy-pasteable two-step curl example:
start a session, then continue it with the returned chat_id.
Documentation-only: no handler was touched. PingResponse deliberately
documents the array that /ping actually returns rather than a tidier
shape, so no response body changes. Verified by diffing live responses
for /ping, /chat 422, delete, /cache/stats, /confidence/policy and
/tafsir against dev — byte-identical.
tests/test_openapi_docs.py asserts the acceptance criteria against the
generated schema, so the docs cannot rot silently.
Closes Deen-Bridge#18
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThe API now exposes richer OpenAPI metadata, typed response schemas, field descriptions, examples, and session guidance. README documentation explains interactive docs and chat lifecycle usage, while dedicated tests and CI steps validate the generated schema. ChangesOpenAPI Documentation
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 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 `@main.py`:
- Around line 252-271: The ChatRequest model’s json_schema_extra examples must
be plain request payloads rather than OpenAPI Example Object wrappers; update
the examples in ChatRequest and adjust tests/test_openapi_docs.py accordingly.
If preserving summaries and descriptions, move those labeled variants to the
/chat request-body documentation and assert them there, while validating the
model schema against unwrapped payload examples.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b175aa10-a448-410d-bf26-f089d3c0d1d6
📒 Files selected for processing (4)
.github/workflows/ci.ymlREADME.mdmain.pytests/test_openapi_docs.py
JSON Schema `examples` take instances of the model itself, so the
{summary, description, value} wrappers on ChatRequest were rendering the
wrapper in the schema view instead of a usable request body.
The plain payloads now live on the model, and the labelled variants a
reader picks between — "Start a new session" and "Continue a session" —
move to the /chat operation as OpenAPI Example Objects via
Body(openapi_examples=...), which is where Swagger's example dropdown
reads them from.
Tests assert both shapes: that schema examples carry no Example Object
wrapper keys and only real model fields, and that the operation's
labelled examples exist, with the "start" one genuinely omitting chat_id
and the "continue" one genuinely sending it.
Responses remain byte-identical to dev.
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)
main.py (1)
213-221: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMake
chat_idvalidation match the session contract.ChatRequest.chat_idaccepts any string, and an unknown ID silently creates a new chat under that caller-provided value. That makes typos look like valid continuations and leaves the API contract misleading. Constrainchat_idto the expected session-id format and return a documented 4xx for unknown IDs, or update the schema to say arbitrary IDs are accepted and will start a new session.🤖 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 213 - 221, Update ChatRequest.chat_id and the session lookup flow to enforce the documented session contract: validate the expected session-ID format and return a documented 4xx when the ID is unknown, rather than silently creating a session under it. Ensure the description and examples remain consistent with this behavior.Source: Path instructions
🤖 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`:
- Around line 540-544: Update the safety-disabled branch in chat so the
synchronous generate()/send_message() model call does not execute on the event
loop; offload it to an appropriate worker thread or use an async client while
preserving the existing response behavior.
---
Outside diff comments:
In `@main.py`:
- Around line 213-221: Update ChatRequest.chat_id and the session lookup flow to
enforce the documented session contract: validate the expected session-ID format
and return a documented 4xx when the ID is unknown, rather than silently
creating a session under it. Ensure the description and examples remain
consistent with this behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e895f2d5-b156-4c8e-bdbe-2f4615cecb1f
📒 Files selected for processing (2)
main.pytests/test_openapi_docs.py
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/test_openapi_docs.py
| async def chat( | ||
| http_request: Request, | ||
| fastapi_response: Response, | ||
| request: ChatRequest = Body(..., openapi_examples=CHAT_REQUEST_EXAMPLES), | ||
| ): |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline main.py --items all --match 'chat|generate'
rg -n -C 4 'SAFETY_PIPELINE_ENABLED|def generate|send_message\(|run_async\(' main.py
fd -HI -t f '^(pyproject\.toml|requirements.*\.txt|setup\.py|Pipfile)$' . -0 |
xargs -0 -r grep -nE 'google-generativeai|google-genai|fastapi'Repository: Deen-Bridge/dnb-ai
Length of output: 1804
Offload the synchronous model call on the safety-disabled path. When SAFETY_PIPELINE_ENABLED is false, generate() is called directly and reaches send_message() on the event loop, which blocks this worker during model round-trips. Use an async client or offload the call to keep concurrent requests moving.
🧰 Tools
🪛 Ruff (0.15.21)
[warning] 543-543: Do not perform function call Body in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
🤖 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 540 - 544, Update the safety-disabled branch in chat so
the synchronous generate()/send_message() model call does not execute on the
event loop; offload it to an appropriate worker thread or use an async client
while preserving the existing response behavior.
Source: Path instructions
|
Thanks — the first finding is fixed in d9c1466; the second is real but I've deliberately left it out of this PR. Schema vs. operation examples — fixed. You were right: JSON Schema Tests assert both shapes — that schema examples carry no Example Object keys and only real model fields, and that the labelled "start" example genuinely omits Blocking I'm not fixing it here because #18 states this is a "pure documentation-metadata change — behavior must not change; don't refactor handlers while in there," and the PR's central claim is that responses are byte-identical to @zeemscript happy to open it as its own PR — the fix is to offload the generator call inside |
|
@coderabbitai review |
✅ Action performedReview finished.
|
1 participant
Closes #18
What
/docsis the primary integration reference for the backend and frontend teams, but the app gave FastAPI only a title — so it rendered untagged, unsummarized routes with no field descriptions and only happy-path schemas.App metadata — description covering session semantics and the response envelope, version, contact, license, and a tag list with descriptions covering every router (
chat,health,tafsir,study,stellar,scholar-review).Every route now carries a tag and a summary, plus a docstring explaining what the schema can't — above all that omitting
chat_idstarts a session and passing it back continues one./chatdocuments200,422and500with example bodies.ChatRequestships two worked request examples that Swagger's dropdown picks up: "Start a new session" (nochat_id) and "Continue a session".DELETE /chat/{chat_id}and/pingdeclare typed response models. The delete route also documents its idempotent case — deleting an unknown session returns200with a "not found" message, not a 404 — and its path parameter is described.Every field on
ChatRequest,ChatResponseandMessagehas a description and, where scalar, an example.README links
/docsand adds a copy-pasteable two-step curl example: start a session, then continue it with the returnedchat_id, then delete it.No runtime behavior change
This is documentation metadata only — no handler was touched.
One judgement call worth flagging:
/pingreturns a Python set literal, which FastAPI serializes to a one-element JSON array, not an object. I gave it aRootModel[List[str]]that documents what it actually returns rather than a tidier{"status": "ok"}, because changing the body would break any client asserting on it and the issue explicitly requires identical responses. If you'd like/pingreshaped, that's a behavior change worth its own PR and I'm happy to open one.I verified this rather than assuming it: I captured live responses for
/ping,/chat(422),DELETE /chat/{id},/cache/stats,/confidence/policy,/tafsir/sourcesand/tafsir(400) ondev, then again on this branch — byte-identical.Tests
tests/test_openapi_docs.py(27 tests) asserts the acceptance criteria against the generated schema, so the docs can't rot silently: every route is tagged and summarized, every declared tag is used and every used tag is declared,/chatdocuments all three statuses each with an example, delete and ping expose typed schemas, and every model field has a description (and scalars an example). It also checks the "start a session" example genuinely omitschat_id— an example that contradicts its own caption would be worse than none.main.pyconfigures the Gemini SDK at import, so the suite stubs it before importing — no API key, no model calls. CI extends flake8/compile and adds the new pytest step. Full suite: 489 passed, 1 skipped.Summary by CodeRabbit
Documentation
/docs) and OpenAPI (/openapi.json) links.chat_idusage and idempotent delete behavior.Improvements
Tests
Chores