Skip to content

status_check - #31

Merged
alain-sv merged 10 commits into
developfrom
status_check
May 3, 2026
Merged

status_check#31
alain-sv merged 10 commits into
developfrom
status_check

Conversation

@alain-sv

@alain-sv alain-sv commented May 3, 2026

Copy link
Copy Markdown
Contributor

What changed- Added POST /api/supervaizeragents/{agent_slug}/ endpoint (Controller.POST_AGENT_STATUS) and mapped route.

  • Endpoint now returns standardized JobResponse model and handler return type updated accordingly.
  • Added integration tests that post to the status endpoint and verify job_id, status, message payload are returned and serialized correctly.
  • Updated contract to assert the new endpoint is resolved to ensure contract consistency.
  • Adjusted from info to debug in case methods to reduce noisy logs during normal.

Why- Expose agent job status via a dedicated, standardized route so clients can reliably obtain job metadata.

  • Ensure API contract stability and prevent regressions via tests.
  • Reduce noisy logs during normal operation.

Notes- Tests patch Agent.job_status to return JobResponse with EntityStatus to validate mapping and serialization.

alain-sv added 8 commits May 3, 2026 22:54
…AGENT_STATUS and map to /api/supervaizer/agents/{agent_slug}/status the controller contract a dedicated status route.

Update contracts tests to assert new endpoint name resolved, ensuring API contract consistency.
- Add HTTP POST / route handler response model changed to JobResponse and update the handler return type accordingly the endpoint returns standardized job statuss.
- integration test that posts to the agent status endpoint and verifies_id, status and payload fields are returned; confirms agent.job is wired and serialized correctly.
- Change logging in case methods from info to debug to avoid noisy logs normal operation.
…erifies POST //{slug}/status returns the agent'sjob_status as a JobResponse. The test patches Agent.job_statusreturn aResponse with job_id, EntityStatus_PROGRESS, message, mounts the agents routes the app, sends a request withjob_id and the200 response and that job_id status andpayload fields match the patched response.

Also import JobResponse and EntityStatus in the tests to construct theexpected response object and check the returned status string This ensures status endpoint correctly agent job status responses to the APIoutput.
…w test that verifies POST /agents/{slug}/status returns theagent's job_status response as a JobResponse with correct job_idstatus, message, and payload. Patch Agent.job_status return aJobResponse with EntityStatus.IN_PROGRESS and assert the API mapsthe status and payload values correctly.

This ensures the status route preserves and exposes jobmetadata (id, status, payload) for clients and guards againstregressions in status serialization.
@qodo-code-review

Copy link
Copy Markdown

Review Summary by Qodo

Add agent status endpoint with standardized JobResponse model

✨ Enhancement 🧪 Tests

Grey Divider

Walkthroughs

Description
• Add POST /api/supervaizer/agents/{agent_slug}/status endpoint returning JobResponse
• Implement standardized job status response with job_id, status, and payload fields
• Add integration test verifying status endpoint returns correct job metadata
• Update controller contract to include new POST_AGENT_STATUS endpoint
• Reduce logging noise by changing case methods from info to debug level
Diagram
flowchart LR
  A["Agent Status Request"] -->|POST /status| B["status_agent Handler"]
  B -->|calls| C["agent.job_status"]
  C -->|returns| D["JobResponse"]
  D -->|serialized| E["Client Response"]
  F["ControllerEndpoint"] -->|maps| G["POST_AGENT_STATUS"]
  G -->|routes to| B
Loading

Grey Divider

File Changes

1. src/supervaizer/case.py Logging +2/-2

Reduce logging noise in case methods

• Changed logging level from info to debug in request_human_input method
• Changed logging level from info to debug in request_human_input_sync method
• Reduces noisy logs during normal operation

src/supervaizer/case.py


2. src/supervaizer/contracts.py ⚙️ Configuration changes +2/-0

Add POST_AGENT_STATUS to controller contract

• Added POST_AGENT_STATUS enum value to ControllerEndpoint
• Mapped POST_AGENT_STATUS to /api/supervaizer/agents/{agent_slug}/status route
• Ensures API contract consistency for new status endpoint

