feat: support self/cls in function_tool for class methods#2734
feat: support self/cls in function_tool for class methods#2734mvanhorn wants to merge 6 commits intoopenai:mainfrom
Conversation
When a class method decorated with @function_tool is used as a tool, the self/cls parameter was included in the generated JSON schema, causing OpenAI API 400 errors. This skips the first parameter when it has no type annotation and is named "self" or "cls". Also handles the case where RunContextWrapper/ToolContext follows self/cls as the second parameter. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 008294bc56
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
seratch
left a comment
seratch
left a comment
There was a problem hiding this comment.
If you're willing to take more time, please continue working until Codex auto review gives 👍 to your PR. Then, we'll check the direction and details.
The self_or_cls_skipped flag persisted across all remaining params, so `def f(self, x: int, ctx: RunContextWrapper)` incorrectly passed validation. Now only the param at index 1 (immediately after self/cls) is treated as the effective first param. Adds a test for this edge case. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 73b9a9f677
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
When self/cls was skipped during schema generation, the original signature (including self/cls) was stored in FuncSchema. This caused to_call_args to misalign parameter indices - skipping self instead of the context param. Now we rebuild the signature without self/cls so to_call_args iterates only the parameters the caller passes.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2e85520886
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Use a permanent receiver_was_skipped flag instead of checking params[0] name at call_sig construction time. Prevents stripping a self/cls parameter that is actually the context argument (e.g. def tool(self: RunContextWrapper[Any], q: str)). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 22f1f83d08
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
src/agents/function_schema.py
Outdated
| elif first_name in ("self", "cls"): | ||
| self_or_cls_skipped = True # Skip bound method receiver parameter | ||
| receiver_was_skipped = True |
There was a problem hiding this comment.
Skipping a leading unannotated self/cls here makes unbound methods look supported, but the invocation path still calls the original callable without injecting an instance/class receiver (it only prepends tool context when takes_context is true). In scenarios like function_tool(MyTools.greet) or class-defined methods decorated with @function_tool, schema creation now succeeds but execution fails with a missing-argument TypeError at runtime because the receiver was removed from argument construction.
Useful? React with 👍 / 👎.
src/agents/function_schema.py
Outdated
| remaining = [p for name, p in params if name not in ("self", "cls")] | ||
| call_sig = sig.replace(parameters=remaining) |
There was a problem hiding this comment.
This filter removes every parameter named self or cls, not just the first receiver that was skipped. For valid signatures like def f(self, cls: int, value: str), the generated schema still includes cls, but to_call_args iterates a signature where cls was deleted, so the provided value is ignored and argument mapping becomes incorrect. The replacement signature should remove only the single leading receiver parameter.
Useful? React with 👍 / 👎.
Codex review found that skipping self/cls from schema generation makes unbound methods appear supported, but execution fails with TypeError because the receiver is never injected. Bound methods (instance.method) already work - Python strips self before inspect.signature sees it. - Raise UserError for unbound self/cls instead of silent schema-only skip - Remove dead receiver_was_skipped logic and call_sig stripping (P2 fix) - Clean up self_or_cls_skipped variable (now unreachable) - Update tests to expect UserError for unbound methods
|
@codex review |
|
Addressed both Codex findings in 98ba075: P1 (bind receiver): Unbound methods now raise P2 (drop only leading receiver): Removed entirely - the filter is now dead code since we reject instead of skip. Net effect: -26 lines. Simpler code, no silent runtime failures. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 98ba075e1c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
src/agents/function_schema.py
Outdated
| elif first_name in ("self", "cls"): | ||
| # An unannotated self/cls means this is an unbound method or classmethod. | ||
| # Bound methods (e.g., instance.method) already have self stripped by Python. | ||
| raise UserError( | ||
| f"Function {func.__name__} has an unbound '{first_name}' parameter. " |
There was a problem hiding this comment.
This branch now throws UserError for any unannotated first parameter named self/cls, but @function_tool is applied to unbound functions during class definition, so class T: @function_tool def f(self, x: str) now fails immediately. Because function_tool always calls function_schema(the_func, ...) on that unbound callable, this change blocks the common decorator workflow for method-based tools instead of allowing them to be registered and later invoked as bound methods.
Useful? React with 👍 / 👎.
The UserError for unannotated self/cls blocked @function_tool usage on class methods during decoration time. Now silently skips self/cls and stores a self_or_cls_skipped flag. The stored signature excludes self/cls so to_call_args produces correctly aligned arguments. Context params immediately after self/cls are recognized as effective-first position.
2 participants
Summary
When a class method decorated with
@function_toolis used as a tool, theself(orcls) parameter leaks into the generated JSON schema. The OpenAI API rejects this with a 400 error becauseselfhas notypekey. This PR skips the first parameter when it has no type annotation and is namedselforcls, and also handles the case whereRunContextWrapper/ToolContextfollowsself/clsas the second parameter.Test plan
test_method_self_param_skipped- verifiesselfis excluded from schema for bound methodstest_classmethod_cls_param_skipped- verifiesclsis excluded for classmethod-like functionstest_method_self_with_context_second_param- verifiesselfskip +RunContextWrapperas second param workstest_regular_unannotated_first_param_still_included- verifies non-self/cls unannotated params still workmake tests- 2405 passed)Issue number
Closes #94
Checks
make lintandmake formatThis contribution was developed with AI assistance (Claude Code).