remove redundant msg normalization + align env_response api - #1027
Merged
Conversation
mikasenghaas
marked this pull request as ready for review
March 17, 2026 10:34
eligotts
reviewed
Mar 17, 2026
willccbb
approved these changes
Mar 17, 2026
willccbb
left a comment
Member
There was a problem hiding this comment.
Approving in advance, but let's auto-convert or hard-fail if get_prompt_messages is overridden to return dicts. would prefer auto-convert i think, writing dicts is often the more ergonomic way to do it, but fine either way (could surface a helper fn + mention in docstring?).
env_response api
This reverts commit 8b26a4250a581954a3edbb12268deb4b0ebeeed9.
mikasenghaas
force-pushed
the
pydantic-normalization
branch
from
March 18, 2026 13:06
3b62a82 to
162b0ac
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
eligotts
approved these changes
Mar 18, 2026
hallerite
added a commit
that referenced
this pull request
Apr 28, 2026
* docs/evaluation.md: --api-client-type list was missing nemorl_chat_completions and ordered inconsistently with verifiers/types.py; align with the source of truth. * base.py / renderer_client.py: simplify factory closures from default-arg-as-pseudo-closure (factory(_name=…, _model=…, …)) to plain factory() -> Renderer; the captured locals are stable for the function's lifetime, no late-binding footgun. * clients: drop redundant maybe_normalize_messages calls from OpenAIChatCompletionsClient.to_native_prompt and RendererClient.to_native_prompt. PR #1027 explicitly centralized message normalization in the env loop and removed downstream copies; we re-introduced the redundancy by accident. * renderer_client.py: collapse _get_renderer_or_pool's inline factory + RendererPool() construction into a direct create_renderer_pool() call (single source of truth for pool construction lives in packages/renderers/renderers/base.py). * README: switch the create_renderer / create_renderer_pool import examples to the top-level package path. Tests in test_renderer_client.py updated to patch create_renderer_pool instead of the now-uncalled create_renderer. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

Description
Reduces calls to
normalize_messagesto minimal set:init_state(this is safe, because not on hot path)get_prompt_messagesorenv_response) we usemaybe_normalize_messageswhich emits a warning before normalizing the message to nudge users towards using the types directly to avoid potential performance bottlenecksenv_responsereturn type fromMessages | strto justMessagesto be consistent with the rest of the API (e.g.get_model_response,rollout, etc just operate onMessagesandstr -> vf.Messagesconversion is an example of (undesired) conversion to custom types) and thus be handled the same way as list of dictsType of Change
Testing
uv run pytestlocally.Checklist
Additional Notes
Note
Medium Risk
Tightens the core
MultiTurnEnv/Environment.get_model_responseAPIs to requireMessagesand introduces conditional normalization, which could break downstream custom envs/providers that still return raw dicts/strings or pass string prompts.Overview
Standardizes environment and model prompting to operate on typed
vf.Messagesonly:env_response()andget_model_response()now take/returnMessages(no morestr), and built-in envs (alphabet_sort,doublecheck,sentence_repeater,GymEnv) were updated to emitvf.UserMessageobjects instead of raw dicts.To cut hot-path Pydantic overhead,
MultiTurnEnvnow usesmaybe_normalize_messages()(new helper) to only normalize when needed and logs a warning once when callers still return raw dicts/strings; supportinglog_once/warning_onceutilities were added, andparse_response_message()now constructsAssistantMessagedirectly instead of round-tripping throughmodel_validate.Written by Cursor Bugbot for commit cd87b2c. This will update automatically on new commits. Configure here.