src/supervaizer/contracts.py


3. src/supervaizer/routes.py ✨ Enhancement +9/-10

Update status endpoint to return JobResponse model

• Changed status_agent handler return type from AgentResponse to JobResponse
• Updated response_model decorator to JobResponse
• Updated HTTP 202 response model to JobResponse
• Simplified handler to return agent.job_status() result directly
• Added error handling for null job_status responses with HTTP 404

src/supervaizer/routes.py


View more (2)
4. tests/test_contracts.py 🧪 Tests +12/-0

Add contract tests for status endpoint

• Added assertion for POST_AGENT_STATUS endpoint in contract info test
• Added assertion for resolve_controller_endpoint with POST_AGENT_STATUS
• Verifies new endpoint is properly resolved in controller contract

tests/test_contracts.py


5. tests/test_routes.py 🧪 Tests +35/-0

Add integration test for status endpoint

• Added imports for JobResponse and EntityStatus
• Added new test test_agent_status_endpoint_returns_job_status_response
• Test mocks Agent.job_status to return JobResponse with job metadata
• Test verifies POST request returns correct job_id, status, and payload fields
• Confirms endpoint serialization and response mapping

tests/test_routes.py


Grey Divider

Qodo Logo

@qodo-code-review

qodo-code-review Bot commented May 3, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Remediation recommended

1. Misleading 202 status docs🐞 Bug ≡ Correctness
Description
The new POST /status route advertises only a 202 Accepted response in its OpenAPI responses, but
the handler returns a normal value with no status_code override (tests assert 200). This mismatch
can break client generation/expectations and makes the contract ambiguous.
Code

src/supervaizer/routes.py[R881-889]

     "/status",
     summary=f"Get the status of the agent: {agent.name}",
     description="Get the status of the agent",
+        response_model=JobResponse,
     responses={
-            http_status.HTTP_202_ACCEPTED: {"model": AgentResponse},
+            http_status.HTTP_202_ACCEPTED: {"model": JobResponse},
     },
     dependencies=[
         Depends(require_scope("write"))
Evidence
The /status decorator declares only HTTP_202_ACCEPTED in responses while the integration test
asserts a 200 response for the same endpoint; additionally, other endpoints that truly mean 202 set
status_code=HTTP_202_ACCEPTED, which /status does not.

src/supervaizer/routes.py[880-887]
tests/test_routes.py[151-181]
src/supervaizer/routes.py[724-736]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`POST /status` documents only a `202 Accepted` response, but the implementation returns a normal `JobResponse` without setting `status_code`, and tests expect `200 OK`. This creates an OpenAPI/behavior mismatch.
### Issue Context
Other endpoints that intend `202` explicitly set `status_code=http_status.HTTP_202_ACCEPTED`.
### Fix Focus Areas
- src/supervaizer/routes.py[880-891]
### Suggested fix
- Decide the intended status code:
- If this is a read-style status fetch, set `status_code=http_status.HTTP_200_OK` and update `responses` to include `HTTP_200_OK` (and optionally `404`).
- If it should be `202`, add `status_code=http_status.HTTP_202_ACCEPTED` and update the test expectation accordingly.
- Update `responses` to reflect actual success + error codes (e.g., 200/404/400).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Sync call blocks event loop🐞 Bug ➹ Performance
Description
The async status_agent handler calls agent.job_status(...) synchronously; since
Agent.job_status ultimately runs the underlying method via a direct Python call, the request will
block the FastAPI event loop for the duration of the agent code. This harms concurrency/latency,
especially if clients poll status frequently.
Code

src/supervaizer/routes.py[R893-903]

 async def status_agent(
     params: AgentMethodParams, agent: Agent = Depends(get_agent)
-    ) -> AgentResponse:
+    ) -> JobResponse:
     log.info(f"📥  POST /status [Status agent] {agent.name} with params {params}")
     result = agent.job_status(params.params)
