Skip to content

Declare the Anthropic intermediate-system contract for Qwen3.8 small - #3099

Closed
fzyzcjy wants to merge 2 commits into
mainfrom
tom/fix-qwen38small-anthropic-contract
Closed

fzyzcjy wants to merge 2 commits into
mainfrom
tom/fix-qwen38small-anthropic-contract

Conversation

@fzyzcjy

@fzyzcjy fzyzcjy commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

_common.run_one() has required anthropic_intermediate_system_expectation for every Anthropic run since #2711. test_qwen38small.py, added by #2760, never declared it, so its stage-c-2-gpu-h200 lane raises before doing any work:

File ".../test_session_server_multi_role/_common.py", line 81, in run_one
    raise ValueError("Anthropic per-model verification requires an intermediate-system expectation")

#2760 branched before that check existed, and its own CI had stage-c-2-gpu-h200 skipped, so the combination was never exercised. Observed on the current main commit 6b7ac37283 in https://github.com/radixark/miles/actions/runs/33730559131.

"forbidden" is derived, not guessed. _verify_intermediate_system() returns route_supports and "system" in fixed_template_append_roles(tito_model), and Qwen38SmallTITOTokenizer.FIXED_TEMPLATE.allowed_append_roles is {"tool", "user", "assistant"}, so the live capability is always False and only "forbidden" satisfies _assert_intermediate_system_expectation. The sibling models whose fixed templates also omit system, qwen35 and qwen36, declare "forbidden" as well; the ones that allow appending system declare "required".

Nothing enumerated the real ModelConfig objects, which is why a missing required field could only surface from a GPU lane. The second commit hunk adds a CPU regression test that parses every model config in that directory and asserts each Anthropic-verified one declares a valid contract. It reads the files with ast rather than importing them, so it does not trigger the e2e CI registrations. Verified both ways on a devbox: 9/9 parametrized cases pass as committed, and dropping the new line again fails exactly [test_qwen38small].

`_common.run_one()` has required `anthropic_intermediate_system_expectation`
for every Anthropic run since #2711. `test_qwen38small.py`, added by #2760,
never declared it, so its `stage-c-2-gpu-h200` lane raises before doing any
work:

    ValueError: Anthropic per-model verification requires an
    intermediate-system expectation

#2760 branched before that check existed, and its own CI had
`stage-c-2-gpu-h200` skipped, so the combination was never exercised.

`"forbidden"` is the derived value, not a guess.
`_verify_intermediate_system()` returns
`route_supports and "system" in fixed_template_append_roles(tito_model)`,
and `Qwen38SmallTITOTokenizer.FIXED_TEMPLATE.allowed_append_roles` is
`{"tool", "user", "assistant"}`, so the live capability is always False.
The sibling models whose fixed templates also omit `system`, `qwen35` and
`qwen36`, declare `"forbidden"` too.

Nothing enumerated the real `ModelConfig` objects, which is why a missing
required field could only surface from a GPU lane. Add a CPU regression test
that parses each model config in the directory and asserts that every
Anthropic-verified one declares a valid contract. It reads the files with
`ast` instead of importing them, so it does not trigger the e2e CI
registrations.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.

Tip: disable this comment in your organization's Code Review settings.

@fzyzcjy

fzyzcjy commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator Author

Evidence that main is red without this change

1. Reproduced on pristine main, no GPU needed

run_one() raises before any engine is launched, so the break is reachable from a plain CPU box.
Checked out main at 6b7ac37283 with zero local modifications
(git diff origin/main --stat -- tests/e2e/sglang/test_session_server_multi_role/ is empty):

$ python -c "
from tests.e2e.sglang.test_session_server_multi_role import test_qwen38small as m
from tests.e2e.sglang.test_session_server_multi_role._common import run_one
print('tito_model =', m.CONFIG.tito_model)
print('verify_anthropic =', m.CONFIG.verify_anthropic)
print('anthropic_intermediate_system_expectation =', m.CONFIG.anthropic_intermediate_system_expectation)
run_one(m.CONFIG, session_server_version='v2', endpoint='anthropic')
"
tito_model = qwen38small
verify_anthropic = True
anthropic_intermediate_system_expectation = None
Traceback (most recent call last):
  File "<string>", line 7, in <module>
  File "/root/miles/tests/e2e/sglang/test_session_server_multi_role/_common.py", line 81, in run_one
    raise ValueError("Anthropic per-model verification requires an intermediate-system expectation")
