Repository navigation
fix(frontend): Preserve Anthropic tool strictness and bump frontend-crates for dependencies - #15547
Conversation
Signed-off-by: Ryan McCormick <rmccormick@nvidia.com>
This comment was marked as outdated.
This comment was marked as outdated.
Signed-off-by: Ryan McCormick <rmccormick@nvidia.com>
|
/ok to test eafa074 |
Signed-off-by: Ryan McCormick <rmccormick@nvidia.com>
Signed-off-by: Ryan McCormick <rmccormick@nvidia.com>
|
/ok to test e6f6901 |
Signed-off-by: Ryan McCormick <rmccormick@nvidia.com>
|
/ok to test 7bd5f17 |
nv-yna
left a comment
There was a problem hiding this comment.
Reviewed d7fc3cafab1a11cb3d487bdbedc38da873ec58c8 with Codex only.
Two verified findings are attached: High — byte-fallback output is lost across migration; Medium — buffered text is attributed to the wrong token's logprob. These findings remain actionable despite this approval.
Validation: 19 backend, 900 protocol, 162 HTTP/OpenAI, and 35 existing migration test executions passed; 10 protocol tests ignored. Two added review-only regression probes reproduced the attached defects. No GPU inference or HTTP end-to-end test was run. The existing visible-stop-token discussion was checked and not duplicated.
dmitry-tokarev-nv
left a comment
There was a problem hiding this comment.
No P0 or P1 found in the dependency files. One P2 is open. Read at d7fc3cafab1a11cb3d487bdbedc38da873ec58c8.
This review covers Cargo.toml, lib/bindings/python/Cargo.toml and the three Cargo.lock files.
[P2] On a migration retry to another worker, the tokenizer pin =3.1.0 at Cargo.toml:68 loses a buffered byte-fallback character. This review reproduced the open thread from @nv-yna on that line. The measurement is in the reply on that thread. Please answer there.
@GuanLuo, yes. The head pins dynamo-protocols = { version = "=8.1.0" } at Cargo.toml:77. lib/llm, lib/backend-common and deploy/inference-gateway/ext-proc take it with workspace = true. The root, lib/bindings/python and lib/bindings/kvbm lockfiles all resolve 8.1.0 with sha256 aca812fe988abff6….
Open items from other reviewers that this review did not test:
- @nv-yna [Medium], selected-token text and logprob,
lib/llm/src/backend.rs:898. - CodeRabbit, a visible stop token and a stop string,
lib/llm/src/backend.rs:912.
The six crate moves are consistent and match their sources, with controls.
- Each lockfile changes six packages and nothing else: root 1052 packages,
lib/bindings/python945,lib/bindings/kvbm884. The dependency list of each moved crate did not change. - Each new
.cratefrom static.crates.io has the sha256 that all three lockfiles record (18 of 18). - The five
dynamo-*crates came from crates.io trusted publishing inai-dynamo/frontend-crates. Each packaged source file is byte-identical to that repository at the commit in.cargo_vcs_info.json, and each commit is onmain. Renderer 8.0.1, protocols 8.1.0, parsers 10.0.1 and parsers-v2 0.7.20 contain ai-dynamo/frontend-crates#322 and ai-dynamo/frontend-crates#375. async-openai0.42.1 has the same sole owner (64bit) and repository as 0.41.1.dynamo-protocolsturns on only*-typesfeatures, so it resolves with 5 dependencies and no HTTP client.- None of the twelve old and new crate versions has a build script, a proc-macro or a
linkskey. The new process spawn indynamo-parsers10.0.1 is in a#[cfg(test)]module. cargo tree --locked --offlineexits 0 in the root,lib/bindings/pythonandlib/bindings/kvbm. With the base lockfiles under the head manifests, it exits 101 in all three.cargo test --locked --offline -p dynamo-llm --no-default-featuresat the head on macOS: lib 3027 passed, 3 failed, 13 ignored. The base tree has the same failure count. The gRPC metrics-bind test fails on both trees, and the other failures change between runs and pass alone. These targets pass:backend1,test_stop_behavior16,test_streaming_usage14,tokenizers22,responses_http_replay24,frontend_protocol_validation_http10,anthropic_http_replay9.- On the head, Pre Merge passed, including
rust-testsandrust-clippyfor the root and both bindings. At 18:40Z the PR run onpull-request/15547still had 4 jobs running, and no job had failed. - Merged with
mainat6c541e70e3, which brings #15979 into the same Anthropic files, the tree builds. The Anthropic lib tests (102) andanthropic_http_replay(10) pass on that merge.
dmitry-tokarev-nv
left a comment
There was a problem hiding this comment.
Approving from DevOps side.
Signed-off-by: Guan Luo <gluo@nvidia.com>
Signed-off-by: Ryan McCormick <rmccormick@nvidia.com>
Signed-off-by: Alex Steiner <asteiner@nvidia.com>
|
/ok to test 4f3cb58 |
Retain the native schema delta while adopting the published dependency pins and runtime fixes from #15547. Validation runs on Linux, not in the local macOS commit hook. Signed-off-by: Guan Luo <gluo@nvidia.com>
ai-dynamo#15547 moved main to async-openai 0.42, whose ImageQuality adds xhigh and max. The frontend on main accepts them on /v1/images. The owned ImageQuality kept the six values of 0.41, so it rejected them with a 400. The enum-value tests listed the owned values, so they could not see the gap. They now take their values from the OpenAI enums through an exhaustive match. A value that a later async-openai adds fails to compile in the test until it is listed, and the test then checks that the Dynamo type accepts it. The image schema and the generated Python model gain the two values. Signed-off-by: Jorge Antonio <matroid@outlook.com>
ai-dynamo#15547 moved main to async-openai 0.42, whose ImageQuality adds xhigh and max. The frontend on main accepts them on /v1/images. The owned ImageQuality kept the six values of 0.41, so it rejected them with a 400. The enum-value tests listed the owned values, so they could not see the gap. They now take their values from the OpenAI enums through an exhaustive match. A value that a later async-openai adds fails to compile in the test until it is listed, and the test then checks that the Dynamo type accepts it. The image schema and the generated Python model gain the two values. Signed-off-by: Jorge Antonio <matroid@outlook.com>
Summary
Preserve Anthropic
tool.strictthrough request deserialization and conversion toFunctionObject.strict, including explicitfalseand omission. Keep the tool input schema intact.Use the published frontend crates released after ai-dynamo/frontend-crates#322 and ai-dynamo/frontend-crates#375
Adapt the Responses API types, enum variants, and usage initializers to the new protocol API. Preserve existing error codes and the legacy prompt-cache retention wire field; reject the new
ultrafasttier because Chat Completions cannot represent it.Finalize the tokenizer at stream termination and stop-token boundaries so tokenizer 3's buffered byte-fallback suffix is emitted through the existing stop-string filter. The regression check covers length completion, bare EOF, predecoded terminal text, hidden stop tokens, and visible stop tokens, including overlapping visible/hidden IDs.
Closes ai-dynamo/frontend-crates#320.
Internal tracking: DIS-3038.
Validation
Using the committed registry dependencies and lockfiles, with no local Cargo override:
dynamo-llmlibrary tests passed; 12 ignored. Covers protocols, preprocessing, HTTP handlers, backend decoding, migration, and mocker behavior.frontend_protocol_validation_http,responses_http_replay,test_stop_behavior,test_streaming_usage, andtokenizers. The 22 tokenizer tests include the HuggingFace prefill/multibyte cases and now finalize their decode streams.Summary by CodeRabbit