-
Notifications
You must be signed in to change notification settings - Fork 7
function doc fixes and callback router #456
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
Conversation
WalkthroughThis pull request updates API documentation for document and job endpoints to clarify Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
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/upload.md(1 hunks)backend/app/api/routes/doc_transformation_job.py(3 hunks)backend/app/api/routes/documents.py(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use type hints in Python code (Python 3.11+ project)
Files:
backend/app/api/routes/doc_transformation_job.pybackend/app/api/routes/documents.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/doc_transformation_job.pybackend/app/api/routes/documents.py
🧠 Learnings (1)
📚 Learning: 2025-10-08T12:05:01.317Z
Learnt from: CR
Repo: ProjectTech4DevAI/ai-platform PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-08T12:05:01.317Z
Learning: Applies to backend/app/core/doctransform/**/*.py : Place document transformation utilities under backend/app/core/doctransform/
Applied to files:
backend/app/api/routes/documents.py
🧬 Code graph analysis (2)
backend/app/api/routes/doc_transformation_job.py (2)
backend/app/models/document.py (2)
DocTransformationJobPublic(82-87)DocTransformationJobsPublic(90-92)backend/app/utils.py (2)
APIResponse(30-54)load_description(288-293)
backend/app/api/routes/documents.py (2)
backend/app/models/document.py (1)
DocTransformationJobPublic(82-87)backend/app/utils.py (1)
APIResponse(30-54)
🔇 Additional comments (6)
backend/app/api/docs/documents/info.md (1)
1-1: Include_url description matchesdoc_infoimplementationThe text accurately reflects that the signed URL is only included when
include_urlis true and omitted otherwise, consistent with thedoc_inforoute behavior.backend/app/api/docs/documents/job_list.md (1)
1-1: Clear description for multi-job status endpointThe explanation of
include_urland signed URLs for transformed documents is consistent with theget_multiple_transformation_jobsendpoint’s behavior.backend/app/api/docs/documents/upload.md (1)
5-5: Upload docs now correctly document callback behaviorThe new bullet clearly explains that providing a
callback_urlresults in a notification when the transformation job completes, matching theupload_docendpoint and callback setup.backend/app/api/docs/documents/job_info.md (1)
1-1: Single-job include_url behavior documented consistentlyThis description aligns with the
get_transformation_jobendpoint: wheninclude_urlis true and the job is successful, a signed URL is included; otherwise, it’s omitted.backend/app/api/docs/documents/list.md (1)
1-1: List endpoint docs now accurately describe include_urlThe description of how
include_urlaffects inclusion of signed URLs in the documents list matches the behavior of thelist_docsendpoint.backend/app/api/routes/doc_transformation_job.py (1)
4-4: Dynamic route descriptions wired correctly to new job docsUsing
load_description("documents/job_info.md")andload_description("documents/job_list.md")for the two GET endpoints is consistent with the existing pattern in other routers and correctly ties these routes to the new markdown docs. The imports are minimal and appropriate, and there’s no functional change to the handlers themselves.Also applies to: 12-12, 23-23, 55-55
| @@ -0,0 +1 @@ | |||
| Get the status and details of a document transformation job. If you set the ``include_url`` parameter to true, a signed URL will be included in the response, which is a clickable link to access the transformed document if the job has been successful. If you don't set it to true, the URL will not be included in the response. | |||
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.
Suggestion:
Instead of placing the information about include_url in the top-level documentation, it would be clearer to include it within the description of that specific field.
The top-level section should focus on explaining the overall API behavior, note any exceptional or abnormal behaviors, and include general guidance or notes where appropriate.
avirajsingh7
left a 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.
Approved. please go through above comment
Prajna1999
left a 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.
LGTM!
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
Documentation Updates: Clarified include_url parameter behavior (adds signed URL when true, omits when false) and detailed callback notification workflow for document transformation completion.
Route Description Refactoring: Replaced static route descriptions with dynamic load_description calls in doc_transformation_job.py to keep the consistency with rest of the document endpoints; cleaned up imports.
Callback Router Implementation: Added doctransformation_callback_router and integrated a placeholder callback endpoint (POST {$callback_url}) into the document upload flow.