fix: make ToolContext hashable to match RunContextWrapper#3097
Merged
seratch merged 1 commit intoopenai:mainfrom May 4, 2026
Merged
fix: make ToolContext hashable to match RunContextWrapper#3097seratch merged 1 commit intoopenai:mainfrom
seratch merged 1 commit intoopenai:mainfrom
Conversation
ToolContext was decorated with bare @DataClass, which sets __hash__ = None when eq defaults to True. Its parent RunContextWrapper uses @DataClass(eq=False) and is hashable by identity, so the subclass silently dropped the parent's hashability contract. Switch ToolContext to @DataClass(eq=False) to restore consistency with the parent and add a regression test.
32d2f72 to
b5f9b8a
Compare
Member
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Contributor
Author
|
Thank you for the quick response, @seratch ! 🙌 |
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
ToolContext(src/agents/tool_context.py) is decorated with bare@dataclass, which defaults toeq=Trueand therefore sets__hash__ = None, making instances unhashable. Its parentRunContextWrapper(src/agents/run_context.py) is decorated@dataclass(eq=False)and is hashable by identity. The subclass silently dropped the parent's hashability contract, so user code that puts a tool context into aset,dictkey,WeakKeyDictionary,lru_cache, etc. — a reasonable thing to do given the parent supports it — fails withTypeError: unhashable type: 'ToolContext'.Repro against
main/ v0.15.1:Fix: switch the decorator to
@dataclass(eq=False)so the subclass preserves the parent's identity-based hashing. This is a one-line change with no behavior impact for code that does not rely on hashing —ToolContextalready defines its own__init__, so the dataclass-generated__eq__was the only thing the bare decorator was adding, and identity equality is what the parent intentionally uses.Test plan
tests/test_tool_context.py::test_tool_context_is_hashable_like_run_context_wrappercoveringhash(ToolContext(...)), equality of repeated hashes, and use as a dict key.uv run pytest tests/test_tool_context.py -v— all 15 tests pass.make format,make lint— clean.make typecheck— no new errors in the touched files (pre-existing errors in voice/sandbox modules are unrelated).make tests— same 12 pre-existing failures asmain(sandbox/runloop and voice modules); no new failures.Issue number
N/A
Checks
make lintandmake format