feat(server): add Agent Turn Cache for faster coding-agent loops - #614
Conversation
There was a problem hiding this comment.
38 issues found across 38 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="server/src/server/tool_speculation.cpp">
<violation number="1" location="server/src/server/tool_speculation.cpp:127">
P2: The child receives the entire server environment, including unrelated credentials and model configuration. Construct a minimal allowlisted environment instead of forwarding `environ`.</violation>
<violation number="2" location="server/src/server/tool_speculation.cpp:779">
P2: The FD-isolation guarantee is only enforced on glibc >= 2.34. `addclosefrom_np` is guarded by `__GLIBC_PREREQ(2, 34)`, so on glibc 2.17-2.33, musl-based Linux, or any non-glibc POSIX build the child executor inherits every open descriptor from the long-running server (listening sockets, live client connections, model IPC pipes, accelerator descriptors) — directly contradicting the header comment "The executor receives only stdin/stdout/stderr" and the PR's privacy claim. The failure is silent: the code compiles and launches without any FD isolation.</violation>
<violation number="3" location="server/src/server/tool_speculation.cpp:802">
P2: When CPU affinity is configured, `posix_spawn` lets the executor run before `pin_and_verify_child_cpu_affinity` applies the mask. Startup work can consume model CPUs before isolation; spawn through a pre-exec affinity handshake instead.</violation>
</file>
<file name="optimizations/ooo_spec_lucebox5_cpu/benchmark_cpu_tool_speculation.py">
<violation number="1" location="optimizations/ooo_spec_lucebox5_cpu/benchmark_cpu_tool_speculation.py:334">
P1: When model generation consumes the configured timeout, `finish_executor` grants the tool another full timeout because `communicate` starts timing too late. Compute the remaining timeout from `handle["started"]` before waiting.</violation>
<violation number="2" location="optimizations/ooo_spec_lucebox5_cpu/benchmark_cpu_tool_speculation.py:336">
P2: Timeout and miss cancellation kill only the executor leader, so forked descendants can survive and retain the CPU lane. Launch the executor in its own process group and terminate the group on every cleanup path.</violation>
<violation number="3" location="optimizations/ooo_spec_lucebox5_cpu/benchmark_cpu_tool_speculation.py:504">
P2: If `post_model` raises after the private miss executor starts, `run_direct_miss` skips cleanup and the process continues using reserved CPUs. Put the model request and cleanup in a `try/finally`.</violation>
<violation number="4" location="optimizations/ooo_spec_lucebox5_cpu/benchmark_cpu_tool_speculation.py:565">
P2: The private_miss_result_hidden qualification gate is vacuous: run_direct_miss hardcodes 'private_result_exposed': False rather than deriving it, and the private executor is launched by the benchmark itself, so it is never routed through the engine and cannot expose a result. The check in qualify() therefore always passes and adds no real privacy coverage. The native() phase's miss_check ('result' in miss_metadata) is the actual isolation test. Either drop the qualify() gate or compute it from the native path; don't report it as a passed safety gate.</violation>
<violation number="5" location="optimizations/ooo_spec_lucebox5_cpu/benchmark_cpu_tool_speculation.py:1157">
P2: The automatic-Qwen production gate can run without explicitly verifying a qualified profile or disjoint model CPUs. Require the same profile and model/tool affinity checks as the `native` gate before measuring pairs.</violation>
</file>
<file name="server/src/common/qwen3_tool_predictor_ipc.h">
<violation number="1" location="server/src/common/qwen3_tool_predictor_ipc.h:29">
P2: When the configured IPC binary never sends its initial status, `start()` blocks in `BackendIpcProcess::start()` and server startup never reaches `run()`. Add a bounded readiness deadline, terminate the child on expiry, and treat the native predictor as unavailable so the configured HTTP fallback or normal server path can proceed.</violation>
</file>
<file name="server/src/server/native_semantic_tool_predictor.cpp">
<violation number="1" location="server/src/server/native_semantic_tool_predictor.cpp:52">
P2: When prompt construction or tokenization exceeds `config_.timeout_ms`, `predict()` still blocks before its only deadline check, so before-model requests exceed the configured timeout and HTTP fallback loses its remaining budget. Make preprocessing deadline-aware or otherwise bound it before entering the predictor lane.</violation>
</file>
<file name="server/src/server/server_main.cpp">
<violation number="1" location="server/src/server/server_main.cpp:626">
P2: When `--tool-hint-native-gpu` is malformed, `std::atoi` silently selects GPU 0. Parse this option strictly and reject non-numeric or out-of-range values before starting the native predictor.</violation>
<violation number="2" location="server/src/server/server_main.cpp:673">
P2: When `--tool-hint-execution-confidence` is malformed, `std::atof` converts it to `0` and startup accepts it. Parse the argument strictly and reject trailing or non-numeric input instead of silently changing the admission policy.</violation>
</file>
<file name="server/src/common/qwen3_tool_predictor_ipc_daemon.cpp">
<violation number="1" location="server/src/common/qwen3_tool_predictor_ipc_daemon.cpp:90">
P2: For predictions of at least three tokens, this lane produces later logits from a skipped Qwen3 KV-cache position. Fix `Qwen3Backend::do_decode`'s position update before routing predictor requests through this backend.</violation>
</file>
<file name="optimizations/ooo_spec_lucebox5_cpu/results/trace-compiled-engine-qwen-production-6pairs-compact.json">
<violation number="1" location="optimizations/ooo_spec_lucebox5_cpu/results/trace-compiled-engine-qwen-production-6pairs-compact.json:653">
P2: The trace-compilation provenance recorded in this results artifact cannot be reproduced from the repo. `pattern.training_report` and `pattern.workflow_registry` point to absolute `/home/lucebox5/tool-spec-cpu-20260813/results/...` paths to files that are not committed (`multiturn-cached-wordref-production-6tasks.json`, `trace-workflow-registry.json`), while the README's reproduce command feeds `results/trace-compiled-training-traces.json`, whose sha256 is `ba73d1aef7c6...`, not the recorded `training_report_sha256` `2475697d...`. The recorded hashes therefore match nothing in the repo, so the 6/6 exact-hit and gate evidence cannot be independently regenerated or verified. Record the canonical source path (relative to the repo), commit the workflow registry that defines the `execute_customer_workflows` macro allowlist/canonicalization, and make the recorded sha256 correspond to the committed input.</violation>
</file>
<file name="server/src/server/http_server.cpp">
<violation number="1" location="server/src/server/http_server.cpp:2467">
P2: `launch_semantic_tool_prediction` shares one `NativeSemanticToolPredictor` instance (`native_semantic_predictor_`) across every request and invokes `native->predict(payload, tools)` from a fresh `std::async` thread per request. `predict` is non-const and the object owns a `Qwen3ToolPredictorIpcClient ipc_` with no visible synchronization; concurrent tool-using requests can therefore call `predict` concurrently on the same IPC client. If that client is not internally thread-safe, this is a data race. Additionally, spawning two `std::async(std::launch::async)` threads per tool request is unbounded under concurrency.</violation>
<violation number="2" location="server/src/server/http_server.cpp:2664">
P2: When upstream proxy mode is enabled with a before-model native predictor, proxied requests wait for an unused prediction before forwarding. Skip semantic prediction for forwarded requests, or explicitly cancel it before the early `forward_upstream()` return.</violation>
<violation number="3" location="server/src/server/http_server.cpp:4572">
P2: In overlap mode (the default for HTTP predictors, and the experimental native overlap schedule), `launch_semantic_tool_prediction` starts the predictor on a `std::async` worker before the request is enqueued, then `finish_tool_speculation` calls `req.automatic_tool_speculation.get()` / `req.semantic_tool_prediction.get()`. If generation finishes before the prediction (e.g. the model returns a plain answer with no tool call, or a short response), `.get()` blocks the worker thread for the remaining predictor time (up to `--tool-hint-timeout-ms`, default 2000 ms). This delays that response and, because the worker thread is shared, stalls other queued jobs. The overlap schedule is supposed to run generation and prediction concurrently, so blocking on the prediction here partially negates it.</violation>
</file>
<file name="optimizations/ooo_spec_lucebox5_cpu/cpu_sparse_tool_executor.cpp">
<violation number="1" location="optimizations/ooo_spec_lucebox5_cpu/cpu_sparse_tool_executor.cpp:141">
P1: When an allowlisted call contains these extra fields, the executor can allocate several GiB despite the small public `iterations` schema. Reject unknown arguments and keep the qualified sparse shape fixed, rather than letting model-supplied JSON select dimensions and worker count.</violation>
<violation number="2" location="optimizations/ooo_spec_lucebox5_cpu/cpu_sparse_tool_executor.cpp:179">
P1: When this executor is configured without a CPU lane, the empty expected mask bypasses isolation and the workload competes with model decoding. Require a non-empty expected affinity and fail closed when it is absent.</violation>
</file>
<file name="optimizations/ooo_spec_lucebox5_cpu/run_native_cpu_server_lucebox5.sh">
<violation number="1" location="optimizations/ooo_spec_lucebox5_cpu/run_native_cpu_server_lucebox5.sh:43">
P2: When `CANDIDATE_BUILD` is overridden for this launcher, the qualified launcher clears that ambient override before the wrapper runs, so the wrapper may execute its symlink/default build instead of the validated build. Pass build selection through a durable launcher-supported mechanism, or preflight the wrapper’s actual candidate.</violation>
<violation number="2" location="optimizations/ooo_spec_lucebox5_cpu/run_native_cpu_server_lucebox5.sh:49">
P2: When `PREDICTOR_TIMEOUT_MS` is overridden, this launcher drops the override before the qualified launcher starts the wrapper, so prediction continues using the 2000 ms default. Propagate the timeout setting here so it reaches `--tool-hint-timeout-ms`.</violation>
</file>
<file name="optimizations/ooo_spec_lucebox5_cpu/bfcl_replay_tool_executor.py">
<violation number="1" location="optimizations/ooo_spec_lucebox5_cpu/bfcl_replay_tool_executor.py:53">
P2: When `cpu_affinity` is explicitly null or another falsy non-list, `or []` skips the affinity check and can report an unisolated executor as successful. Use a default-only lookup so malformed supplied values reach the type validation.</violation>
</file>
<file name="server/src/server/chat_template.cpp">
<violation number="1" location="server/src/server/chat_template.cpp:390">
P2: The new `tool_call_required` parameter is documented (header) as strengthening the template for OpenAI `tool_choice="required"` and forced-function requests, and both call sites in http_server.cpp pass `tool_choice_requires_call(req.tool_choice)`. However it is only honored in the DEEPSEEK4 case (line 390); the QWEN3, LAGUNA, and GEMMA4 branches ignore it entirely. For QWEN3 — the default/primary architecture for the workflows targeted by this PR — a `tool_choice="required"` or forced-function request produces the same prompt as a normal request, and its preamble even says "If there is no function call available, answer the question like normal", so the model is not actually forced to emit a call. Wire the flag into the other tool-capable branches (at minimum QWEN3, and LAGUNA's non-thinking/thinking tool-block) so the required-call contract holds across architectures.</violation>
</file>
<file name="server/src/qwen3/qwen3_loader.cpp">
<violation number="1" location="server/src/qwen3/qwen3_loader.cpp:150">
P1: When the compact GGUF has Q8_0 projection weights but BF16 embeddings or output weights, this assignment applies Q8_0 to every 2-D tensor and the drafter fails to load. Allocate each tensor using its source storage type or explicitly convert the non-Q8 tensors instead of deriving one global type from `blk.0.attn_q.weight`.</violation>
<violation number="2" location="server/src/qwen3/qwen3_loader.cpp:151">
P2: When an F16 GGUF is loaded on HIP, this branch accepts it but leaves `out.weight_type` as BF16, so the loader rejects the F16→BF16 copies later. Set `out.weight_type` to F16 for an F16 source or add an explicit F16→BF16 conversion.</violation>
</file>
<file name="optimizations/ooo_spec_lucebox5_cpu/trace_compiled_tool_executor.py">
<violation number="1" location="optimizations/ooo_spec_lucebox5_cpu/trace_compiled_tool_executor.py:100">
P2: When a leaf executor returns a non-object envelope, `.get("ok")` raises an uncaught `AttributeError` and the child emits no protocol response. Check the envelope type before calling `.get()`.</violation>
</file>
<file name="server/src/common/qwen3_tool_predictor_ipc.cpp">
<violation number="1" location="server/src/common/qwen3_tool_predictor_ipc.cpp:169">
P1: When native prediction starts, the daemon inherits every non-CLOEXEC descriptor held by the server and target backend. That allows the predictor process to retain or inspect model, tool, or server IPC descriptors, so this path does not provide the promised descriptor privacy. Launch the predictor with a close-on-exec descriptor policy or an explicit inherited-descriptor allowlist before enabling this lane.</violation>
</file>
<file name="server/src/server/semantic_tool_hint.cpp">
<violation number="1" location="server/src/server/semantic_tool_hint.cpp:109">
P3: `parse_content_call` scans every byte of predictor `content`, and for each position holding `{` it runs a full `json::parse(content.begin()+offset, content.end(), nullptr, false)` over the rest of the string. This is O(n²) on unpredictable sidecar output and can be triggered on arbitrary predictor text (HTTP fallback path in `parse_semantic_tool_prediction`). It also accepts the first embedded JSON object as the tool call with no envelope/delimiter requirement, so a response whose real call is not the first object (or that has trailing prose after the object) silently falls through to the authoritative call, defeating the speculative fast path. The per-offset full-parse is the main cost; tail after the object also makes most scan positions fail.</violation>
<violation number="2" location="server/src/server/semantic_tool_hint.cpp:243">
P2: When a sidecar returns multiple `tool_calls` plus content containing one JSON call, this condition skips the array and accepts the content. Reject non-single `tool_calls` responses instead of selecting one representation, so malformed predictions cannot start speculation.</violation>
<violation number="3" location="server/src/server/semantic_tool_hint.cpp:377">
P2: When `tool_choice` is `"none"`, the native prompt still asks for functions, so it can launch an allowlisted speculative call that authoritative decoding will always cancel. Return an opt-out/error prompt for `none` before scheduling prediction.</violation>
</file>
<file name="optimizations/ooo_spec_lucebox5_cpu/benchmark_trace_compiled_workflows.py">
<violation number="1" location="optimizations/ooo_spec_lucebox5_cpu/benchmark_trace_compiled_workflows.py:617">
P1: When `--tool-cpus` differs from the server configuration, the benchmark compares different CPU lanes and can run authoritative tools on model CPUs. Require `tool_speculation.tool_cpu_affinity == args.tool_cpus` before measuring.</violation>
<violation number="2" location="optimizations/ooo_spec_lucebox5_cpu/benchmark_trace_compiled_workflows.py:977">
P1: The final-answer gate never consumes the generated tool result because `post_final()` is context-free and receives the expected receipt directly. Send the accumulated assistant/tool conversation to the final turn and validate that response instead.</violation>
<violation number="3" location="optimizations/ooo_spec_lucebox5_cpu/benchmark_trace_compiled_workflows.py:1453">
P2: When either trace path is customized, the benchmark and executor read different files because the executor paths are never propagated. Require matching executor environment at launch or reject non-default paths before running.</violation>
</file>
<file name="server/src/common/backend_ipc.cpp">
<violation number="1" location="server/src/common/backend_ipc.cpp:452">
P2: The new ownership/mode and lstat checks are applied in the shared `BackendIpcProcess::init_work_dir`, so they tighten every backend-IPC mode (remote DFlash draft, PFlash compress, target shards, moe-expert-compute), not just the new Qwen3 predictor. Any existing deployment that passes a user-supplied `--remote-*-work-dir` that is a symlink (previously accepted via `stat` + `S_ISDIR`) or is owned by a different euid / has mode other than exactly 0700 (e.g. a shared scratch dir) now hard-fails `BackendIpcProcess::start` at startup. If this hardening is intended only for the predictor lane, scope it to that caller or document the cross-mode behavior change for existing multi-GPU deployments that already configure `work_dir`.</violation>
</file>
<file name="optimizations/ooo_spec_lucebox5_cpu/dflash_server_native_tool_predictor_wrapper.sh">
<violation number="1" location="optimizations/ooo_spec_lucebox5_cpu/dflash_server_native_tool_predictor_wrapper.sh:29">
P2: When an operator sets the valid cache-disable value `PREFIX_CACHE_SLOTS_OVERRIDE=0`, this wrapper rejects it before starting the server. Accept zero in the override regex so the wrapper can pass `--prefix-cache-slots 0`.</violation>
<violation number="2" location="optimizations/ooo_spec_lucebox5_cpu/dflash_server_native_tool_predictor_wrapper.sh:45">
P2: When a binary override exists but is not an executable regular file, this preflight accepts it and startup fails later with a generic exec/IPC error. Check the two binaries with `-f && -x` and the model with `-f` before launching.</violation>
</file>
<file name="optimizations/ooo_spec_lucebox5_cpu/README.md">
<violation number="1" location="optimizations/ooo_spec_lucebox5_cpu/README.md:88">
P3: The override advice here conflicts with the next sentence. This paragraph tells operators to override placement with `PREDICTOR_MODEL`/`PREDICTOR_GPU`/`PREDICTOR_MAX_CTX`/`PREDICTOR_MAX_TOKENS`/`PREDICTOR_TIMEOUT_MS` env vars, but immediately notes the qualified launcher clears ambient variables (which is why the `candidate-build` symlink is needed as the durable override). If that launcher clears ambient env, those `PREDICTOR_*` overrides are discarded when launching via `run_native_cpu_server_lucebox5.sh`, so the documented override silently does not take effect through the qualified path. Clarify that the `PREDICTOR_*` overrides apply when launching the wrapper directly, or state how to pass them so they survive the launcher.</violation>
</file>
<file name="server/test/smoke_qwen3_tool_predictor_ipc.cpp">
<violation number="1" location="server/test/smoke_qwen3_tool_predictor_ipc.cpp:190">
P3: The batch gate returns success when names match but does not require exact argument matches, even though exact_matches is already tracked and reported. With the current baseline at 9/12 exact args, this automation would still exit 0 on an argument-canonicalization regression, which is exactly the safety property this PR emphasizes. Gate on exact_matches == cases.size() (or assert the expected baseline explicitly) so the check protects the exact-match commit guarantee.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 38 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 38 files (changes from recent commits).
Not reviewed (too large): optimizations/ooo_spec_lucebox5_cpu/results/multiturn-cached-wordref-production-6tasks.json (~5,595 lines) - if these are generated or fixture files, add them to ignored paths to exclude them from future reviews.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 16 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 16 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
…, lane hardening Measured on real tools the pre-generation predictor alone does not pay back (3/17 hits on live public APIs, 0/75 on terminal-bench; ~0.5 s serial cost per turn). Keep every existing lane and add model-agnostic mechanisms that do: - --early-dispatch: launch every allowlisted tool call through the isolated executor lane the moment its call block closes in the token stream (<function_call>, <tool_call>, <function=> wrappers; nested blocks once). N calls per response, each committed on exact canonical match against the parsed authoritative calls; results returned as dflash_early_dispatch (JSON body) / dflash_tool_speculation.early_dispatch (SSE). Authoritative launches need no measured resource profile; --tool-spec-profile is now optional and only gates predictor-based (confidence < 1) speculation. - --end-turn-snapshot: after tool-enabled generations snapshot prompt+output into the inline prefix cache so the next turn restores the conversation and prefills one delta. DeepSeek-V4 continuation snapshots store no final logits / drafter window (DFLASH_DS4_SNAPSHOT_STALE_LOGITS=1, set by the flag); the server never restores them at full prompt length. - Executor result budget counts from the commit (authoritative call known), with a 20x absolute lifetime cap, so slow target turns cannot expire a finished result. - Native predictor daemon relaunches lazily (30 s cooldown) after a timeout or transport failure instead of staying off for the server lifetime. - Wrapper toggles EARLY_DISPATCH / END_TURN_SNAPSHOT / PREDICTOR; README with the measured numbers; real-API and terminal-bench harnesses + artifacts. Unit suite: 412/412 (adds closed-block scanner and commit-deadline tests).
…esolved server-side) Calls that reference earlier calls' results are held back from the stream, substituted once their inputs resolve and executed wave by wave on the same isolated lane at resolve time; entries report dependencies, resolved arguments, wave and result. Placeholder paths must start with a letter so money-like text is never treated as a reference. Unit suite 413/413.
There was a problem hiding this comment.
All reported issues were addressed across 32 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
…ue core + prefetch-prefill Remove the pre-generation Qwen3 predictor lane (native IPC daemon, semantic hint parser, HTTP sidecar, its tokenizer/loader/ipc plumbing and tests) and the replay-benchmark bulk: measured on real tools it hit 3/17 calls on live APIs and 0/75 on terminal-bench while costing ~0.5 s of serial predictor time per turn. Client-supplied predictions via the tool_speculation extension keep working; --tool-spec-profile now only gates those. Add --prefetch-prefill: after a tool turn whose early-dispatched results all committed, the next request is fully determined, so the server renders it, restores the end-of-turn snapshot, prefills the delta and caches the KV before the client's next request arrives. Each early-dispatch hit carries tool_message_content, the canonical tool text a client echoes for a guaranteed hit; every failure path falls back silently. optimizations/ now ships one small real-API harness (realapi/) with compact artifacts instead of 20k lines of replay benchmarks. Unit suite 386/386.
…ll prefetched off the critical path
There was a problem hiding this comment.
All reported issues were addressed across 53 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
- Prefetch-prefill yields to real traffic: skipped entirely when a request is already queued, and cancelled at the next prefill chunk boundary when one arrives mid-prefetch (logged as 'yielded to a queued request'). A waiting client never pays for speculative prefill. - Server-wide executor saturation cap (--tool-spec-max-executors, default 16): attempts beyond the cap are deferred with reason executor_saturated instead of spawning unbounded children under concurrent load; slot accounting is exact across the reap, terminate and destructor paths. - Unit suite 387/387 (adds the saturation-cap test). Verified live: paired REST bench with a competing client every 12 s on the same server — no crashes, competitor 10/10 correct, bench answers identical.
There was a problem hiding this comment.
All reported issues were addressed across 6 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 24 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 24 files
Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
Re-trigger cubic
There was a problem hiding this comment.
2 issues found across 54 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="server/src/server/tool_parser.h">
<violation number="1" location="server/src/server/tool_parser.h:4">
P3: The new header prose announces DeepSeek `<?DSML?tool>`/`<?DSML?tool_call>` wrapper support, but the enumerated list of patterns directly below it (items 1-11) never lists the DSML form. Add a DSML entry to the list so the claimed support is discoverable where the other patterns are documented.</violation>
</file>
<file name="server/src/server/server_main.cpp">
<violation number="1" location="server/src/server/server_main.cpp:1032">
P3: When agent-turn caching starts with disabled inline cache, this diagnostic tells operators to remove a nonexistent `--no-prefix-cache` flag. Name the actual `--prefix-cache-slots 0` setting so the startup failure is actionable.</violation>
</file>
Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| // | ||
| // Supports 11 detection patterns: | ||
| // Supports the common XML/JSON compatibility patterns plus DeepSeek's native | ||
| // `<?DSML?tool>` / `<?DSML?tool_call>` JSON wrappers: |
There was a problem hiding this comment.
P3: The new header prose announces DeepSeek <?DSML?tool>/<?DSML?tool_call> wrapper support, but the enumerated list of patterns directly below it (items 1-11) never lists the DSML form. Add a DSML entry to the list so the claimed support is discoverable where the other patterns are documented.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/src/server/tool_parser.h, line 4:
<comment>The new header prose announces DeepSeek `<?DSML?tool>`/`<?DSML?tool_call>` wrapper support, but the enumerated list of patterns directly below it (items 1-11) never lists the DSML form. Add a DSML entry to the list so the claimed support is discoverable where the other patterns are documented.</comment>
<file context>
@@ -1,6 +1,7 @@
//
-// Supports 11 detection patterns:
+// Supports the common XML/JSON compatibility patterns plus DeepSeek's native
+// `<?DSML?tool>` / `<?DSML?tool_call>` JSON wrappers:
// 1. <tool_call><function=name>...</function></tool_call> (Qwen XML)
// 2. <function=name>...</function> (bare function XML)
</file context>
| "cache; remove --no-prefix-cache/--paged-attention or configure " | ||
| "at least one prefix-cache slot\n"); |
There was a problem hiding this comment.
P3: When agent-turn caching starts with disabled inline cache, this diagnostic tells operators to remove a nonexistent --no-prefix-cache flag. Name the actual --prefix-cache-slots 0 setting so the startup failure is actionable.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/src/server/server_main.cpp, line 1032:
<comment>When agent-turn caching starts with disabled inline cache, this diagnostic tells operators to remove a nonexistent `--no-prefix-cache` flag. Name the actual `--prefix-cache-slots 0` setting so the startup failure is actionable.</comment>
<file context>
@@ -1012,6 +1026,13 @@ int main(int argc, char ** argv) {
+ if (sconfig.agent_turn_cache && sconfig.prefix_cache_cap <= 0) {
+ std::fprintf(stderr,
+ "[server] --agent-turn-cache requires an enabled inline prefix "
+ "cache; remove --no-prefix-cache/--paged-attention or configure "
+ "at least one prefix-cache slot\n");
+ return 2;
</file context>
| "cache; remove --no-prefix-cache/--paged-attention or configure " | |
| "at least one prefix-cache slot\n"); | |
| "cache; use --prefix-cache-slots N>0 and avoid --paged-attention\n"); |
There was a problem hiding this comment.
All reported issues were addressed across 5 files (changes from recent commits).
Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
6 issues found across 39 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="server/src/server/sse_emitter.cpp">
<violation number="1" location="server/src/server/sse_emitter.cpp:588">
P2: When a stop sequence precedes an incomplete UTF-8 suffix in the same token, `emit_token` leaves that suffix in `utf8_tail_` after setting `stop_hit_`. `emit_finish` then appends U+FFFD after the stop frame; skip the tail when `stop_hit_` is true.</violation>
</file>
<file name="server/src/server/server_main.cpp">
<violation number="1" location="server/src/server/server_main.cpp:423">
P2: When `--agent-turn-cache` is combined with `--prefill-cache-slots`, the separate exact-match full-prompt cache remains enabled. Disable `prefill_cache_cap` when enabling agent-turn caching so the agent path uses only the inline prefix cache.</violation>
</file>
<file name="server/src/server/http_server.cpp">
<violation number="1" location="server/src/server/http_server.cpp:976">
P1: Anthropic tool loops lose their assistant call and tool result during normalization. Restore the Anthropic `tool_use`/`tool_result` handling before the generic content-array fallback.</violation>
<violation number="2" location="server/src/server/http_server.cpp:3320">
P2: `agent_turn_cache: false` is ignored on enabled servers, so clients cannot disable caching per request as documented. Parse and validate the request override, then gate staging and saving on its effective value.</violation>
<violation number="3" location="server/src/server/http_server.cpp:3323">
P1: With `--agent-turn-cache`, Anthropic coding-agent requests never use the Agent Turn Cache because this predicate excludes `ANTHROPIC`. Include Anthropic after restoring its canonical transcript handling, or explicitly reject/document that unsupported format.</violation>
</file>
<file name="server/docs/API.md">
<violation number="1" location="server/docs/API.md:34">
P3: This section scopes `usage.timings` to the Responses API only, and the same delta removed the `usage.timings.*` rows from the Chat Completions Response Fields table. The code emits these fields for all three endpoints (see the comment in http_server.cpp: "usage.timings (OpenAI Chat usage chunk, Anthropic message_delta usage, Responses response.completed usage)"), so Chat Completions and Anthropic users now have no documented `usage.timings` fields. Widen the wording to all endpoints (or keep the rows in the Chat tab) so the doc matches the emitted response shape.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| const bool stage_agent_prompt = config_.agent_turn_cache && | ||
| !req.tools.empty() && !prepared.compressed && !ppp_rewrote && | ||
| !effective_prompt.empty() && | ||
| (req.format == ApiFormat::OPENAI_CHAT || |
There was a problem hiding this comment.
P1: With --agent-turn-cache, Anthropic coding-agent requests never use the Agent Turn Cache because this predicate excludes ANTHROPIC. Include Anthropic after restoring its canonical transcript handling, or explicitly reject/document that unsupported format.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/src/server/http_server.cpp, line 3323:
<comment>With `--agent-turn-cache`, Anthropic coding-agent requests never use the Agent Turn Cache because this predicate excludes `ANTHROPIC`. Include Anthropic after restoring its canonical transcript handling, or explicitly reject/document that unsupported format.</comment>
<file context>
@@ -3605,16 +3314,14 @@ HttpServer::GenerationCacheState HttpServer::prepare_generation_cache(
!req.tools.empty() && !prepared.compressed && !ppp_rewrote &&
!effective_prompt.empty() &&
- cache.cache_slot != kAgentTurnStagingSlot;
+ (req.format == ApiFormat::OPENAI_CHAT ||
+ req.format == ApiFormat::RESPONSES);
</file context>
| @@ -252,6 +252,32 @@ bool should_clamp_flowkv_disk_cache( | |||
| return flowkv && policy.compress; | |||
There was a problem hiding this comment.
P1: Anthropic tool loops lose their assistant call and tool result during normalization. Restore the Anthropic tool_use/tool_result handling before the generic content-array fallback.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/src/server/http_server.cpp, line 976:
<comment>Anthropic tool loops lose their assistant call and tool result during normalization. Restore the Anthropic `tool_use`/`tool_result` handling before the generic content-array fallback.</comment>
<file context>
@@ -1145,39 +973,11 @@ std::vector<ChatMessage> normalize_chat_messages(
- responses_call_group_exact = exact_raw;
- responses_call_group_raw = exact_raw
- ? raw : std::string();
+ if (raw.empty()) {
+ raw = render_tool_call_xml(m.value("name", ""),
+ parse_responses_arguments(m));
</file context>
|
|
||
| // A tail still pending at end-of-stream is a genuinely truncated | ||
| // codepoint; sanitize it into the window so nothing is silently lost. | ||
| if (!utf8_tail_.empty()) { |
There was a problem hiding this comment.
P2: When a stop sequence precedes an incomplete UTF-8 suffix in the same token, emit_token leaves that suffix in utf8_tail_ after setting stop_hit_. emit_finish then appends U+FFFD after the stop frame; skip the tail when stop_hit_ is true.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/src/server/sse_emitter.cpp, line 588:
<comment>When a stop sequence precedes an incomplete UTF-8 suffix in the same token, `emit_token` leaves that suffix in `utf8_tail_` after setting `stop_hit_`. `emit_finish` then appends U+FFFD after the stop frame; skip the tail when `stop_hit_` is true.</comment>
<file context>
@@ -559,6 +583,15 @@ std::vector<std::string> SseEmitter::emit_finish(int completion_tokens,
+ // A tail still pending at end-of-stream is a genuinely truncated
+ // codepoint; sanitize it into the window so nothing is silently lost.
+ if (!utf8_tail_.empty()) {
+ const std::string flushed = utf8_sanitize(utf8_tail_);
+ utf8_tail_.clear();
</file context>
| if (!utf8_tail_.empty()) { | |
| if (!stop_hit_ && !utf8_tail_.empty()) { |
| } else if (std::strcmp(argv[i], "--prefix-cache-slots") == 0 && i + 1 < argc) { | ||
| sconfig.prefix_cache_cap = std::atoi(argv[++i]); | ||
| } else if (std::strcmp(argv[i], "--agent-turn-cache") == 0) { | ||
| sconfig.agent_turn_cache = true; |
There was a problem hiding this comment.
P2: When --agent-turn-cache is combined with --prefill-cache-slots, the separate exact-match full-prompt cache remains enabled. Disable prefill_cache_cap when enabling agent-turn caching so the agent path uses only the inline prefix cache.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/src/server/server_main.cpp, line 423:
<comment>When `--agent-turn-cache` is combined with `--prefill-cache-slots`, the separate exact-match full-prompt cache remains enabled. Disable `prefill_cache_cap` when enabling agent-turn caching so the agent path uses only the inline prefix cache.</comment>
<file context>
@@ -469,6 +419,8 @@ int main(int argc, char ** argv) {
} else if (std::strcmp(argv[i], "--prefix-cache-slots") == 0 && i + 1 < argc) {
sconfig.prefix_cache_cap = std::atoi(argv[++i]);
+ } else if (std::strcmp(argv[i], "--agent-turn-cache") == 0) {
+ sconfig.agent_turn_cache = true;
} else if (std::strcmp(argv[i], "--prefill-cache-slots") == 0 && i + 1 < argc) {
sconfig.prefill_cache_cap = std::atoi(argv[++i]);
</file context>
| // A generation can save one snapshot during prefill. For supported tool | ||
| // APIs, reserve it at the exact prompt end so the generated tool call can | ||
| // later deepen the existing prefix cache without replaying the prompt. | ||
| const bool stage_agent_prompt = config_.agent_turn_cache && |
There was a problem hiding this comment.
P2: agent_turn_cache: false is ignored on enabled servers, so clients cannot disable caching per request as documented. Parse and validate the request override, then gate staging and saving on its effective value.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/src/server/http_server.cpp, line 3320:
<comment>`agent_turn_cache: false` is ignored on enabled servers, so clients cannot disable caching per request as documented. Parse and validate the request override, then gate staging and saving on its effective value.</comment>
<file context>
@@ -3605,16 +3314,14 @@ HttpServer::GenerationCacheState HttpServer::prepare_generation_cache(
+ // A generation can save one snapshot during prefill. For supported tool
+ // APIs, reserve it at the exact prompt end so the generated tool call can
+ // later deepen the existing prefix cache without replaying the prompt.
+ const bool stage_agent_prompt = config_.agent_turn_cache &&
!req.tools.empty() && !prepared.compressed && !ppp_rewrote &&
!effective_prompt.empty() &&
</file context>
| templates whose completed turn is not an exact token extension safely fall | ||
| back to ordinary prefix caching. | ||
|
|
||
| Responses expose the measured result under `usage.timings`: |
There was a problem hiding this comment.
P3: This section scopes usage.timings to the Responses API only, and the same delta removed the usage.timings.* rows from the Chat Completions Response Fields table. The code emits these fields for all three endpoints (see the comment in http_server.cpp: "usage.timings (OpenAI Chat usage chunk, Anthropic message_delta usage, Responses response.completed usage)"), so Chat Completions and Anthropic users now have no documented usage.timings fields. Widen the wording to all endpoints (or keep the rows in the Chat tab) so the doc matches the emitted response shape.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/docs/API.md, line 34:
<comment>This section scopes `usage.timings` to the Responses API only, and the same delta removed the `usage.timings.*` rows from the Chat Completions Response Fields table. The code emits these fields for all three endpoints (see the comment in http_server.cpp: "usage.timings (OpenAI Chat usage chunk, Anthropic message_delta usage, Responses response.completed usage)"), so Chat Completions and Anthropic users now have no documented `usage.timings` fields. Widen the wording to all endpoints (or keep the rows in the Chat tab) so the doc matches the emitted response shape.</comment>
<file context>
@@ -18,6 +18,27 @@ backend (deepseek4, qwen35, qwen3, gemma4, laguna).
+templates whose completed turn is not an exact token extension safely fall
+back to ordinary prefix caching.
+
+Responses expose the measured result under `usage.timings`:
+
+- `agent_turn_cache_hit`: the restored prefix includes a generated agent turn.
</file context>
| Responses expose the measured result under `usage.timings`: | |
| The server exposes the measured result in `usage.timings` for every endpoint (OpenAI Chat Completions, Anthropic Messages, and Responses): |
There was a problem hiding this comment.
1 issue found across 6 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="server/src/server/prefix_cache.cpp">
<violation number="1">
P2: When `--prefix-cache-slots 64` is used with the disk cache enabled, the inline cache can allocate slot 63, which `http_server.cpp` concurrently uses as `kDiskStagingSlot`; disk loads or continued saves then replace the inline snapshot and leave its metadata pointing at unrelated KV state. Cap the inline pool below the reserved staging slot.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Outcome
This PR adds Agent Turn Cache, a generation-aware extension of Lucebox's existing prefix cache for coding-agent tool loops.
Main already caches stable system prompts, tool schemas, prior input turns, exact repeated prompts, disk snapshots, and FlowKV-compressed histories. The remaining gap is the assistant tool-call turn the model has just generated: the next stateless request sends that turn back before the tool result, but ordinary prefix caching has not saved it yet.
Agent Turn Cache deepens an existing compatible snapshot through that completed assistant turn while the client executes the tool. On the next request, Lucebox can restore through the assistant turn and prefill only the returned tool result plus any new suffix.
This is an additive latency optimization. It does not replace the cache on main, predict requests, execute tools, change agent permissions, or eliminate prefill compute.
How it works
Replay uses the engine's existing prefix snapshots; it does not reserve another staging slot. This update also caps in-memory cache entries at 63 because backend slot 63 is already reserved for disk-cache staging.
Operator and client contract
Enable it with:
The existing inline prefix cache must remain enabled. Request bodies do not change. The optimization currently applies to OpenAI Chat Completions and Responses tool turns.
Paged attention and
--max-concurrencydo not yet support shared prefix blocks and cannot be combined with Agent Turn Cache. Unsupported formats, compressed or token-rewritten prompts, missing compatible checkpoints, parse failures, cancellations, and disconnects safely use the ordinary path.usage.timingsreports backend-confirmed behavior:agent_turn_cache_hit: the restored snapshot includes a generated agent turn.cached_prefix_tokens: prompt tokens supplied by restored KV state.prefilled_tokens: prompt tokens evaluated for this request.Performance boundary
Canonical replay evaluates the generated assistant-turn tail once after the response. The optimization helps follow-up latency only when enough of that replay overlaps tool execution. Instant tools may see no benefit, large tool results remain new input, and speculative replay can add background work when no follow-up arrives.
Scope
The earlier request predictor, speculative tool executor, benchmark framework, fixtures, and raw result dumps are gone. Relative to
main, this PR is 18 files, 419 additions, and 47 deletions, including tests and documentation.Verification
read_fileforprefix_cache.cpp, consumed the returned source summary, then calledread_fileforhttp_server.cpp.That matched smoke pair shows 69% fewer newly-prefilled tokens and 70% lower follow-up prefill time. It validates the path; it is not a throughput result or a publication benchmark.
A separate real-file scheduler probe covered both safety branches. With zero client delay, replay was cancelled,
agent_turn_cache_hitstayed false, and the follow-up restored the ordinary 512-token prefix. On a warmed delayed run, the follow-up restored a backend-confirmed 812-token Agent Turn Cache prefix. That second request overlapped the replay completion boundary, so this is behavioral coverage rather than a clean crossover-time claim.