-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Python: Include constructor tools in agent-hooks startup #7600
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
1395b93
08094a0
9e75ecd
eb00f36
b212dc9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -857,21 +857,44 @@ def _agent_updates_from_response(response: AgentResponse[Any]) -> list[AgentResp | |
|
|
||
|
|
||
| def _tool_names(context: AgentContext) -> list[str]: | ||
| """Project the registered tool names for ``agent_startup`` (spec ``tools_registered``).""" | ||
| from ._tools import _get_tool_name, normalize_tools # type: ignore[reportPrivateUsage] | ||
| """Project the registered tool names for ``agent_startup`` (spec ``tools_registered``). | ||
|
|
||
| The projection mirrors run preparation: constructor tools, agent/run options tools, | ||
| and the run-level tool overrides are all combined, so a run that supplies extra | ||
| tools still reports the agent's configured tools alongside them. | ||
| """ | ||
| from ._tools import _get_tool_name, normalize_tools # pyright: ignore[reportPrivateUsage] | ||
|
|
||
| merged: list[Any] = [] | ||
|
|
||
| def _extend(source: Any) -> None: | ||
| if source is None: | ||
| return | ||
| try: | ||
| merged.extend(normalize_tools(source)) | ||
| except Exception: | ||
| logger.warning( | ||
| "agent-hooks could not normalize the run's tools for the agent_startup projection." | ||
| ) | ||
|
|
||
| agent = context.agent | ||
| _extend(getattr(agent, "tools", None)) | ||
| default_options = getattr(agent, "default_options", None) | ||
| if isinstance(default_options, Mapping): | ||
| _extend(cast(Mapping[str, Any], default_options).get("tools")) | ||
| options = context.options | ||
| if isinstance(options, Mapping): | ||
| _extend(cast(Mapping[str, Any], options).get("tools")) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we avoid consuming |
||
| _extend(context.tools) | ||
|
|
||
| tools: Any = context.tools if context.tools is not None else getattr(context.agent, "tools", None) | ||
| if tools is None: | ||
| return [] | ||
| try: | ||
| normalized = normalize_tools(tools) | ||
| except Exception: | ||
| logger.warning("agent-hooks could not normalize the run's tools for the agent_startup projection.") | ||
| return [] | ||
| names: list[str] = [] | ||
| for item in normalized: | ||
| seen: set[str] = set() | ||
| for item in merged: | ||
| name = _get_tool_name(item) | ||
| names.append(name if name else type(item).__name__) | ||
| label = name if name else type(item).__name__ | ||
| if label not in seen: | ||
| seen.add(label) | ||
| names.append(label) | ||
| return names | ||
|
|
||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would make sense for the source selection behind
tools_registeredto reuse the same helper as_prepare_run_context. This block independently combinesagent.tools,default_options["tools"],options["tools"], andcontext.tools, while run preparation has separate precedence and uniqueness rules. A call supplying bothtools=andoptions["tools"]reports all sources at startup even though the prepared model options follow a different path. Extracting one_resolve_run_tools(...)helper for both places would keep the audit snapshot aligned.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, in general this handling of tools should be done once across the stack, not once here in this hook and again later on!