feat: agentic-server stateful conversation/responses endpoints.#48
Conversation
Signed-off-by: maral <maralbahari.98@gmail.com>
Signed-off-by: maral <maralbahari.98@gmail.com>
Signed-off-by: maral <maralbahari.98@gmail.com>
Signed-off-by: maral <maralbahari.98@gmail.com>
Signed-off-by: maral <maralbahari.98@gmail.com>
Signed-off-by: maral <maralbahari.98@gmail.com>
Signed-off-by: maral <maralbahari.98@gmail.com>
Signed-off-by: maral <maralbahari.98@gmail.com>
Signed-off-by: maral <maralbahari.98@gmail.com>
Signed-off-by: maral <maralbahari.98@gmail.com>
Signed-off-by: maral <maralbahari.98@gmail.com>
Add executor module: rehydration, LLM inference, SSE accumulation, and persistence for both conversation and response stateful flows. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: maral <maralbahari.98@gmail.com>
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: maral <maralbahari.98@gmail.com>
Signed-off-by: maral <maralbahari.98@gmail.com>
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: maral <maralbahari.98@gmail.com>
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: maral <maralbahari.98@gmail.com>
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: maral <maralbahari.98@gmail.com>
…ame entry Signed-off-by: maral <maralbahari.98@gmail.com>
Signed-off-by: maral <maralbahari.98@gmail.com>
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: maral <maralbahari.98@gmail.com>
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: maral <maralbahari.98@gmail.com>
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: maral <maralbahari.98@gmail.com>
Signed-off-by: maral <maralbahari.98@gmail.com>
agentic-server stateful conversation/responses endpoints.agentic-server stateful conversation/responses endpoints.
Signed-off-by: maral <maralbahari.98@gmail.com>
Signed-off-by: maral <maralbahari.98@gmail.com>
Signed-off-by: maral <maralbahari.98@gmail.com>
Signed-off-by: maral <maralbahari.98@gmail.com>
Signed-off-by: maral <maralbahari.98@gmail.com>
Signed-off-by: maral <maralbahari.98@gmail.com>
|
@franciscojavierarceo @ashwing @leseb Ready for review |
Signed-off-by: maral <maralbahari.98@gmail.com>
Signed-off-by: maral <maralbahari.98@gmail.com>
Signed-off-by: maral <maralbahari.98@gmail.com>
ashwing
left a comment
There was a problem hiding this comment.
Nice work wiring the executor into the server — clean routing design. Left some inline comments, mostly architectural questions for the future (tool dispatch, concurrency). Nothing blocking.
| use tracing::warn; | ||
|
|
||
| use agentic_core::executor::{BoxStream, ExecutionContext, ExecutorError, create_conversation, execute}; | ||
| use agentic_core::proxy::{ProxyBody, ProxyRequest, ProxyResponse, error_response, proxy_request}; |
There was a problem hiding this comment.
The body is parsed twice: read_body deserializes to serde_json::Value to peek at store, then execute_responses re-parses into RequestPayload. For the proxy path this is fine (raw bytes forwarded), but for the executor path the double parse adds CPU cost that scales with input size.
Not blocking — conversations are small today. But when history gets long, consider parsing once into Value and using serde_json::from_value() for the executor path.
There was a problem hiding this comment.
@ashwing I refactored further to avoid the double parsing.
| let trimmed = key.trim(); | ||
| if !trimmed.is_empty() { | ||
| if let Ok(v) = reqwest::header::HeaderValue::from_str(&format!("Bearer {trimmed}")) { | ||
| headers.insert(reqwest::header::AUTHORIZATION, v); |
There was a problem hiding this comment.
When the client sends a different Bearer token, the entire ExecutionContext is cloned (both handlers, client Arc, config) to set one field. Under concurrency this creates many short-lived Arc<ExecutionContext> allocations.
Could client_auth move to being a parameter of execute() or a field on RequestContext (which is already per-turn)? That way ExecutionContext stays truly shared — one Arc for the server lifetime.
There was a problem hiding this comment.
@ashwing yeh I agree I do not like the current way of handling client_auth but the change requires a bit of a refactoring in agentic-core. so we can use builder pattern where we use RequectContext().auth().run(). to not expand this PR larger with such refactoring I'll open another PR to address this.
| Ok(resp) => { | ||
| warn!("LLM backend not ready: status {}", resp.status()); | ||
| warn!("LLM backend not ready: {}", resp.status()); | ||
| StatusCode::SERVICE_UNAVAILABLE |
There was a problem hiding this comment.
Clean design — when tool dispatch lands, the change is just:
// Current:
match execute(payload, resolve_exec_ctx(state, &parts)).await {
// Future:
match execute_loop(payload, resolve_exec_ctx(state, &parts), tool_ctx).await {Should ToolContext live in AppState (configured at startup)? Or constructed per-request from the request's tools array? If per-request, the handler builds it here.
There was a problem hiding this comment.
@ashwing I think it should be handled per-request the app state is only one upon application run while the ToolContext would change and vary per-request.
| return Err(convert_response(error_response( | ||
| StatusCode::PAYLOAD_TOO_LARGE, | ||
| "body_too_large", | ||
| "request body too large", |
There was a problem hiding this comment.
If a client sends {"store": false, "previous_response_id": "resp_abc"}, this routes to proxy_responses which forwards bytes to vLLM. But vLLM doesn't know about previous_response_id — it ignores it.
The client thinks they're continuing a conversation, but they're getting a fresh response with no history. Should store=false + previous_response_id either (a) validate the ID exists then proxy, or (b) reject with 400?
There was a problem hiding this comment.
@ashwing thanks for catching. I added a check in response function before diverging between proxy and gateway response rehydration path to return with InvalidRequest error.
| Ok(resp) => { | ||
| warn!("LLM backend not ready: status {}", resp.status()); | ||
| warn!("LLM backend not ready: {}", resp.status()); | ||
| StatusCode::SERVICE_UNAVAILABLE |
There was a problem hiding this comment.
Architectural note for future: two simultaneous requests with the same conversation_id both rehydrate the same item_ids snapshot, both infer, both persist. The second persist overwrites — first request's items get lost from the ordered list.
Not a bug today (single-user, low concurrency). Worth a TODO for when multi-user or high-concurrency scenarios arise. Options: optimistic locking (version column), or append-only with conflict detection.
There was a problem hiding this comment.
@ashwing I think this should be handled in agentic-core/storage module not in server level cause it would be messy to handle in server level. while in storage module we can get help of DB transaction for concurrent request for such cases. will add this into Issue.
| @@ -37,7 +38,7 @@ | |||
There was a problem hiding this comment.
If the body is valid JSON but missing model (empty string after deser), the executor forwards it to vLLM which returns its own 400/422. This works (error surfaces via ExecutorError::LLMRequest) but the error message comes from vLLM, not us.
Similarly POST /v1/conversations ignores all fields except store. Is that intentional per spec, or should we validate required fields at the handler layer for clearer error messages?
There was a problem hiding this comment.
@ashwing for v1/converstion it is intentional cause it is only creating a conversation session. for the model request validity actually I tested by not passing correct name and our gateway catches the error from proxy as we should to tell the user their requested model is wrong. we could actually use v1/models from vLLM server to actually get the model name if user request payload didn't send the model name or even verify the request payload matches the what is actually hosted through v1/models. also we should expose v1/models to our agentic-api gateway to proxy to upstream.
I am thinking once we have most of the components in we can have a request payload validation once per request. right now the validation of store, previouse_response_id, conversation_id and etc are all scattered and making the bookkeeping difficult. we could think of a design the request payload is instantiated we do the validations per case documented in its struct . like request_payload.validation_for_proxy , request_payload.validate_for_conv_rehydration` etc.
Signed-off-by: maral <maralbahari.98@gmail.com>
…ute API Signed-off-by: maral <maralbahari.98@gmail.com>
4c2004c to
de87216
Compare
Signed-off-by: maral <maralbahari.98@gmail.com>
leseb
left a comment
There was a problem hiding this comment.
a few things from my agent:
-
[P1] Kill vLLM when DB initialization fails
server.rs:77
Inservemode, ifDATABASE_URLis invalid/unwritable or schema
setup fails after the child LLM becomes ready, this?returns
before the cleanup block at the end of the function, leaving the
spawned vLLM process running. Please either initialize state before
spawning or kill/wait the child on this error path. -
[P2] Preserve no-store continuations through the executor
handler.rs:180-183
Forstore:falserequests that include a storedprevious_response_id
orconversation_id, the core executor already supports rehydrating
prior history while skipping persistence for the new response. This
preemptive 400 breaks that stateless-continuation use case and forces
clients either to persist the new turn or lose context; these requests
should route through the executor instead of being rejected. -
[P2] Return 4xx for malformed response requests
handler.rs:63-64
When a client sends malformed JSON or a schema-invalid body to
/v1/responses, this wraps theserde_json::ErrorasJsonError;
ExecutorError::http_status()falls through to 500 for that variant,
so bad client input is reported as an internal server error. Map
request-body parse failures to an invalid-request/parse error here
so callers get a 4xx.
════════════════════════════════════════════════════════════
Signed-off-by: maral <maralbahari.98@gmail.com>
Signed-off-by: maral <maralbahari.98@gmail.com>
Signed-off-by: maral <maralbahari.98@gmail.com>
@leseb |
Summary
To be reviewed after #46
Wires
agentic-core's executor intoagentic-server, giving the gatewaystateful conversation and response management on top of the existing vLLM
proxy. Implements the Layer 2 (HTTP API) component of
ADR-03.
New HTTP endpoints:
/v1/conversationscreate_conversation(): creates a DB-backed conversation record; response body includesid(theconversation_id) along withcreated_at,object, andmetadata/v1/responsesstorefield; executor path returns aResponsePayloadJSON object (id,object,created_at,model,status,output,usage,previous_response_id,conversation_id, …) for non-streaming requests, or an SSE stream of the same shape for streaming; proxy path forwards the vLLM response verbatimRouting logic (
storefield):store=true(default per spec) → executor path:execute()handles rehydration, LLM inference, and persistencestore=false→ proxy path: request forwarded directly to vLLM, no DB involvementBoth paths share a single
/v1/responseshandler that reads the body once, peeks atstore, and dispatches without re-buffering.Rehydration scope (
conversation_idfield):Within the executor path,
/v1/responsesselects its rehydration strategy based on the presence ofconversation_idin the request body:conversation_idpresent →rehydrate_from_conversation(): loads the full conversation turn history from the DB and reconstructs context for the next turnconversation_idabsent,previous_response_idpresent →rehydrate_from_response(): loads only the chained response history, with no conversation record involvedState design:
AppStateholds both states simultaneously, with no feature flags or optional fields:Configgains an optionaldb_urlfield. The server defaults tosqlite://./agentic_api.dbwhen unset.Per-request auth override:
If the incoming
Authorization: Bearer <token>differs from the configured key,a lightweight
ExecutionContextclone is created for that request so auth isnot shared across concurrent requests.
Test Plan
Unit / integration tests (13 new, all passing):
tests/responses_test.rs(4 tests):store=falseproxies JSON response from mock vLLMstore=falseproxies SSE stream from mock vLLM with correct content-typestore=truereaches executor path, not the proxy's 200tests/conversations_test.rs(3 tests):store=falsein body returns 400store=true, reaches executor pathstore=true, reaches executor pathtests/health_test.rsandtests/cors_test.rsrefactored to share helpers viatests/common/mod.rs.All 135 tests pass across the workspace.
cargo testRunning Tests
cargo test -p agentic-serverRunning Benchmarks
Benchmark Results
BENCH_TURNS=10 LLM_BASE_URL=http://localhost:9090 \ BENCH_MODEL=Qwen/Qwen3-30B-A3B-FP8 \ cargo bench -p agentic-server --bench benches -- \ conversation_rehydration response_rehydration --sample-size 10conversation_rehydrationresponse_rehydrationAnalysis
Both paths stay well under 3 s across all turn counts. Gateway overhead (DB reads, rehydration, prompt reconstruction) is not the bottleneck; LLM inference time dominates, which is why confidence intervals are wide (often spanning ~0.5–1 s) even with 10 samples.
conversation_rehydrationshows a mild upward trend with turn count. Non-streaming median grows from ~2.1 s at turn 1 (no prior context) to ~2.8 s at turn 10 (9 prior turns rehydrated), an incremental cost of roughly 0.08 s per prior turn. This is expected: each additional turn adds to the reconstructed prompt sent to the LLM. The cost is low enough that it does not accumulate dangerously over long conversations.response_rehydrationshows no clear trend with chain depth. Medians are flat and noisy across turns (2.03–2.85 s non-streaming), which suggests the DB fetch overhead per chained response is negligible relative to inference variance.Streaming and non-streaming medians are comparable per turn. Because benchmarks seed prior turns before Criterion starts timing, each measurement isolates a single turn's round-trip cost. The similarity confirms that SSE framing adds no meaningful overhead on the gateway side.
conversation_rehydrationis modestly faster thanresponse_rehydrationat low turn counts (2.06 s vs 2.85 s at turn 1). At higher turn counts they converge. The turn-1 gap reflects the fact that response rehydration always issues a DB lookup for the prior response even on the first measured turn, while conversation rehydration with no prior context skips that step.