feat(server): lossless CPU-isolated tool-call prediction - #614
Open
davide221 wants to merge 13 commits into
Open
feat(server): lossless CPU-isolated tool-call prediction#614davide221 wants to merge 13 commits into
davide221 wants to merge 13 commits into
Conversation
davide221
changed the title
10 tasks
davide221
marked this pull request as ready for review
August 18, 2026 11:51
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
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
| predictor.max_tokens); | ||
| const json tools = req.tools; | ||
| const auto native = native_semantic_predictor_; | ||
| req.semantic_tool_prediction = std::async( |
Contributor
There was a problem hiding this comment.
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.
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 2467:
<comment>`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.</comment>
<file context>
@@ -1969,6 +2434,136 @@ void HttpServer::log_parsed_request(const ParsedRequest & req) const {
+ predictor.max_tokens);
+ const json tools = req.tools;
+ const auto native = native_semantic_predictor_;
+ req.semantic_tool_prediction = std::async(
+ std::launch::async,
+ [predictor, payload, tools, native]() {
</file context>
Contributor
There was a problem hiding this comment.
7 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/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/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="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: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="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/server/tool_speculation.cpp">
<violation number="1" 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>
</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>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| def finish_executor(handle: dict[str, Any], timeout: float) -> dict[str, Any]: | ||
| process: subprocess.Popen[str] = handle["process"] | ||
| try: | ||
| stdout, stderr = process.communicate(timeout=timeout) |
Contributor
There was a problem hiding this comment.
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.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At optimizations/ooo_spec_lucebox5_cpu/benchmark_cpu_tool_speculation.py, line 334:
<comment>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.</comment>
<file context>
@@ -0,0 +1,1409 @@
+def finish_executor(handle: dict[str, Any], timeout: float) -> dict[str, Any]:
+ process: subprocess.Popen[str] = handle["process"]
+ try:
+ stdout, stderr = process.communicate(timeout=timeout)
+ except subprocess.TimeoutExpired:
+ process.kill()
</file context>
| expected_affinity.end()); | ||
| } | ||
| const std::vector<int> affinity = observed_affinity(); | ||
| if (!expected_affinity.empty() && affinity != expected_affinity) { |
Contributor
There was a problem hiding this comment.
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.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At optimizations/ooo_spec_lucebox5_cpu/cpu_sparse_tool_executor.cpp, line 179:
<comment>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.</comment>
<file context>
@@ -0,0 +1,259 @@
+ expected_affinity.end());
+ }
+ const std::vector<int> affinity = observed_affinity();
+ if (!expected_affinity.empty() && affinity != expected_affinity) {
+ throw std::runtime_error("observed CPU affinity does not match request");
+ }
</file context>
Suggested change
| if (!expected_affinity.empty() && affinity != expected_affinity) { | |
| if (expected_affinity.empty() || affinity != expected_affinity) { |
| if (!arguments.is_object()) { | ||
| throw std::runtime_error("arguments must be an object"); | ||
| } | ||
| const int rows = arguments.contains("rows") |
Contributor
There was a problem hiding this comment.
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.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At optimizations/ooo_spec_lucebox5_cpu/cpu_sparse_tool_executor.cpp, line 141:
<comment>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.</comment>
<file context>
@@ -0,0 +1,259 @@
+ if (!arguments.is_object()) {
+ throw std::runtime_error("arguments must be an object");
+ }
+ const int rows = arguments.contains("rows")
+ ? integer_argument(arguments, "rows", 64, 1 << 20) : kRows;
+ const int nonzeros = arguments.contains("nonzeros_per_row")
</file context>
| } | ||
| } | ||
| if (wtype == GGML_TYPE_Q8_0) { | ||
| out.weight_type = GGML_TYPE_Q8_0; |
Contributor
There was a problem hiding this comment.
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.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/src/qwen3/qwen3_loader.cpp, line 150:
<comment>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`.</comment>
<file context>
@@ -134,16 +134,28 @@ bool load_qwen3_drafter_model(const std::string & path,
}
}
+ if (wtype == GGML_TYPE_Q8_0) {
+ out.weight_type = GGML_TYPE_Q8_0;
+ } else if (wtype != GGML_TYPE_BF16 && wtype != GGML_TYPE_F16) {
+ set_last_error(std::string("unsupported Qwen3-0.6B weight type: ") +
</file context>
| launch.work_dir = work_dir; | ||
| launch.args.push_back("--target-gpu=" + std::to_string(std::max(0, gpu))); | ||
| launch.args.push_back("--max-ctx=" + std::to_string(max_ctx)); | ||
| if (!process_.start(launch)) { |
Contributor
There was a problem hiding this comment.
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.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/src/common/qwen3_tool_predictor_ipc.cpp, line 169:
<comment>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.</comment>
<file context>
@@ -0,0 +1,274 @@
+ launch.work_dir = work_dir;
+ launch.args.push_back("--target-gpu=" + std::to_string(std::max(0, gpu)));
+ launch.args.push_back("--max-ctx=" + std::to_string(max_ctx));
+ if (!process_.start(launch)) {
+ std::fprintf(stderr, "[tool-predictor-ipc] backend process start failed\n");
+ return false;
</file context>
| predictor.max_tokens); | ||
| const json tools = req.tools; | ||
| const auto native = native_semantic_predictor_; | ||
| req.semantic_tool_prediction = std::async( |
Contributor
There was a problem hiding this comment.
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.
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 2467:
<comment>`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.</comment>
<file context>
@@ -1969,6 +2434,136 @@ void HttpServer::log_parsed_request(const ParsedRequest & req) const {
+ predictor.max_tokens);
+ const json tools = req.tools;
+ const auto native = native_semantic_predictor_;
+ req.semantic_tool_prediction = std::async(
+ std::launch::async,
+ [predictor, payload, tools, native]() {
</file context>
| result.ok() ? nullptr : "generation_failed"; | ||
| if (req.stream && !client_disconnected) { | ||
| auto final_chunks = emitter.emit_finish(completion_tokens, &gen_timings); | ||
| if (auto metadata = finish_tool_speculation( |
Contributor
There was a problem hiding this comment.
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.
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 4572:
<comment>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.</comment>
<file context>
@@ -3882,22 +4512,95 @@ void HttpServer::process_job(ServerJob * job) {
+ result.ok() ? nullptr : "generation_failed";
if (req.stream && !client_disconnected) {
auto final_chunks = emitter.emit_finish(completion_tokens, &gen_timings);
+ if (auto metadata = finish_tool_speculation(
+ generation_cancel_reason)) {
+ const std::string extension = render_tool_speculation_sse(
</file context>
|
|
||
| The launcher defaults to Qwen3-0.6B Q8_0 on predictor GPU 1. Override placement | ||
| with `PREDICTOR_MODEL`, `PREDICTOR_GPU`, `PREDICTOR_MAX_CTX`, | ||
| `PREDICTOR_MAX_TOKENS`, and `PREDICTOR_TIMEOUT_MS`. The adjacent |
Contributor
There was a problem hiding this comment.
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.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At optimizations/ooo_spec_lucebox5_cpu/README.md, line 88:
<comment>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.</comment>
<file context>
@@ -0,0 +1,137 @@
+
+The launcher defaults to Qwen3-0.6B Q8_0 on predictor GPU 1. Override placement
+with `PREDICTOR_MODEL`, `PREDICTOR_GPU`, `PREDICTOR_MAX_CTX`,
+`PREDICTOR_MAX_TOKENS`, and `PREDICTOR_TIMEOUT_MS`. The adjacent
+`candidate-build` symlink in the wrapper selects a build even though the
+qualified launcher clears ambient variables.
</file context>
| for (size_t offset = 0; offset < content.size(); ++offset) { | ||
| if (content[offset] != '{') continue; | ||
| try { | ||
| const auto value = json::parse( |
Contributor
There was a problem hiding this comment.
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.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/src/server/semantic_tool_hint.cpp, line 109:
<comment>`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.</comment>
<file context>
@@ -0,0 +1,500 @@
+ for (size_t offset = 0; offset < content.size(); ++offset) {
+ if (content[offset] != '{') continue;
+ try {
+ const auto value = json::parse(
+ content.begin() + static_cast<std::ptrdiff_t>(offset),
+ content.end(), nullptr, false);
</file context>
| {"wall_p50_ms", wall_p50}, | ||
| }; | ||
| std::printf("%s\n", summary.dump().c_str()); | ||
| return valid == cases.size() && name_matches == cases.size() ? 0 : 1; |
Contributor
There was a problem hiding this comment.
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.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/test/smoke_qwen3_tool_predictor_ipc.cpp, line 190:
<comment>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.</comment>
<file context>
@@ -0,0 +1,191 @@
+ {"wall_p50_ms", wall_p50},
+ };
+ std::printf("%s\n", summary.dump().c_str());
+ return valid == cases.size() && name_matches == cases.size() ? 0 : 1;
+}
</file context>
Suggested change
| return valid == cases.size() && name_matches == cases.size() ? 0 : 1; | |
| return valid == cases.size() && name_matches == cases.size() && | |
| exact_matches == cases.size() ? 0 : 1; |
Contributor
There was a problem hiding this comment.
2 issues found 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.
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/common/backend_ipc.h">
<violation number="1" location="server/src/common/backend_ipc.h:135">
P2: When `native_work_dir` is under a writable parent, an attacker can replace it after validation and make prompt-file creation follow the replacement path. Validate and retain an opened directory descriptor, then create prompt files relative to it with `openat`/`O_NOFOLLOW`, or always use a freshly created private directory.</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:1375">
P3: The PR description's headline results are stale and now contradict the committed artifact. It cites ~5.60x end-to-end and '14.6s vs 81.0s (p50)' (speculative p50 ~14.6s, normal ~81s), but this artifact records 4.36x paired / 4.27x total speedup with speculative p50 ≈ 20.5s and stage-batched ≈ 88s. Update the description to match the committed numbers.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| bool isolate_inherited_fds = false; | ||
| // Keep legacy backend work directories compatible while allowing private | ||
| // sidecars to require an owned, non-symlink 0700 directory. | ||
| bool require_private_work_dir = false; |
Contributor
There was a problem hiding this comment.
P2: When native_work_dir is under a writable parent, an attacker can replace it after validation and make prompt-file creation follow the replacement path. Validate and retain an opened directory descriptor, then create prompt files relative to it with openat/O_NOFOLLOW, or always use a freshly created private directory.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/src/common/backend_ipc.h, line 135:
<comment>When `native_work_dir` is under a writable parent, an attacker can replace it after validation and make prompt-file creation follow the replacement path. Validate and retain an opened directory descriptor, then create prompt files relative to it with `openat`/`O_NOFOLLOW`, or always use a freshly created private directory.</comment>
<file context>
@@ -124,6 +124,15 @@ struct BackendIpcLaunchConfig {
+ bool isolate_inherited_fds = false;
+ // Keep legacy backend work directories compatible while allowing private
+ // sidecars to require an owned, non-symlink 0700 directory.
+ bool require_private_work_dir = false;
};
</file context>
| "stage_batched_to_speculative_speedup_p05": 3.6679818836822, | ||
| "stage_batched_to_speculative_speedup_p50": 4.359695041609839, | ||
| "tasks": 6, | ||
| "total_wall_speedup": 4.272913990402306 |
Contributor
There was a problem hiding this comment.
P3: The PR description's headline results are stale and now contradict the committed artifact. It cites ~5.60x end-to-end and '14.6s vs 81.0s (p50)' (speculative p50 ~14.6s, normal ~81s), but this artifact records 4.36x paired / 4.27x total speedup with speculative p50 ≈ 20.5s and stage-batched ≈ 88s. Update the description to match the committed numbers.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At optimizations/ooo_spec_lucebox5_cpu/results/trace-compiled-engine-qwen-production-6pairs-compact.json, line 1375:
<comment>The PR description's headline results are stale and now contradict the committed artifact. It cites ~5.60x end-to-end and '14.6s vs 81.0s (p50)' (speculative p50 ~14.6s, normal ~81s), but this artifact records 4.36x paired / 4.27x total speedup with speculative p50 ≈ 20.5s and stage-batched ≈ 88s. Update the description to match the committed numbers.</comment>
<file context>
@@ -801,66 +1312,67 @@
+ "stage_batched_to_speculative_speedup_p50": 4.359695041609839,
"tasks": 6,
- "total_wall_speedup": 5.570717496895011
+ "total_wall_speedup": 4.272913990402306
},
"workload": {
</file context>
Contributor
There was a problem hiding this comment.
1 issue found across 16 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/common/backend_ipc.cpp">
<violation number="1" location="server/src/common/backend_ipc.cpp:71">
P3: When `SYS_close_range` is unavailable (older/seccomp-blocked Linux kernels or non-Linux builds), `close_descriptor_range` falls back to a per-fd `close()` loop that runs up to `RLIMIT_NOFILE` syscalls (commonly 1M on servers) in the child before exec, and aborts the child on any single non-EBADF close failure. Consider bounding the fallback scan (e.g. reuse the cached FD_SETSIZE-style small range or iterate only up to the highest retained descriptor) and treating transient close errors as non-fatal when the fd is confirmed closed.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| // was captured before fork, and verify every ambiguous close failure. | ||
| if (scan_limit == 0 || first >= scan_limit) return scan_limit != 0; | ||
| const unsigned int upper = (std::min)(last, scan_limit - 1U); | ||
| for (unsigned int fd = first; fd <= upper; ++fd) { |
Contributor
There was a problem hiding this comment.
P3: When SYS_close_range is unavailable (older/seccomp-blocked Linux kernels or non-Linux builds), close_descriptor_range falls back to a per-fd close() loop that runs up to RLIMIT_NOFILE syscalls (commonly 1M on servers) in the child before exec, and aborts the child on any single non-EBADF close failure. Consider bounding the fallback scan (e.g. reuse the cached FD_SETSIZE-style small range or iterate only up to the highest retained descriptor) and treating transient close errors as non-fatal when the fd is confirmed closed.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/src/common/backend_ipc.cpp, line 71:
<comment>When `SYS_close_range` is unavailable (older/seccomp-blocked Linux kernels or non-Linux builds), `close_descriptor_range` falls back to a per-fd `close()` loop that runs up to `RLIMIT_NOFILE` syscalls (commonly 1M on servers) in the child before exec, and aborts the child on any single non-EBADF close failure. Consider bounding the fallback scan (e.g. reuse the cached FD_SETSIZE-style small range or iterate only up to the highest retained descriptor) and treating transient close errors as non-fatal when the fd is confirmed closed.</comment>
<file context>
@@ -35,77 +36,70 @@ namespace dflash::common {
+ // was captured before fork, and verify every ambiguous close failure.
+ if (scan_limit == 0 || first >= scan_limit) return scan_limit != 0;
+ const unsigned int upper = (std::min)(last, scan_limit - 1U);
+ for (unsigned int fd = first; fd <= upper; ++fd) {
+ if (::close(static_cast<int>(fd)) == 0 || errno == EBADF) continue;
+ const int close_error = errno;
</file context>
Contributor
There was a problem hiding this comment.
1 issue found across 16 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/common/backend_ipc.cpp">
<violation number="1" location="server/src/common/backend_ipc.cpp:147">
P2: On non-Linux POSIX builds, enabling inherited-FD isolation now makes every Qwen predictor launch fail before `execv`. Keep a portable descriptor-scan fallback for those hosts, or gate this predictor path and its isolation flag on Linux.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| } | ||
| if (!ok) errno = failure == 0 ? EIO : failure; | ||
| return ok; | ||
| #else |
Contributor
There was a problem hiding this comment.
P2: On non-Linux POSIX builds, enabling inherited-FD isolation now makes every Qwen predictor launch fail before execv. Keep a portable descriptor-scan fallback for those hosts, or gate this predictor path and its isolation flag on Linux.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/src/common/backend_ipc.cpp, line 147:
<comment>On non-Linux POSIX builds, enabling inherited-FD isolation now makes every Qwen predictor launch fail before `execv`. Keep a portable descriptor-scan fallback for those hosts, or gate this predictor path and its isolation flag on Linux.</comment>
<file context>
@@ -62,44 +44,131 @@ bool close_descriptor_range(unsigned int first,
+ }
+ if (!ok) errno = failure == 0 ? EIO : failure;
+ return ok;
+#else
+ (void)keep;
+ errno = ENOTSUP;
</file context>
Contributor
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Prediction runs before target compute. On Lucebox5, Qwen3-0.6B Q8_0 runs briefly on Strix; the admitted CPU tool then overlaps DeepSeek-V4 + DS4/DSpark on R9700 + Strix. There is no token injection and no change to decoder selection or recovery.
Production result
Lucebox5, six paired tasks, two each with 10, 15, and 20 leaf calls, randomized arm order:
The slowdown gate uses three alternating controlled A/B repetitions per task with matched cache state, tokens, call digest, and active DS4 decoding. All 22/22 production checks pass. A separate valid-call mismatch probe returns
miss / invocation_mismatchand exposes no private result.The 4.36x applies to recognized recurring workflows. It combines fewer model/tool barriers from trace compilation with a 1.77x early-launch gain; it is not a claim about arbitrary single calls. The broader Qwen smoke protects a 9/12 exact-argument baseline, and misses fall back without changing target output.
Production hardening
tool_choice: noneAutomatic prediction removes the need for a client-supplied hint. Clients still need to consume
dflash_tool_speculation.resulton a hit to realize the latency gain; ignoring the extension remains correct.The same before-target schedule supports one GPU when both models fit, because predictor and target GPU compute do not overlap. The benchmark above is specifically the measured Lucebox5 dual-GPU placement.
Verification
main(0 commits behind)dflash_server,backend_ipc_daemon, unit suite, Qwen smokegit diff --checkEvidence and exact reproduction commands are in
optimizations/ooo_spec_lucebox5_cpu/README.md. The final compact artifact SHA-256 isb1194bd1447f772dc9c90e6e801e99646bdce121e1b300398c45b54540b87d20.Review map
server/src/server/tool_speculation.*server/src/server/http_server.*server/src/server/semantic_tool_hint.*server/src/server/native_semantic_tool_predictor.*server/src/common/qwen3_tool_predictor_ipc*optimizations/ooo_spec_lucebox5_cpu/