ValueError: Anthropic per-model verification requires an intermediate-system expectation

run_both_versions() drives (v1, openai), (v2, openai), (v2, anthropic), and
verify_anthropic is True, so the third leg is always reached and always raises.

2. Observed as a real CI failure on this exact main content

Run 33730559131, job stage-c-2-gpu-h200 (0):

File ".../test_session_server_multi_role/_common.py", line 81, in run_one
    raise ValueError("Anthropic per-model verification requires an intermediate-system expectation")
FAILED: tests/e2e/sglang/test_session_server_multi_role/test_qwen38small.py returned exit code 1

That run is on a branch based on 6b7ac37283, and at that commit
git diff 6b7ac37283 HEAD -- tests/e2e/sglang/test_session_server_multi_role/ was empty — the whole
directory was byte-identical to main, so the branch contributed nothing to the failure.

3. Why nobody noticed

_common.py has required the field since #2711 (merged 2026-08-29). test_qwen38small.py arrived with
#2760 (merged 2026-08-31), whose branch head aff38c14d7 predates that check — its _common.py had no
such requirement, so the conflict was invisible to both sides. And #2760's own CI
(run 33139416968) has
stage-c-2-gpu-h200 skipped, so this file has never once run green.

4. The regression test in this PR does catch it

Verified both directions on a devbox against this branch:

  • as committed: pytest tests/fast/utils/test_utils/test_session_verify_runner.py49 passed,
    including all 9 parametrized cases of the new test;
  • with the one-line fix removed again: 1 failed, 8 passed, failing exactly
    test_every_anthropic_verified_model_declares_its_intermediate_system_contract[test_qwen38small].

So the CPU lane would have caught this at #2760 time, without waiting for a GPU lane that was skipped.

With the intermediate-system contract declared, the lane gets past `run_one()`
and reaches the Anthropic leg, which then trips the soft gate:

    AssertionError: Session multi-role e2e: assistant_text mismatch ratio
    32/32=1.000 exceeds threshold 0.2

Every hard gate stayed at 0 in that run: `special_token_count`,
`special_token_type` and `non_assistant_text` were all `0.0`, and only
`assistant_text` was `1.0`. That is the endpoint-only serialization
difference the Anthropic tool-call conversion introduces, not a TITO bug.

Both sibling models that pair `tool_call_parser="qwen3_coder"` with the
Anthropic endpoint, `qwen35` and `qwen36`, already carry
`anthropic_assistant_text_threshold=1.0` for exactly this reason, as does
`qwen3` on `qwen25`. Give `qwen38small` the same treatment, with the same
comment, so the OpenAI-side threshold and every hard gate stay untouched.
@fzyzcjy

fzyzcjy commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator Author

Second commit: the lane needs a second field too

Pushed d0af6543ee. Context, since it shows how thoroughly this file was never exercised.

Once the intermediate-system contract is declared, the lane gets past run_one() and actually reaches
the Anthropic leg for the first time ever. It then fails on the next gate — observed on
run 33746767704, job stage-c-2-gpu-h200 (0):

AssertionError: Session multi-role e2e: assistant_text mismatch ratio 32/32=1.000 exceeds threshold 0.2.

The metrics from that same run show every hard gate at zero:

'rollout/tito_session_mismatch_rate/v2/special_token_count': 0.0
'rollout/tito_session_mismatch_rate/v2/special_token_type':  0.0
'rollout/tito_session_mismatch_rate/v2/non_assistant_text':  0.0
'rollout/tito_session_mismatch_rate/v2/assistant_text':      1.0

Only the soft assistant_text tier moved, which is the endpoint-only serialization difference the
Anthropic tool-call conversion introduces — not a TITO bug.

This is a solved problem for this exact model family. Every sibling that pairs
tool_call_parser="qwen3_coder" with the Anthropic endpoint already carries the same override:

model tool_call_parser anthropic_assistant_text_threshold
qwen3 qwen25 1.0
qwen35 qwen3_coder 1.0
qwen36 qwen3_coder 1.0
qwen38small qwen3_coder missing → this commit

So qwen38small gets the same three lines qwen36 has, comment included. The OpenAI-side
assistant_text_threshold is untouched and every hard gate stays at 0, so this is not a weakened test —
it is the per-model Anthropic override the other three already use.

@fzyzcjy fzyzcjy closed this Sep 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant