Speed up train completion usage serialization - #1787
Merged
Conversation
Contributor
ApprovabilityVerdict: Approved This is a simple performance optimization replacing Pydantic model construction with direct dict building. The serialized output format remains identical, making this a low-risk mechanical change with no behavioral impact. You can customize Macroscope's approvability policy. Learn more. |
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
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.
Overview
Build the OpenAI-compatible completion usage payload directly from the already-typed
vf.Usageobject in the renderer training client.Rationale
The previous path constructed
CompletionUsageplus optional nested OpenAI Pydantic detail models, then immediately dumped those temporary objects back into a dictionary. Usage has already been validated earlier in the Verifiers pipeline, so reconstructing and serializing SDK models adds allocations and validation work without changing the wire contract.The new path assembles the final dictionary directly. It preserves cached-token accounting, total-token calculation, omission of absent optional details, inclusion of zero-valued details, and
usage: nullwhen usage is absent. A short code comment documents why direct construction is intentional.Performance impact
A Python 3.13.12 microbenchmark using the real
vf.Responseandvf.Usagetypes ran 100,000 serializations across seven repetitions:This keeps the optimization local to synthesized completion responses from
TrainClient; other clients and shared usage types are unchanged.Note
Low Risk
Localized serialization change in the training client with the same intended wire shape; no auth, security, or shared pipeline changes.
Overview
serialize_completionin the rendererTrainClientnow builds the OpenAI-styleusageobject as a plain dict instead of instantiatingCompletionUsageand nested token-detail Pydantic models and then callingmodel_dump.The mapping is unchanged:
input_tokens→prompt_tokens, optionalreasoning_tokensandcached_input_tokensnested undercompletion_tokens_details/prompt_tokens_details, andusagestays absent when there is no usage. A short comment notes that usage is already validated upstream.OpenAI SDK imports used only for this path are removed. Other clients and interception serialization are untouched.
Reviewed by Cursor Bugbot for commit 65cc626. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Speed up train completion usage serialization by replacing OpenAI model objects with plain dicts
In
serialize_completion, theusagefield is now built as a plain Pythondictinstead of constructing anopenai.types.CompletionUsageobject and callingmodel_dump(). Sub-dicts forcompletion_tokens_detailsandprompt_tokens_detailsare built conditionally based on the presence ofreasoning_tokensandcached_input_tokens. This avoids Pydantic model instantiation overhead on every call.Macroscope summarized 65cc626.