Skip to content

feat(server): use shared model-agnostic canonical request/response types#307

Open
sercand wants to merge 4 commits into
Avarok-Cybersecurity:mainfrom
sercand:feat/canonical-chat-ir
Open

feat(server): use shared model-agnostic canonical request/response types#307
sercand wants to merge 4 commits into
Avarok-Cybersecurity:mainfrom
sercand:feat/canonical-chat-ir

Conversation

@sercand

@sercand sercand commented Jul 13, 2026

Copy link
Copy Markdown

This PR is quite big, sorry for putting everything into single PR.

Summary

Introduces a canonical, model-agnostic chat IR (crates/spark-server/src/ir/) and migrates every API surface to it, so the core pipeline (build_msg_entries, template rendering, scheduler) consumes one provider-free representation instead of the OpenAI wire structs. Each protocol becomes a thin, unit-tested adapter pair: openai/to_ir.rs / anthropic/to_ir.rs on the request side, openai/encode.rs + encode_stream.rs, anthropic/translate.rs (ir_to_anthropic_response) + the translator.rs streaming state machine, and typed responses_translate.rs / responses_stream.rs on the response side.

Because Message.content is now always a list of parts for every role, the motivating bug — images attached to tool-result messages being silently dropped — falls out for free, along with the rest of the OpenAI-shape tax: the Anthropic struct→JSON→struct round-trip is deleted, the Responses API no longer drops images, and the <think>\n\n</think> and [tool error]\n string-smuggling hacks become real fields (reasoning, an error flag on tool results). The migration commit also fixes a scheduler bug surfaced by end-to-end verification of the tool+image agent loop: the post-think EOS guard fired on every tools-armed turn (eating the model's natural <|im_end|> and causing scaffold garbage plus spurious duplicate tool calls); it is now gated on a sticky think_force_closed flag so it only applies after a watchdog/budget force-injected </think>.

Closes #165 and #164

Test plan

  • cargo fmt --all -- --check
  • ATLAS_SKIP_BUILD=1 cargo clippy --workspace --tests -- -Dwarnings
  • bash scripts/check-license-headers.sh
  • Tested against a real model: live on Qwen3.6-35B-A3B-NVFP4 (grammar ON) — /v1/messages turn-2 tool replays 4/4 clean end_turn; full 2-turn tool+image agent loop 3/3 clean (previously 4 turns with chat-template scaffold garbage and duplicate tool calls); new scripts/test-qwen36-tool-image.sh drives the loop end-to-end
  • Added or updated tests: cargo test -p spark-server → 689 passed, 0 failed (30 lib + 659 bin). New suites: ir/tests.rs, anthropic/tests/ir_carry.rs (wire→IR→wire carry), anthropic/tests/translator_stream.rs (streaming-framing goldens), expanded openai/tests.rs; prompt-byte stability gates (prompt_json_stability_gate, rendered_prompt_json_matches_retired_json_hop_output) pin rendered prompts to the pre-migration output

Notes for reviewers

  • Invariants the tests pin: prompt bytes are the KV-cache prefix and wire framing is what agent clients execute, so two golden gates guard the migration — the rendered-prompt JSON gates (template + Anthropic retired-JSON-hop) and the Anthropic streaming goldens. Any future change to message lowering or stream framing should keep these green.
  • Echo-only wire fields (service_tier, metadata, store, include_usage) deliberately do not enter the IR; they ride api/chat/echo.rs::ResponseEcho handler-side.
  • Responses API lowers to the chat wire inside openai/ before hitting the IR — intra-provider normalization, deliberate; it now carries images instead of dropping them (from_responses_input_item).
  • Accepted wire references outside the adapter edges: response_store (caches wire bodies) and chat_stream --dump synthesis.
  • Deferred follow-ups: a neutral ApiError type; top-level reasoning_effort remains dead wire-compat (never wired into the thinking ladder).
  • count_tokens now shares api/chat/prepare.rs::prepare_chat_prompt with the serving path (measured end-to-end drift 0.0%).

CLA

Introduce crates/spark-server/src/ir/ — a provider-free chat IR
(ChatRequest, Message with content always a list of parts,
ChatResponse, StreamDelta) — and migrate every API surface onto it:
openai/to_ir.rs + anthropic/to_ir.rs adapt requests in; encode.rs /
encode_stream.rs / translate.rs / responses_translate.rs serialize
responses out. Deletes the Anthropic struct→JSON→struct round-trip,
the Responses-API image drop, and the <think> / [tool error] string
prefixes (now real fields). Images ride every role uniformly,
including tool results (issue Avarok-Cybersecurity#165).

Also scopes the post-think EOS guard to force-closed </think> spans
(sticky ActiveSeq::think_force_closed): it previously fired on every
tools-armed turn, eating the model's natural <|im_end|> after brief
post-tool answers and causing scaffold garbage + spurious duplicate
tool calls. Verified live on Qwen3.6-35B-A3B-NVFP4: turn-2 /v1/messages
replays 4/4 clean end_turn; 2-turn tool+image agent loop 3/3 clean.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jul 13, 2026

Copy link
Copy Markdown

All contributors have signed the CLA. Thank you!
Posted by the CLA Assistant Lite bot.

@sercand

sercand commented Jul 13, 2026

Copy link
Copy Markdown
Author

I have read the CLA Document and I hereby sign the CLA

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[feat] Canonical internal chat IR: convert every API surface into one model-agnostic request/response type (unblocks images in tool results)

2 participants