-
Notifications
You must be signed in to change notification settings - Fork 7
Enhancing swagger and redocs #484
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?
Conversation
|
Warning Rate limit exceeded@nishika26 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 7 minutes and 45 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughStandardizes OpenAPI/route tag metadata, adds OpenAPI schema customization (tag groups and deprecated badges), rewrites and adds many API documentation pages, updates onboarding docs to support multi-credential examples, introduces API_VERSION config, and wires custom OpenAPI generation into the FastAPI app. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
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: 4
🧹 Nitpick comments (3)
backend/app/api/docs/evaluation/create_evaluation.md (1)
26-57: Consider clarifying that example IDs are placeholders.The examples are helpful, but the hardcoded IDs (e.g.,
dataset_id: 123,"vs_abc123","asst_xyz789") might confuse users. Consider adding a note that these are placeholder values that should be replaced with actual IDs from their system.Apply this diff to add clarification:
**Example: Using Direct Configuration** +*Note: Replace the IDs below with actual values from your system.* + ```jsonbackend/app/api/docs/evaluation/list_datasets.md (1)
1-3: Consider keeping a minimal “Parameters/Response shape” stub for Markdown readers.
The new text is clearer/shorter, but if these.mdfiles are a primary reference, dropping all explicit params/response structure may reduce usability.backend/app/api/docs/evaluation/list_evaluations.md (1)
1-3: Consider retaining a minimal “Returns includes …” bullet list for scan-ability.
The single-sentence summary is fine, but a tiny bullet list (IDs/status/progress/results) tends to be easier to skim in long docs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
backend/app/api/docs/collections/create.md(1 hunks)backend/app/api/docs/collections/list.md(1 hunks)backend/app/api/docs/evaluation/create_evaluation.md(1 hunks)backend/app/api/docs/evaluation/delete_dataset.md(0 hunks)backend/app/api/docs/evaluation/get_dataset.md(1 hunks)backend/app/api/docs/evaluation/get_evaluation.md(2 hunks)backend/app/api/docs/evaluation/list_datasets.md(1 hunks)backend/app/api/docs/evaluation/list_evaluations.md(1 hunks)backend/app/api/docs/evaluation/upload_dataset.md(1 hunks)backend/app/api/docs/llm/llm_call.md(1 hunks)backend/app/api/docs/onboarding/onboarding.md(1 hunks)backend/app/api/docs/openapi_config.py(1 hunks)backend/app/api/routes/collection_job.py(1 hunks)backend/app/api/routes/collections.py(1 hunks)backend/app/api/routes/credentials.py(1 hunks)backend/app/api/routes/cron.py(1 hunks)backend/app/api/routes/doc_transformation_job.py(1 hunks)backend/app/api/routes/documents.py(1 hunks)backend/app/api/routes/evaluation.py(1 hunks)backend/app/api/routes/fine_tuning.py(1 hunks)backend/app/api/routes/login.py(1 hunks)backend/app/api/routes/model_evaluation.py(1 hunks)backend/app/api/routes/onboarding.py(1 hunks)backend/app/api/routes/organization.py(1 hunks)backend/app/api/routes/project.py(1 hunks)backend/app/api/routes/responses.py(1 hunks)backend/app/api/routes/threads.py(1 hunks)backend/app/api/routes/users.py(1 hunks)backend/app/main.py(2 hunks)backend/app/models/collection.py(2 hunks)
💤 Files with no reviewable changes (1)
- backend/app/api/docs/evaluation/delete_dataset.md
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use type hints in Python code (Python 3.11+ project)
Files:
backend/app/api/routes/documents.pybackend/app/api/routes/credentials.pybackend/app/models/collection.pybackend/app/api/routes/model_evaluation.pybackend/app/api/routes/users.pybackend/app/api/routes/fine_tuning.pybackend/app/api/routes/doc_transformation_job.pybackend/app/api/routes/organization.pybackend/app/api/routes/evaluation.pybackend/app/api/docs/openapi_config.pybackend/app/api/routes/cron.pybackend/app/api/routes/onboarding.pybackend/app/api/routes/login.pybackend/app/api/routes/threads.pybackend/app/api/routes/project.pybackend/app/api/routes/collections.pybackend/app/main.pybackend/app/api/routes/responses.pybackend/app/api/routes/collection_job.py
backend/app/api/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Expose FastAPI REST endpoints under backend/app/api/ organized by domain
Files:
backend/app/api/routes/documents.pybackend/app/api/routes/credentials.pybackend/app/api/routes/model_evaluation.pybackend/app/api/routes/users.pybackend/app/api/routes/fine_tuning.pybackend/app/api/routes/doc_transformation_job.pybackend/app/api/routes/organization.pybackend/app/api/routes/evaluation.pybackend/app/api/docs/openapi_config.pybackend/app/api/routes/cron.pybackend/app/api/routes/onboarding.pybackend/app/api/routes/login.pybackend/app/api/routes/threads.pybackend/app/api/routes/project.pybackend/app/api/routes/collections.pybackend/app/api/routes/responses.pybackend/app/api/routes/collection_job.py
backend/app/models/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Define SQLModel entities (database tables and domain objects) in backend/app/models/
Files:
backend/app/models/collection.py
🧬 Code graph analysis (1)
backend/app/main.py (1)
backend/app/api/docs/openapi_config.py (1)
customize_openapi_schema(115-135)
🪛 markdownlint-cli2 (0.18.1)
backend/app/api/docs/onboarding/onboarding.md
27-27: Headings must start at the beginning of the line
(MD023, heading-start-left)
28-28: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
29-29: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: checks (3.11.7, 6)
🔇 Additional comments (26)
backend/app/api/docs/collections/list.md (1)
6-6: Doc deprecation label standardization looks good.backend/app/models/collection.py (3)
62-70: Deprecation label update in field description is fine.
72-79: Deprecation label update in field description is fine.
80-88: Deprecation label update in field description is fine.backend/app/api/docs/collections/create.md (1)
10-15: Doc deprecation label standardization looks good.backend/app/api/routes/model_evaluation.py (1)
33-33: No action needed—no code or configuration depends on the old tag name.The search found only database table definitions and migration references to "model_evaluation", which are unrelated to the OpenAPI tags parameter. The tag change from the old value to "Model Evaluation" is safe and has no breaking dependencies.
backend/app/api/routes/doc_transformation_job.py (1)
18-18: No action needed. The tag is already consistent with the OpenAPI configuration and all other document-related routers use the same"Documents"tag.backend/app/api/routes/organization.py (1)
23-23: Remove this comment—the tag is already properly configured in OpenAPI schema.The
tags=["Organizations"]at line 23 matches the tag definition inbackend/app/api/docs/openapi_config.py(line 55) and is correctly included in the "Administration" tag group. No broken references exist.backend/app/api/routes/cron.py (1)
13-13: The current tag "Cron" in the router definition does not conflict with any other FastAPI router tags. All references to "cron" found in the codebase are Celery queue names, not router tags, and therefore are unrelated to this change. No tag-based tooling or tests would be impacted by the current router tag configuration.backend/app/api/routes/fine_tuning.py (1)
44-44: No action required—the OpenAPI tag is already correctly configured.The tag
"Fine Tuning"at line 44 matches the centralized OpenAPI metadata inbackend/app/api/docs/openapi_config.py(line 35) and is included in the tag_groups (line 93). All naming conventions align with other routers (e.g., "Model Evaluation", "OpenAI Conversations"). There is no mismatch between the router tag and OpenAPI configuration.backend/app/api/docs/llm/llm_call.md (1)
6-6: LGTM! Improved section heading.The change from "Request Fields" to "Key Parameters" improves clarity and aligns with documentation best practices.
backend/app/api/routes/threads.py (1)
21-21: LGTM! Consistent tag capitalization.The router tag capitalization aligns with the PR-wide standardization of OpenAPI metadata. This change only affects documentation grouping, not routing behavior.
backend/app/api/routes/responses.py (1)
26-26: LGTM! Consistent tag capitalization.The router tag capitalization maintains consistency with other route modules updated in this PR and only affects OpenAPI documentation metadata.
backend/app/api/routes/collection_job.py (1)
25-25: LGTM! Consistent tag capitalization.The capitalized tag maintains consistency with the PR-wide OpenAPI metadata standardization.
backend/app/api/routes/project.py (1)
20-20: LGTM! Consistent tag capitalization.The router tag change aligns with the standardized capitalization pattern across all API routes in this PR.
backend/app/api/routes/onboarding.py (1)
11-11: LGTM! Consistent tag capitalization.The capitalized tag maintains consistency with the OpenAPI metadata standardization across the codebase.
backend/app/api/docs/evaluation/get_dataset.md (1)
1-3: Documentation simplified effectively.The consolidated documentation provides a clear summary of what the endpoint returns. This aligns with the PR's approach of simplifying API documentation to be more concise and feature-focused rather than field-by-field detailed.
backend/app/api/docs/evaluation/create_evaluation.md (2)
1-24: Clear and well-structured documentation.The feature-focused overview and workflow description provide good clarity for users. The referenced endpoint
GET /evaluations/{evaluation_id}exists and is correctly documented.
59-59: No changes needed—the documentation is accurate.backend/app/api/routes/collections.py (1)
38-38: Tag capitalization change looks safe.
This only affects OpenAPI grouping/labeling (tags=["Collections"]) and shouldn’t impact routing/runtime behavior.backend/app/api/routes/users.py (1)
30-31: Tag capitalization change looks safe.
tags=["Users"]is doc-only metadata and shouldn’t alter routing behavior.backend/app/api/routes/credentials.py (1)
20-20: Tag capitalization change looks safe.
tags=["Credentials"]should only affect generated docs grouping.backend/app/api/routes/documents.py (1)
39-39: Tag capitalization change looks safe.
tags=["Documents"]is OpenAPI metadata; routes/prefixes are unchanged.backend/app/api/routes/login.py (1)
21-21: Tag capitalization change looks safe.
This should only affect OpenAPI tag display/grouping.backend/app/api/routes/evaluation.py (1)
48-48: Tag capitalization looks consistent with centralized OpenAPI tag metadata (but ensure it matches everywhere).
This aligns withtags_metadata/x-tagGroupsexpecting"Evaluation"; just make sure other evaluation routers (and any clients/tests that rely on the old"evaluation"tag) are updated consistently.backend/app/api/docs/evaluation/get_evaluation.md (1)
1-7: Avoid hard-coded “status checks every 60 seconds” unless that cadence is guaranteed.
Consider rephrasing to “periodically” or documenting it as “typically ~60s” only if backed by code/config that enforces it.
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
♻️ Duplicate comments (1)
backend/app/api/docs/onboarding/onboarding.md (1)
27-29: Fix markdown heading/list indentation.The "### Supported Providers" heading and bullet list are indented, causing MD023/MD007 lint warnings. This issue was already flagged in a previous review.
🧹 Nitpick comments (9)
.env.example (1)
19-19: Consider reordering to follow linter convention.The static analysis linter suggests placing
API_VERSIONbeforePROJECT_NAME(line 17) for consistency with the file's ordering convention. This is purely organizational and doesn't affect functionality.backend/app/api/docs/model_evaluation/get_top_model.md (1)
1-3: Doc is too minimal for Swagger/Redoc usability (add method/path/params/response + examples).
Consider adding: HTTP method + path (GET /model_evaluation/{document_id}/top_model), parameter details (document_idUUID), what “top-performing” means (tie-breaker if MCC ties), and a short sample response showingprediction_data_file_urlpresent/absent.backend/app/api/routes/model_evaluation.py (1)
32-32: Router tag rename to “Model Evaluation” is fine (docs-only change).
Just ensure other routers follow the same casing/spaces convention so tag grouping stays consistent in Redoc.backend/app/api/routes/openai_conversation.py (2)
28-28: Type mismatch between load_description signature and usage.The
load_descriptionfunction signature expects aPathobject, but string literals are being passed. While this may work at runtime, it violates the type hints.As per coding guidelines, proper type hints should be used in this Python 3.11+ project.
Consider one of these solutions:
Option 1: Wrap strings in Path constructor (recommended if load_description should only accept Path)
- description=load_description("openai_conversation/get.md"), + description=load_description(Path("openai_conversation/get.md")),Apply similar changes to lines 52, 77, and 104. You'll also need to add
from pathlib import Pathto the imports.Option 2: Update load_description signature to accept both types
In
backend/app/utils.py:def load_description(filename: str | Path) -> str: filename = Path(filename) if isinstance(filename, str) else filename if not filename.exists(): this = Path(__file__) filename = this.parent.joinpath("api", "docs", filename) return filename.read_text()Also applies to: 52-52, 77-77, 104-104
133-137: Consider adding a summary parameter for consistency.All other routes in this file include both
summaryanddescriptionparameters, but the delete route only hasdescription. Adding asummarywould improve consistency and provide a concise label in the API documentation.Apply this diff:
@router.delete( "/{conversation_id}", response_model=APIResponse, + summary="Delete a conversation by its ID", description=load_description("openai_conversation/delete.md"), )backend/app/api/routes/responses.py (2)
42-48: Consider renaming_current_usertocurrent_user.The underscore prefix typically indicates an unused variable in Python, but
_current_useris actively used in lines 45-48 to extractproject_idandorganization_id. Renaming tocurrent_userwould better reflect its usage and improve code clarity.Apply this diff to rename the variable:
- _current_user: UserProjectOrg = Depends(get_current_user_org_project), + current_user: UserProjectOrg = Depends(get_current_user_org_project), ): """Asynchronous endpoint that processes requests using Celery.""" project_id, organization_id = ( - _current_user.project_id, - _current_user.organization_id, + current_user.project_id, + current_user.organization_id, )
76-82: Consider renaming_current_usertocurrent_user.Similar to the async endpoint,
_current_useris actively used in lines 79-82. The underscore prefix is misleading since the variable is not unused.Apply this diff to rename the variable:
- _current_user: UserProjectOrg = Depends(get_current_user_org_project), + current_user: UserProjectOrg = Depends(get_current_user_org_project), ): """Synchronous endpoint for benchmarking OpenAI responses API with Langfuse tracing.""" project_id, organization_id = ( - _current_user.project_id, - _current_user.organization_id, + current_user.project_id, + current_user.organization_id, )backend/app/api/routes/project.py (1)
2-2: Replace deprecatedtyping.Listwith built-inlist.In Python 3.9+, the built-in
listcan be used directly for type hints. Since this is a Python 3.11+ project (per coding guidelines), thetyping.Listimport is unnecessary.Apply this diff:
-from typing import ListUpdate the type hints:
- response_model=APIResponse[List[ProjectPublic]], + response_model=APIResponse[list[ProjectPublic]],backend/app/api/routes/organization.py (1)
2-2: Replace deprecatedtyping.Listwith built-inlist.Similar to the project.py file, this uses the deprecated
typing.List. Use the built-inlisttype instead for Python 3.11+.Apply this diff:
-from typing import ListUpdate the type hint:
- response_model=APIResponse[List[OrganizationPublic]], + response_model=APIResponse[list[OrganizationPublic]],
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (41)
.env.example(1 hunks)backend/app/api/docs/api_keys/create.md(1 hunks)backend/app/api/docs/api_keys/delete.md(1 hunks)backend/app/api/docs/api_keys/list.md(1 hunks)backend/app/api/docs/credentials/create.md(1 hunks)backend/app/api/docs/credentials/delete_all.md(1 hunks)backend/app/api/docs/credentials/delete_provider.md(1 hunks)backend/app/api/docs/credentials/get_provider.md(1 hunks)backend/app/api/docs/credentials/list.md(1 hunks)backend/app/api/docs/credentials/update.md(1 hunks)backend/app/api/docs/evaluation/upload_dataset.md(1 hunks)backend/app/api/docs/model_evaluation/evaluate.md(1 hunks)backend/app/api/docs/model_evaluation/get_top_model.md(1 hunks)backend/app/api/docs/model_evaluation/list_by_document.md(1 hunks)backend/app/api/docs/onboarding/onboarding.md(1 hunks)backend/app/api/docs/openai_conversation/delete.md(1 hunks)backend/app/api/docs/openai_conversation/get.md(1 hunks)backend/app/api/docs/openai_conversation/get_by_ancestor_id.md(1 hunks)backend/app/api/docs/openai_conversation/get_by_response_id.md(1 hunks)backend/app/api/docs/openai_conversation/list.md(1 hunks)backend/app/api/docs/organization/create.md(1 hunks)backend/app/api/docs/organization/delete.md(1 hunks)backend/app/api/docs/organization/get.md(1 hunks)backend/app/api/docs/organization/list.md(1 hunks)backend/app/api/docs/organization/update.md(1 hunks)backend/app/api/docs/projects/create.md(1 hunks)backend/app/api/docs/projects/delete.md(1 hunks)backend/app/api/docs/projects/get.md(1 hunks)backend/app/api/docs/projects/list.md(1 hunks)backend/app/api/docs/projects/update.md(1 hunks)backend/app/api/docs/responses/create_async.md(1 hunks)backend/app/api/docs/responses/create_sync.md(1 hunks)backend/app/api/routes/api_keys.py(4 hunks)backend/app/api/routes/credentials.py(6 hunks)backend/app/api/routes/model_evaluation.py(4 hunks)backend/app/api/routes/openai_conversation.py(6 hunks)backend/app/api/routes/organization.py(6 hunks)backend/app/api/routes/project.py(6 hunks)backend/app/api/routes/responses.py(2 hunks)backend/app/core/config.py(1 hunks)backend/app/main.py(2 hunks)
✅ Files skipped from review due to trivial changes (20)
- backend/app/api/docs/projects/delete.md
- backend/app/api/docs/model_evaluation/list_by_document.md
- backend/app/api/docs/responses/create_async.md
- backend/app/api/docs/organization/delete.md
- backend/app/api/docs/credentials/get_provider.md
- backend/app/api/docs/organization/create.md
- backend/app/api/docs/projects/create.md
- backend/app/api/docs/organization/update.md
- backend/app/api/docs/organization/list.md
- backend/app/api/docs/credentials/delete_all.md
- backend/app/api/docs/openai_conversation/get.md
- backend/app/api/docs/credentials/delete_provider.md
- backend/app/api/docs/credentials/update.md
- backend/app/api/docs/responses/create_sync.md
- backend/app/api/docs/openai_conversation/get_by_response_id.md
- backend/app/api/docs/model_evaluation/evaluate.md
- backend/app/api/docs/credentials/list.md
- backend/app/api/docs/openai_conversation/list.md
- backend/app/api/docs/projects/update.md
- backend/app/api/docs/api_keys/create.md
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/app/main.py
🧰 Additional context used
📓 Path-based instructions (4)
.env.example
📄 CodeRabbit inference engine (CLAUDE.md)
Provide .env.example as the template for .env
Files:
.env.example
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use type hints in Python code (Python 3.11+ project)
Files:
backend/app/api/routes/openai_conversation.pybackend/app/api/routes/responses.pybackend/app/api/routes/model_evaluation.pybackend/app/core/config.pybackend/app/api/routes/credentials.pybackend/app/api/routes/api_keys.pybackend/app/api/routes/organization.pybackend/app/api/routes/project.py
backend/app/api/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Expose FastAPI REST endpoints under backend/app/api/ organized by domain
Files:
backend/app/api/routes/openai_conversation.pybackend/app/api/routes/responses.pybackend/app/api/routes/model_evaluation.pybackend/app/api/routes/credentials.pybackend/app/api/routes/api_keys.pybackend/app/api/routes/organization.pybackend/app/api/routes/project.py
backend/app/core/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Place core functionality (config, DB session, security, exceptions, middleware) in backend/app/core/
Files:
backend/app/core/config.py
🧬 Code graph analysis (6)
backend/app/api/routes/openai_conversation.py (1)
backend/app/utils.py (2)
APIResponse(30-54)load_description(288-293)
backend/app/api/routes/responses.py (1)
backend/app/utils.py (1)
load_description(288-293)
backend/app/api/routes/model_evaluation.py (1)
backend/app/utils.py (3)
get_openai_client(176-206)APIResponse(30-54)load_description(288-293)
backend/app/api/routes/credentials.py (2)
backend/app/utils.py (2)
APIResponse(30-54)load_description(288-293)backend/app/models/credentials.py (1)
CredsPublic(100-107)
backend/app/api/routes/api_keys.py (1)
backend/app/utils.py (2)
APIResponse(30-54)load_description(288-293)
backend/app/api/routes/project.py (3)
backend/app/utils.py (2)
APIResponse(30-54)load_description(288-293)backend/app/api/deps.py (1)
get_current_active_superuser(131-136)backend/app/models/project.py (1)
ProjectPublic(70-74)
🪛 dotenv-linter (4.0.0)
.env.example
[warning] 19-19: [UnorderedKey] The API_VERSION key should go before the PROJECT_NAME key
(UnorderedKey)
🪛 Ruff (0.14.8)
backend/app/api/routes/organization.py
2-2: typing.List is deprecated, use list instead
(UP035)
backend/app/api/routes/project.py
2-2: typing.List is deprecated, use list instead
(UP035)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: checks (3.11.7, 6)
🔇 Additional comments (22)
backend/app/core/config.py (1)
45-45: LGTM! API version configuration added correctly.The new
API_VERSIONfield follows the existing configuration pattern with proper type hints and a reasonable default value that aligns with the.env.exampletemplate.backend/app/api/docs/projects/get.md (1)
1-3: LGTM! Clear and concise endpoint documentation.The documentation provides a clear overview of the endpoint's purpose and return values.
backend/app/api/routes/model_evaluation.py (1)
26-26: No action needed. Theload_descriptionfunction is already properly designed to handle the string arguments:
- The function uses
@functools.singledispatchwith a registered overload at line 297 that convertsstr→Pathbefore calling the main function.- The main function at line 288 includes fallback logic (lines 289-291) that resolves relative paths to the docs directory if the file doesn't exist at the given path.
- All three referenced markdown files exist in
backend/app/api/docs/model_evaluation/.Type safety is ensured and there is no risk of import-time crashes.
backend/app/api/routes/api_keys.py (3)
7-7: LGTM!The
load_descriptionimport is correctly added and enables dynamic loading of endpoint descriptions from markdown files.
13-18: Good enhancement of OpenAPI metadata.The description now loads from a markdown file, improving documentation maintainability. The path correctly resolves to
backend/app/api/docs/api_keys/create.md.
41-45: LGTM!The description metadata for list and delete endpoints follows the same pattern as the create endpoint, ensuring consistency across all API key routes.
Also applies to: 59-63
backend/app/api/docs/api_keys/delete.md (1)
1-3: LGTM!The documentation is clear and concise, appropriately highlighting the permanent nature of deletion and its immediate impact on API requests.
backend/app/api/docs/openai_conversation/get_by_ancestor_id.md (1)
1-3: LGTM!The documentation is clear, concise, and follows a consistent format. The description accurately explains the endpoint's purpose for conversation chain lookup.
backend/app/api/docs/openai_conversation/delete.md (1)
1-3: LGTM!The documentation clearly explains the soft delete behavior, which is important for users to understand that the conversation remains in the database but is hidden from listings.
backend/app/api/routes/openai_conversation.py (1)
19-19: LGTM!The import addition is appropriate for loading dynamic descriptions from markdown files.
backend/app/api/routes/responses.py (3)
22-27: LGTM!The import of
load_descriptionis correctly structured and used consistently in the route decorators below.
31-31: LGTM!Tag capitalization aligns with the PR's goal to standardize OpenAPI documentation across all routers.
34-38: No action needed. The documentation filebackend/app/api/docs/responses/create_async.mdexists and is properly accessible via theload_description()function, which correctly handles string inputs through its registered overload.backend/app/api/docs/credentials/create.md (1)
1-3: LGTM!The documentation clearly explains the endpoint's purpose, security measures, and constraints.
backend/app/api/routes/credentials.py (2)
17-20: LGTM!The import of
load_descriptionand tag capitalization align with the documentation enhancement objectives across the codebase.
26-26: LGTM!The migration to markdown-based descriptions via
load_descriptionimproves documentation maintainability and consistency across the API.Also applies to: 55-55, 76-76, 100-100, 132-132, 156-156
backend/app/api/routes/project.py (2)
17-17: LGTM!The addition of
load_descriptionand tag capitalization are consistent with the documentation enhancements across the project.Also applies to: 20-20
28-28: LGTM!The migration to markdown-based descriptions improves documentation consistency across endpoints.
Also applies to: 49-49, 60-60, 78-78, 103-103
backend/app/api/docs/projects/list.md (1)
1-3: LGTM!The documentation clearly describes the endpoint's behavior and pagination support.
backend/app/api/docs/organization/get.md (1)
1-3: LGTM!The documentation is clear and concise for the organization retrieval endpoint.
backend/app/api/routes/organization.py (2)
19-19: LGTM!The import and tag capitalization changes align with the documentation enhancement pattern across the project.
Also applies to: 22-22
30-30: LGTM!The markdown-based descriptions improve documentation consistency and maintainability.
Also applies to: 47-47, 58-58, 76-76, 106-106
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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
backend/app/api/docs/documents/delete.md (1)
1-8: Fix typos/plurals + consider re-adding “soft delete” to avoid ambiguity vs “permanent delete”
- Lines 6-8: “Noteably” → “Notably”; “Vector Store's and Assistant's” → “Vector Stores and Assistants”.
- Line 1-3: since you also document a “permanent delete”, consider explicitly calling this a “soft delete” (or “logical delete”) to prevent clients from confusing the two.
backend/app/api/docs/documents/permanent_delete.md (1)
4-8: Fix typos and incorrect plurals
- Line 6: "Noteably" → "Notably"
- Lines 7-8: "Vector Store's and Assistant's" → "Vector Stores and Assistants"
The documented behavior—that collections will be deleted when a document is permanently deleted—is confirmed by the implementation (
c_crud.delete(document, remote)is called inpermanent_delete_doc). However, note that the identical text appears in bothdelete.mdandpermanent_delete.mddespite describing different deletion modes; consider clarifying when collection deletion occurs for each operation.
♻️ Duplicate comments (1)
backend/app/api/docs/onboarding/onboarding.md (1)
27-29: Markdown indentation issues remain unresolved.The "Supported Providers" heading and bullet list still have leading indentation, triggering MD023/MD007 linting warnings. A previous review flagged this issue; please apply the suggested fix.
- ### Supported Providers - - **LLM:** openai - - **Observability:** langfuse +### Supported Providers + +- **LLM:** openai +- **Observability:** langfuse
🧹 Nitpick comments (3)
backend/app/api/docs/collections/create.md (1)
4-11: Verify the assistant-creation condition matches the actual request schemaLines 9-11: Docs state an Assistant is created only if both
"model"and"instruction"are provided. Please verify these are the exact field names and requirement logic enforced by the API (and not e.g."instructions"/ additional required fields), otherwise clients will implement the wrong branching.backend/app/api/docs/documents/list.md (1)
1-3: Prefer single-backtick inline code formattingLine 3: consider using
include_url(single backticks) for inline code unless there’s a renderer-specific reason for double backticks.backend/app/api/docs/documents/job_list.md (1)
1-3: Align inline code style with other docs (include_url)Line 3: prefer
include_url(single backticks) for inline code for consistency/readability (and keep wording aligned with documents/list.md).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
backend/app/api/docs/api_keys/list.md(1 hunks)backend/app/api/docs/collections/create.md(1 hunks)backend/app/api/docs/collections/delete.md(1 hunks)backend/app/api/docs/collections/info.md(1 hunks)backend/app/api/docs/collections/list.md(1 hunks)backend/app/api/docs/config/create.md(1 hunks)backend/app/api/docs/credentials/create.md(1 hunks)backend/app/api/docs/credentials/delete_all.md(1 hunks)backend/app/api/docs/documents/delete.md(1 hunks)backend/app/api/docs/documents/info.md(1 hunks)backend/app/api/docs/documents/job_info.md(1 hunks)backend/app/api/docs/documents/job_list.md(1 hunks)backend/app/api/docs/documents/list.md(1 hunks)backend/app/api/docs/documents/permanent_delete.md(1 hunks)backend/app/api/docs/documents/upload.md(1 hunks)backend/app/api/docs/evaluation/create_evaluation.md(1 hunks)backend/app/api/docs/evaluation/delete_dataset.md(1 hunks)backend/app/api/docs/evaluation/upload_dataset.md(1 hunks)backend/app/api/docs/fine_tuning/retrieve.md(1 hunks)backend/app/api/docs/onboarding/onboarding.md(1 hunks)backend/app/api/docs/openai_conversation/delete.md(1 hunks)backend/app/api/docs/organization/update.md(1 hunks)backend/app/api/docs/projects/create.md(1 hunks)backend/app/api/docs/projects/get.md(1 hunks)backend/app/api/docs/projects/update.md(1 hunks)backend/app/api/docs/responses/create_async.md(1 hunks)
✅ Files skipped from review due to trivial changes (8)
- backend/app/api/docs/documents/job_info.md
- backend/app/api/docs/responses/create_async.md
- backend/app/api/docs/collections/info.md
- backend/app/api/docs/documents/info.md
- backend/app/api/docs/config/create.md
- backend/app/api/docs/projects/get.md
- backend/app/api/docs/fine_tuning/retrieve.md
- backend/app/api/docs/collections/list.md
🚧 Files skipped from review as they are similar to previous changes (6)
- backend/app/api/docs/projects/create.md
- backend/app/api/docs/projects/update.md
- backend/app/api/docs/organization/update.md
- backend/app/api/docs/openai_conversation/delete.md
- backend/app/api/docs/credentials/delete_all.md
- backend/app/api/docs/api_keys/list.md
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
backend/app/api/docs/evaluation/create_evaluation.md
27-27: Headings must start at the beginning of the line
(MD023, heading-start-left)
28-28: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
29-29: Unordered list indentation
Expected: 2; Actual: 3
(MD007, ul-indent)
31-31: Headings must start at the beginning of the line
(MD023, heading-start-left)
32-32: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: checks (3.11.7, 6)
🔇 Additional comments (5)
backend/app/api/docs/credentials/create.md (1)
1-3: Clear and concise credential documentation.The documentation appropriately documents the credential creation flow, encryption, and the constraint that only one credential per provider is allowed per organization-project combination.
backend/app/api/docs/onboarding/onboarding.md (1)
21-47: Parameter documentation and examples are clear and well-structured.The update from
credentialtocredentials(list) aligns with the multi-credential support mentioned in the PR summary. The example payload with OpenAI and Langfuse credentials effectively demonstrates the expected format. Once the markdown indentation issue is resolved, this section will be complete.backend/app/api/docs/evaluation/delete_dataset.md (1)
1-3: Verify that documentation simplification to a single sentence is intentional and complete.The file has been reduced to a title and one descriptive sentence, with detailed response/error documentation removed. This appears to be a significant simplification from typical API endpoint documentation.
Clarify:
- Is this minimal documentation style intentional and consistent across other endpoint docs in this PR?
- Are response schemas, error codes, and parameters documented elsewhere (e.g., via OpenAPI schema auto-generation from route handlers or Pydantic models)?
- Does this align with the PR goal to "enhance" swagger and redocs?
If response/error documentation is not generated automatically, consider restoring structured sections for responses, error codes, and parameters to ensure completeness.
backend/app/api/docs/evaluation/create_evaluation.md (2)
13-34: Verify the “Direct Configuration” JSON example matches the actual request schema.This example includes
tools,file_search, andinclude; please double-check these fields are accepted by the create-evaluation endpoint’s Pydantic model (and aren’t OpenAI-only fields that your API doesn’t accept directly).
35-46: Clarify data source + confirm{}is valid forconfig.If
assistant_idpulls config from your DB (vs an external OpenAI fetch), consider tightening wording to “from the database”. Also please confirm validation allowsconfig: {}(and that it’s truly optional whenassistant_idis provided).
Summary
Target issue is #478
Checklist
Before submitting a pull request, please ensure that you mark these task.
fastapi run --reload app/main.pyordocker compose upin the repository root and test.Notes
Summary by CodeRabbit
New Features
Documentation
Chores
Version
✏️ Tip: You can customize this high-level summary in your review settings.