-        return AgentResponse(
-            name=agent.name,
-            id=agent.id,
-            version=agent.version,
-            api_path=agent.path,
-            description=agent.description,
-            **result if result else {},
-        )
+        if result is None:
+            raise HTTPException(
+                status_code=http_status.HTTP_404_NOT_FOUND,
+                detail=f"Agent {agent.name} did not return a job status",
+            )
+        return result
Evidence
status_agent is declared async but calls agent.job_status without await/thread offload.
Agent.job_status delegates to _execute, which imports and invokes the target function via
result = method(**params) synchronously.

src/supervaizer/routes.py[893-903]
src/supervaizer/agent.py[983-988]
src/supervaizer/agent.py[848-862]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`status_agent` is an async route but executes agent code synchronously (via `agent.job_status` -> `_execute` -> `method(**params)`). This blocks the event loop while the agent status function runs.
### Issue Context
Even if many implementations are fast, a single slow `job_status` will stall concurrent requests on the same worker.
### Fix Focus Areas
- src/supervaizer/routes.py[893-903]
- src/supervaizer/agent.py[983-988]
- src/supervaizer/agent.py[848-862]
### Suggested fix
- In `status_agent`, call the sync function in a worker thread:
- `result = await asyncio.to_thread(agent.job_status, params.params)`
- (Optional) Apply the same pattern to other sync agent method endpoints (`/stop`, etc.) if they are expected to do non-trivial work.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Advisory comments

3. Dead None check 🐞 Bug ⚙ Maintainability
Description
status_agent raises a 404 when result is None, but the default Agent._execute enforces that
agent methods return a JobResponse and raises TypeError otherwise, so this None branch is
effectively unreachable under the current implementation. Keeping it can mislead future maintainers
about actual error behavior.
Code

src/supervaizer/routes.py[R896-903]

     log.info(f"📥  POST /status [Status agent] {agent.name} with params {params}")
     result = agent.job_status(params.params)
-        return AgentResponse(
-            name=agent.name,
-            id=agent.id,
-            version=agent.version,
-            api_path=agent.path,
-            description=agent.description,
-            **result if result else {},
-        )
+        if result is None:
+            raise HTTPException(
+                status_code=http_status.HTTP_404_NOT_FOUND,
+                detail=f"Agent {agent.name} did not return a job status",
+            )
+        return result
Evidence
Agent._execute raises if the underlying method does not return a JobResponse, preventing None
from flowing back to the route; therefore if result is None: will not be hit unless
Agent.job_status is bypassed/overridden.

src/supervaizer/routes.py[896-903]
src/supervaizer/agent.py[848-862]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`status_agent` checks `if result is None` and raises 404, but `Agent._execute` already enforces a `JobResponse` return and will raise `TypeError` on `None`/other types.
### Issue Context
This makes the route’s apparent behavior (404 on None) differ from the real behavior (400/500 depending on exception handling).
### Fix Focus Areas
- src/supervaizer/routes.py[896-903]
- src/supervaizer/agent.py[848-862]
### Suggested fix
- Remove the `if result is None:` block, or
- If a 404 is desired for a specific condition (e.g., job_id not found), implement an explicit lookup/validation and raise 404 based on that condition (not on an unreachable `None` return).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

alain-sv added 2 commits May 4, 2026 00:43
…ks off the event loop byexecuting them in background threads via.to_thread.

- Await dynamic_choices in routes.start using asyncio.to_thread to avoid blocking the loop computing choices.
- Run agent.job_stop and agent.job_status in asyncio.to_thread from the HTTP POST handlers so synchronous user code cannot block request processing.
- Update POST /status responses to include200/400/404/500 models clearer OpenAPI descriptions.
- Document synchronous behavior in Agent._execute, job_stop and job_status docstrings to clarify that controller endpoints should call them from worker threads.

This long-running user code from blocking the async server andpro API documentation for/response cases
…int to use JobResponse as mainmodel and an explicit200 status in the route decoratorFastAPI the correct response code default. AlignAPIschema reference JobResponse and add detailed error (400,

404,500 referencing ErrorResponse improve API documentation andclient handling. This makes endpoint behavior and docsconsistent and clearer consumers
@alain-sv
alain-sv merged commit e4ff9c1 into develop May 3, 2026
6 checks passed
@alain-sv
alain-sv deleted the status_check branch May 13, 2026 13:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant