Feature: Add in-memory report storage for reports #14
Conversation
WalkthroughIntroduces an in-memory report store with status and data tracking functions in storage.py. Modifies the GET /reports/{report_id}/data endpoint to return a simplified 202 response containing only a "detail" field when processing. Updates tests to use polling with anyio.to_thread.run_sync instead of fixed sleeps and asyncio orchestration. Changes
Sequence DiagramsequenceDiagram
participant Test
participant API as POST /api/v1/report/generate
participant Store as REPORT_STORE
participant GetData as GET /reports/{report_id}/data
participant Status as GET /reports/{report_id}/status
Test->>API: Submit report generation request
API->>Store: set_report_status(report_id, "processing")
API-->>Test: 200 OK (async)
rect rgb(200, 220, 255)
Note over Test,Status: Polling Phase
loop Until status != "processing"
Test->>Status: Query current status
Status->>Store: get_report_status(report_id)
Store-->>Status: "processing" | "completed"
Status-->>Test: {status: ...}
end
end
rect rgb(220, 240, 220)
Note over Test,GetData: Completion Phase
Test->>GetData: Fetch report data
GetData->>Store: Retrieve saved data
Store-->>GetData: Return data
GetData-->>Test: 200 OK {data: ...}
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
backend/app/services/report_service.py (1)
34-47: Resolve merge conflict and standardize return shape.Unresolved conflict markers prevent the module from loading and break CI. The merge introduced conflicting logic for the completed response structure. Keep the endpoint contract consistent: return data-shaped response only when completed; otherwise None.
Apply this fix to resolve the conflict:
def get_report_data(report_id: str) -> Dict | None: report = in_memory_reports.get(report_id) -<<<<<<< HEAD - if not report: - return None - if report.get("status") == "completed": - return { - "report_id": report_id, - "data": {"agent_results": report.get("agent_results", {})}, - } - return {"report_id": report_id, "status": report.get("status")} -======= - if report and report.get("status") == "completed": - return {"report_id": report_id, "data": report.get("agent_results")} - return None ->>>>>>> 745e0c8 (feat: Add endpoint to retrieve final report data) + if report and report.get("status") == "completed": + return { + "report_id": report_id, + "data": {"agent_results": report.get("agent_results", {})}, + } + return NoneAlso address: this module maintains its own in_memory_reports dict despite the new backend/app/core/storage.py module providing storage helpers. Refactor to delegate to core.storage (set_report_status, get_report_status, save_report_data) to eliminate duplication and prevent state drift.
backend/app/api/v1/routes.py (1)
56-89: Fix merge conflicts in routes.py, test_routes.py, and report_service.py; use JSONResponse (not HTTPException) for 202 to match test expectations.The review comment's proposed HTTPException(202) solution does not match test expectations. The test asserts a 202 response must include
report_idandmessagefields, which HTTPException cannot provide. Use JSONResponse instead. Additionally, conflicts exist in report_service.py and test_routes.py that must be resolved.backend/app/api/v1/routes.py (lines 56–90):
@router.get("/reports/{report_id}/data") async def get_report_data_endpoint(report_id: str): -<<<<<<< HEAD - report_result = get_report_data(report_id) - if report_result: - if "data" in report_result: - return report_result - elif report_result.get("status") == "processing": - # Match test expectations exactly - return JSONResponse( - status_code=202, - content={ - "report_id": report_id, - "message": "Report is still processing.", - "detail": "Report is still processing.", - }, - ) - elif report_result.get("status") == "failed": - return JSONResponse( - status_code=409, - content={ - "report_id": report_id, - "message": "Report failed", - "detail": report_result.get("detail", "Report processing failed."), - }, - ) -======= - report_data = get_report_data(report_id) - if report_data: - return report_data - - report_status = get_report_status_from_memory(report_id) - if report_status and report_status.get("status") == "processing": - raise HTTPException(status_code=202, detail="Report is still processing.") - ->>>>>>> 745e0c8 (feat: Add endpoint to retrieve final report data) + report_result = get_report_data(report_id) + if report_result: + if "data" in report_result: + return report_result + elif report_result.get("status") == "processing": + return JSONResponse( + status_code=202, + content={ + "report_id": report_id, + "message": "Report is still processing.", + "detail": "Report is still processing.", + }, + ) raise HTTPException(status_code=404, detail="Report not found or not completed")backend/app/services/report_service.py (lines 33–42):
def get_report_data(report_id: str) -> Dict | None: report = in_memory_reports.get(report_id) -<<<<<<< HEAD if not report: return None if report.get("status") == "completed": return { "report_id": report_id, "data": {"agent_results": report.get("agent_results", {})}, } return {"report_id": report_id, "status": report.get("status")} -======= - if report and report.get("status") == "completed": - return {"report_id": report_id, "data": report.get("agent_results")} - return None ->>>>>>> 745e0c8 (feat: Add endpoint to retrieve final report data)backend/tests/test_routes.py (remove conflict markers at lines 1, 26, 50):
- Line 1: remove
<<<<<<< HEAD,=======,>>>>>>> 745e0c8and resolve import (keep onlyimport anyiowithoutfunctools)- Line 26 & 50: use
await anyio.to_thread.run_sync(client.post, "/api/v1/report/generate", kwargs={"json": {...}})syntax
🧹 Nitpick comments (1)
backend/app/core/storage.py (1)
1-21: Nice lightweight store; consider adding read/reset helpers to decouple tests and services.Add a getter for data and a reset helper to avoid importing internals in tests. This also enables service/routes to depend solely on this module.
REPORT_STORE = {} def set_report_status(report_id: str, status: str): @@ REPORT_STORE[report_id]["status"] = "completed" + +def get_report_data(report_id: str) -> dict | None: + """Gets the data payload for a report, if any.""" + return REPORT_STORE.get(report_id, {}).get("data") + +def reset_report_store(): + """Clears the in-memory store. Intended for tests/dev only.""" + REPORT_STORE.clear()Note: In-memory dict is not process/worker safe and not thread-safe. If you run multiple Uvicorn workers or threads, states will diverge. For now (dev-only) it’s fine; otherwise wrap mutations in a lock or migrate to a shared cache/DB.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
backend/app/api/v1/routes.py(2 hunks)backend/app/core/storage.py(1 hunks)backend/app/services/report_service.py(2 hunks)backend/tests/test_routes.py(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
backend/app/core/storage.py (2)
backend/app/api/v1/routes.py (1)
get_report_status(48-52)backend/app/services/report_service.py (1)
save_report_data(22-27)
backend/app/api/v1/routes.py (1)
backend/app/services/report_service.py (2)
get_report_data(32-33)get_report_status_from_memory(29-30)
🪛 Ruff (0.14.1)
backend/app/services/report_service.py
34-34: Expected a statement
(invalid-syntax)
34-34: Expected a statement
(invalid-syntax)
34-34: Expected a statement
(invalid-syntax)
34-34: Expected a statement
(invalid-syntax)
43-43: Expected a statement
(invalid-syntax)
43-43: Expected a statement
(invalid-syntax)
43-43: Expected a statement
(invalid-syntax)
43-43: Expected a statement
(invalid-syntax)
43-44: Expected a statement
(invalid-syntax)
44-44: Unexpected indentation
(invalid-syntax)
47-47: Expected a statement
(invalid-syntax)
47-47: Expected a statement
(invalid-syntax)
47-47: Expected a statement
(invalid-syntax)
47-47: Expected a statement
(invalid-syntax)
47-47: Simple statements must be separated by newlines or semicolons
(invalid-syntax)
47-47: Expected ',', found ':'
(invalid-syntax)
47-47: Expected ',', found name
(invalid-syntax)
47-47: Expected ',', found name
(invalid-syntax)
47-47: Expected ',', found name
(invalid-syntax)
47-47: Expected ',', found name
(invalid-syntax)
47-47: Expected ',', found name
(invalid-syntax)
47-47: Expected ',', found name
(invalid-syntax)
backend/tests/test_routes.py
9-9: Expected a statement
(invalid-syntax)
9-9: Expected a statement
(invalid-syntax)
9-9: Expected a statement
(invalid-syntax)
9-9: Expected a statement
(invalid-syntax)
11-11: Expected a statement
(invalid-syntax)
11-11: Expected a statement
(invalid-syntax)
11-11: Expected a statement
(invalid-syntax)
11-11: Expected a statement
(invalid-syntax)
11-12: Expected a statement
(invalid-syntax)
12-12: Expected a statement
(invalid-syntax)
12-12: Expected a statement
(invalid-syntax)
12-12: Expected a statement
(invalid-syntax)
12-12: Expected a statement
(invalid-syntax)
12-12: Simple statements must be separated by newlines or semicolons
(invalid-syntax)
12-12: Expected ',', found ':'
(invalid-syntax)
12-12: Expected ',', found name
(invalid-syntax)
12-12: Expected ',', found name
(invalid-syntax)
12-12: Expected ',', found name
(invalid-syntax)
12-12: Expected ',', found name
(invalid-syntax)
12-12: Expected ',', found name
(invalid-syntax)
12-12: Expected ',', found name
(invalid-syntax)
27-27: Expected an indented block after function definition
(invalid-syntax)
27-27: Expected a statement
(invalid-syntax)
27-27: Expected a statement
(invalid-syntax)
27-27: Expected a statement
(invalid-syntax)
28-28: Unexpected indentation
(invalid-syntax)
32-32: Expected a statement
(invalid-syntax)
32-32: Expected a statement
(invalid-syntax)
32-32: Expected a statement
(invalid-syntax)
32-32: Expected a statement
(invalid-syntax)
32-33: Expected a statement
(invalid-syntax)
34-34: Unexpected indentation
(invalid-syntax)
41-41: Expected a statement
(invalid-syntax)
41-41: Expected a statement
(invalid-syntax)
41-41: Expected a statement
(invalid-syntax)
41-41: Expected a statement
(invalid-syntax)
41-41: Simple statements must be separated by newlines or semicolons
(invalid-syntax)
41-41: Expected ',', found ':'
(invalid-syntax)
41-41: Expected ',', found name
(invalid-syntax)
41-41: Expected ',', found name
(invalid-syntax)
41-41: Expected ',', found name
(invalid-syntax)
41-41: Expected ',', found name
(invalid-syntax)
41-41: Expected ',', found name
(invalid-syntax)
41-41: Expected ',', found name
(invalid-syntax)
52-52: Expected an indented block after function definition
(invalid-syntax)
52-52: Expected a statement
(invalid-syntax)
52-52: Expected a statement
(invalid-syntax)
52-52: Expected a statement
(invalid-syntax)
53-53: Unexpected indentation
(invalid-syntax)
57-57: Expected a statement
(invalid-syntax)
57-57: Expected a statement
(invalid-syntax)
57-57: Expected a statement
(invalid-syntax)
57-57: Expected a statement
(invalid-syntax)
57-58: Expected a statement
(invalid-syntax)
58-58: Unexpected indentation
(invalid-syntax)
64-64: Expected a statement
(invalid-syntax)
64-64: Expected a statement
(invalid-syntax)
64-64: Expected a statement
(invalid-syntax)
64-64: Expected a statement
(invalid-syntax)
64-64: Simple statements must be separated by newlines or semicolons
(invalid-syntax)
64-64: Expected ',', found ':'
(invalid-syntax)
64-64: Expected ',', found name
(invalid-syntax)
64-64: Expected ',', found name
(invalid-syntax)
64-64: Expected ',', found name
(invalid-syntax)
64-64: Expected ',', found name
(invalid-syntax)
64-64: Expected ',', found name
(invalid-syntax)
64-64: Expected ',', found name
(invalid-syntax)
backend/app/api/v1/routes.py
56-56: Expected an indented block after function definition
(invalid-syntax)
56-56: Expected a statement
(invalid-syntax)
56-56: Expected a statement
(invalid-syntax)
56-56: Expected a statement
(invalid-syntax)
80-80: Expected a statement
(invalid-syntax)
80-80: Expected a statement
(invalid-syntax)
80-80: Expected a statement
(invalid-syntax)
80-80: Expected a statement
(invalid-syntax)
80-81: Expected a statement
(invalid-syntax)
81-81: Unexpected indentation
(invalid-syntax)
89-89: Expected a statement
(invalid-syntax)
89-89: Expected a statement
(invalid-syntax)
89-89: Expected a statement
(invalid-syntax)
89-89: Expected a statement
(invalid-syntax)
89-89: Simple statements must be separated by newlines or semicolons
(invalid-syntax)
89-89: Expected ',', found ':'
(invalid-syntax)
89-89: Expected ',', found name
(invalid-syntax)
89-89: Expected ',', found name
(invalid-syntax)
89-89: Expected ',', found name
(invalid-syntax)
89-89: Expected ',', found name
(invalid-syntax)
89-89: Expected ',', found name
(invalid-syntax)
89-89: Expected ',', found name
(invalid-syntax)
2 participants
Overview: This PR introduces a temporary in-memory storage solution for report data and statuses, facilitating rapid development.
Changes
app/core/storage.pyto manage report data.REPORT_STOREto map report IDs to their status and data.set_report_status,get_report_status, andsave_report_datafor easy interaction.Summary by CodeRabbit