fix: replace mutable default arguments with None defaults#1063
Open
dieutx wants to merge 1 commit intoPrimeIntellect-ai:mainfrom
Open
fix: replace mutable default arguments with None defaults#1063dieutx wants to merge 1 commit intoPrimeIntellect-ai:mainfrom
dieutx wants to merge 1 commit intoPrimeIntellect-ai:mainfrom
Conversation
Replace dangerous mutable default arguments (dict={}, list=[]) with
None defaults across core library files. Mutable defaults are shared
across all calls to a function, which can cause silent data corruption
when the default object is stored or mutated.
Changed functions:
- Environment.__init__, _ensure_prompt, _ensure_task, _format_dataset,
_format_completion_dataset: map_kwargs: dict = {} -> dict | None = None
- EnvGroup.__init__, _format_dataset, _format_completion_dataset:
map_kwargs: dict = {} -> dict | None = None
- StatefulToolEnv.add_tool: args_to_skip: list[str] = [] -> list[str] | None = None
- format_dataset: map_kwargs: dict = {} -> dict | None = None
- OpenAIChatCompletionsTokenClient.tokenize: extra_kwargs: dict = {} -> dict | None = None
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.
Summary
Replace mutable default arguments (
dict = {},list = []) withNonedefaults in core library files. This is a well-known Python bug where mutable defaults are shared across all calls to a function — if the object is stored as an instance attribute or mutated, it silently corrupts state across unrelated callers.Changed functions
Environment.__init__:map_kwargsis stored asself.map_kwargs, meaning all instances using the default share the same dictEnvironment._ensure_prompt,_ensure_task,_format_dataset,_format_completion_dataset:map_kwargsspread via**— safe today but fragile if any caller later stores or mutates itEnvGroup.__init__,_format_dataset,_format_completion_dataset: same pattern as EnvironmentStatefulToolEnv.add_tool:args_to_skipis stored inself.skipped_args[tool_name]— all tools added without explicitargs_to_skipshare the same listformat_datasetindata_utils.py:map_kwargsspread via**OpenAIChatCompletionsTokenClient.tokenize:extra_kwargsspread via**Pattern applied
Test plan
ruff checkpasses on all changed filesruff formatreports no changes neededpytest tests/ --ignore=tests/test_envs --ignore=tests/test_envs.pyNote
Low Risk
Low risk, behavior-preserving change that prevents shared mutable defaults from leaking state across calls/instances. Minor risk that callers relied on mutating the passed-in default object (now always a fresh dict/list when omitted).
Overview
Fixes several functions that previously used mutable default arguments (e.g.
dict = {}/list = []) by switching them toNonedefaults and initializing locally.This affects dataset mapping kwargs plumbing (
map_kwargs) acrossEnvironment,EnvGroup, andformat_dataset, tool registration inStatefulToolEnv.add_tool(args_to_skip), and extra request parameters inOpenAIChatCompletionsTokenClient.tokenize(extra_kwargs), preventing accidental state sharing between calls or instances.Written by Cursor Bugbot for commit e9b8054. This will update automatically on new commits. Configure here.