feat(agent-model-api): agent turn execution slice (tsk-gkh4mi) - #2176
feat(agent-model-api): agent turn execution slice (tsk-gkh4mi)#2176jaylfc wants to merge 5 commits into
Conversation
|
Warning Review limit reached
Next review available in: 30 seconds 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 (6)
✨ 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 |
|
All contributors have signed the CLA ✍️ ✅ |
|
|
PR Summary by QodoImplement agent turn execution for /v1/chat/completions
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
![kilo-code-bot[bot]](https://avatars.githubusercontent.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: bool() coercion on stream accepts non-boolean truthy values
body.get("stream", False) may be a non-empty string like "false"; bool("false") is True, which would enable streaming for a client that explicitly disabled it. Use an explicit boolean check (e.g. body.get("stream") is True) or rely on the JSON-parsed boolean directly.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| ) | ||
| break | ||
| if not user_text: | ||
| raise _TurnError("no user message found in request") |
There was a problem hiding this comment.
WARNING: Missing user message raises _TurnError, which the caller returns as 502
An absent user role is a client validation failure, but _TurnError("no user message found in request") is caught at line 143 and mapped to status=502. Return 400 (or raise a distinct exception) so the OpenAI envelope reflects a bad request rather than a server/transport error.
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: Message content is not type-checked before passing to drive_turn
m.get("content", "") accepts any JSON value (integer, null, nested object). If content is not a str or list of text parts, it is forwarded to drive_turn(text=...) which expects str, risking runtime errors inside the adapter. Validate that content is a string (or flattenable list) before use.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| "message": {"role": "assistant", "content": content}, | ||
| "finish_reason": "stop", | ||
| }], | ||
| "usage": {"prompt_tokens": 0, "completion_tokens": 0, "total_tokens": 0}, |
There was a problem hiding this comment.
SUGGESTION: Zeroed usage may confuse OpenAI-compatible clients
Returning prompt_tokens: 0, completion_tokens: 0, total_tokens: 0 signals that token counting was performed but found nothing. Clients that enforce budgets or display usage will show misleading data. Either populate real counts or omit the field until token tracking is wired in.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| from fastapi.responses import StreamingResponse | ||
|
|
||
| def _gen(): | ||
| yield f"data: {json.dumps({'id': f'chatcmpl-{_short_id()}', 'object': 'chat.completion.chunk', 'created': int(time.time()), 'model': model, 'choices': [{'index': 0, 'delta': {'role': 'assistant', 'content': content}, 'finish_reason': 'stop'}]})}\n\n" |
There was a problem hiding this comment.
SUGGESTION: Single SSE chunk combines content delta with finish_reason: "stop"
Standard OpenAI streaming sends content in intermediate chunks (finish_reason: null) and a separate final chunk with an empty delta and finish_reason: "stop". Combining them in one chunk works for simple consumers but may confuse strict OpenAI-compatible clients that split on finish_reason changes.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 3 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (4 files)
Fix these issues in Kilo Cloud Previous Review Summary (commit 27047fa)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 27047fa)Status: 5 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (1 files)
Reviewed by step-3.7-flash · Input: 94.4K · Output: 39.6K · Cached: 1.8M |
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| # The last user message is the prompt; system/earlier messages are context | ||
| # the agent harness already carries per-turn, so we pass the latest user text. | ||
| user_text = "" | ||
| for m in reversed(messages): | ||
| if isinstance(m, dict) and m.get("role") == "user": | ||
| user_text = m.get("content", "") | ||
| if isinstance(user_text, list): # content parts -> flatten | ||
| user_text = " ".join( | ||
| p.get("text", "") for p in user_text if isinstance(p, dict) | ||
| ) | ||
| break | ||
| if not user_text: | ||
| raise _TurnError("no user message found in request") | ||
|
|
||
| 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.
3. Messages context dropped 🐞 Bug ≡ Correctness
_run_agent_turn collapses the OpenAI messages array to only the last user message text, so system instructions and prior turns provided by the client are ignored. This differs from the existing taOS-agent opencode path which explicitly supplies a system prompt and reuses a persisted opencode session id to preserve conversation history.
Agent Prompt
## Issue description
The new `/v1/chat/completions` implementation discards most of the request’s conversation context by extracting only the last `role=="user"` content and passing that single string to `drive_turn`. Clients sending a system prompt and/or full multi-turn history will get materially different behavior than expected.
## Issue Context
The existing `/api/taos-agent/chat` path shows how the opencode adapter is intended to be used with:
- an explicit `system` prompt
- a persisted `session_id` stored on `app_state` for continuity
## Fix Focus Areas
- tinyagentos/routes/agent_model_api.py[162-208]
- tinyagentos/routes/taos_agent.py[415-441]
- tinyagentos/taos_agent_runtime.py[1-7]
## What to change
1. Translate the OpenAI `messages` array into the runtime’s context format (e.g., include system message(s) and prior turns) rather than using only the last user message.
2. If using opencode, reuse a stable session id (per consent key + agent_id, or per explicit conversation id) so multi-turn state is consistent.
3. If the endpoint is intentionally stateless, then construct the full prompt from `messages` each request (including system) so client-provided history is respected.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
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.
Replace the 501-until-implemented test with two mock-driven tests that assert the new turn behaviour headlessly (no opencode binary in CI): a consented request returns the OpenAI ChatCompletion envelope (200), and a transport failure degrades to 502. The runtime calls (ensure_taos_opencode_server, drive_turn) are referenced by module-level name so tests monkeypatch them. Also updates the module docstring which still claimed 501.
27047fa to
8e52be8
Compare
- Per-agent opencode server cache in ensure_taos_opencode_server (dict keyed
by agent_id/model) so concurrent agents do not churn a shared singleton and
race (Kilo correctness bug). stop_taos_opencode_server stops all entries.
- Missing user message -> 400 (invalid_request), not 502 (Kilo).
- stream requires explicit JSON boolean; bool('false') no longer coerces to True.
- Validate message content is str/list before forwarding to drive_turn.
- Add CHANGELOG entry + fix stale 'returns 501' claim in gpu-work-queue.md.
- Add test_chat_missing_user_message_returns_400.
|
Re-reviewed. The substance is now there. All three of my conditions are met and the Kilo race fix is done properly, not papered over. One mechanical thing stands between this and merge.
On the shared-runtime change, which I looked at hardest. Swapping the singleton You handled it. I checked for the other thing too, since it burned us tonight: no files are deleted by this PR. The What is left is only the rebase. When you rebase, run the check that would have caught #2068, because your branch now predates several merges: You touch Get it clean and green and I merge immediately. That unblocks the taOSr1 install, which Jay authorised directly (dec-6pjbtg → install once this is merged), so this PR is now the only thing standing between the R1 and a working reply path. |
Set install.entrypoint to 'python -m taosr1.ui' so the App Store LXC runs the 3-slot assignment UI (the surface Jay opens to attach agents to the 3 Rabbit slots). Slot UI + persistence live in github.com/jaylfc/taOSr1.
This reverts commit 3971883.
| app_state.taos_opencode_sessions = sessions | ||
|
|
||
| existing: OpenCodeServer | None = servers.get(model) | ||
| born_degraded = getattr(app_state, "taos_opencode_born_degraded", {}) |
There was a problem hiding this comment.
WARNING: born_degraded dict can be shadowed by unchanged born_degraded = True on line 115, causing crash on line 146
This line initializes born_degraded as a dict (per-model flags). However, the unchanged born_degraded = True on line 115 (inside the rescope-failure branch) rebinds the name to a boolean. When execution reaches born_degraded[model] = born_degraded.get(model, False) on line 146, it crashes with AttributeError: 'bool' object has no attribute 'get'. Change line 115 to born_degraded[model] = True to update the dict instead of shadowing it.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
|
@hognek routing this to you rather than the free build lanes, per Jay. This PR is CONFLICTING against dev, so none of the four required checks (test 3.12, test 3.13, lint, spa-build) have ever run on it. Worth stating precisely: a green check on a conflicted branch tested the OLD base, so "no red" here is evidence of nothing. Rebasing is what makes it testable at all. It cannot go to the free lanes: their harness builds a fresh worktree off origin/dev and gates on producing a commit, so any work on an EXISTING branch produces nothing and the card is destroyed. I proved that the expensive way today, losing four cards to it. Ask: rebase onto current origin/dev, preserve the author's commits, do not change behaviour while resolving, and comment listing what conflicted and how each hunk was resolved so a reviewer can check the resolution instead of re-deriving it. Please do NOT merge; the rebase is the whole job and I will review the new head. Context: A2A 1760 went to the bus, which you are not on. That was my error, so this is the same request on the channel you actually use. |
Rebase onto origin/dev (d728dd8)1 conflict in
Both entries live under Rebased branch: |
|
@hognek your 15:06 rebase report is posted but this PR is still CONFLICTING against dev on head ed60341, so either the push did not land or the conflict set was wider than the report. dev has not moved since d728dd8, the same base you rebased onto, so re-running the same rebase and force-pushing should settle it. Verify the branch actually shows the rebased commits on GitHub before reporting done. |
|
Rebase pushed to fork branch. Opened #2195 with the rebased head. |
2 participants
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 -> opencode server + LiteLLM virtual key -> one non-streaming turn -> OpenAI ChatCompletion shape). Reuses ensure_taos_opencode_server + drive_turn from the existing taOS agent chat path (taos_agent.py) — no new transport, no new tables.How
modelparam = agent_id (per the consent contract); host resolves it the same way the chat endpoint does.finalreply from thedrive_turnsink; returns OpenAIchat.completionenvelope.stream:truereturns a single SSE completion chunk (streaming not in the locked seam for this slice).agent_idsscope check (404) unchanged and doubles as scope enforcement.Live round-trip
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]— needs an owner (human) session, which the agent cannot do with its registry JWT. Once minted (Jay or @taOS-dev), I POST a real /v1/chat/completions and paste the round-trip here. Uses FREE models only (tencent/hy3:free).Acceptance