Skip to content

Commit b6b63e1

Browse files
committed
Merge remote-tracking branch 'origin/main' into fix-thought-signature-pruning
2 parents 29b0970 + ecef5f8 commit b6b63e1

2 files changed

Lines changed: 244 additions & 3 deletions

File tree

src/google/adk/plugins/bigquery_agent_analytics_plugin.py

Lines changed: 89 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -306,6 +306,84 @@ def _get_tool_origin(
306306
return "UNKNOWN"
307307

308308

309+
def _extract_tool_declarations(
310+
tools_dict: dict[str, "BaseTool"],
311+
) -> list[dict[str, Any]]:
312+
"""Extracts structured tool metadata for the ``LLM_REQUEST`` event.
313+
314+
Earlier versions logged only the tool names (``list(tools_dict.keys())``).
315+
Downstream consumers such as online evaluation need the tool *description* and
316+
*parameter schema* to judge whether the model selected and invoked the right
317+
tool, so this returns one structured entry per tool instead of a bare name.
318+
319+
Each entry always carries ``name`` and, when available, ``description`` and
320+
``parameters`` (the OpenAPI parameter schema from the tool's
321+
``FunctionDeclaration``). Extraction is best-effort and per-tool: a tool whose
322+
declaration cannot be resolved still contributes its name and description, so
323+
one misbehaving tool never drops the whole ``tools`` attribute.
324+
325+
Args:
326+
tools_dict: Mapping of tool name to ``BaseTool`` from ``LlmRequest``.
327+
328+
Returns:
329+
A list of ``{"name", "description"?, "parameters"?}`` dicts.
330+
"""
331+
tools: list[dict[str, Any]] = []
332+
for name, tool in tools_dict.items():
333+
# Fall back to the dict key when the tool has no (or a falsy) name.
334+
entry: dict[str, Any] = {"name": getattr(tool, "name", None) or name}
335+
description = getattr(tool, "description", None)
336+
if description:
337+
entry["description"] = description
338+
339+
# The parameter schema lives on the tool's FunctionDeclaration, which some
340+
# tools (e.g. built-in tools) do not provide. Resolve defensively so a
341+
# single failing tool does not discard the whole tools list.
342+
#
343+
# Note: FunctionTool._get_declaration() rebuilds the declaration from the
344+
# function signature on each call (no caching), so this repeats work the
345+
# framework already did when assembling the request. Acceptable for typical
346+
# toolsets; revisit with a cache if it shows up on the hot path.
347+
declaration = None
348+
try:
349+
get_declaration = getattr(tool, "_get_declaration", None)
350+
if callable(get_declaration):
351+
declaration = get_declaration()
352+
except Exception: # pylint: disable=broad-except
353+
logger.debug("Failed to get declaration for tool %s", name, exc_info=True)
354+
355+
if declaration is not None:
356+
if "description" not in entry:
357+
decl_description = getattr(declaration, "description", None)
358+
if decl_description:
359+
entry["description"] = decl_description
360+
# A declaration carries its parameter schema in one of two shapes: the
361+
# structured `parameters` Schema, or a raw JSON-schema dict in
362+
# `parameters_json_schema`. Several tools (MCP, OpenAPI, skill, node, and
363+
# environment tools) populate only the latter, and model adapters prefer
364+
# it, so prefer it here too and fall back to `parameters` otherwise.
365+
json_schema = getattr(declaration, "parameters_json_schema", None)
366+
if json_schema is not None:
367+
entry["parameters"] = json_schema
368+
else:
369+
parameters = getattr(declaration, "parameters", None)
370+
if parameters is not None:
371+
try:
372+
entry["parameters"] = parameters.model_dump(
373+
exclude_none=True, mode="json"
374+
)
375+
except Exception: # pylint: disable=broad-except
376+
# Leave parameters off if the schema is not JSON-serializable.
377+
logger.debug(
378+
"Failed to serialize parameters for tool %s",
379+
name,
380+
exc_info=True,
381+
)
382+
383+
tools.append(entry)
384+
return tools
385+
386+
309387
_SENSITIVE_KEYS = frozenset({
310388
"client_secret",
311389
"access_token",
@@ -4028,7 +4106,8 @@ async def before_model_callback(
40284106
"""
40294107

40304108
# 5. Attributes (Config & Tools)
4031-
attributes = {}
4109+
attributes: dict[str, Any] = {}
4110+
tools_truncated = False
40324111
if llm_request.config:
40334112
config_dict = {}
40344113
for field_name in [
@@ -4057,13 +4136,21 @@ async def before_model_callback(
40574136
attributes["labels"] = labels
40584137

40594138
if hasattr(llm_request, "tools_dict") and llm_request.tools_dict:
4060-
attributes["tools"] = list(llm_request.tools_dict.keys())
4139+
# Route tool declarations through the shared safety pipeline so unbounded
4140+
# descriptions / parameter schemas are size-capped and sensitive keys are
4141+
# redacted, consistent with every other captured attribute.
4142+
tools, tools_truncated = _recursive_smart_truncate(
4143+
_extract_tool_declarations(llm_request.tools_dict),
4144+
self.config.max_content_length,
4145+
)
4146+
attributes["tools"] = tools
40614147

40624148
TraceManager.push_span(callback_context, "llm_request")
40634149
await self._log_event(
40644150
"LLM_REQUEST",
40654151
callback_context,
40664152
raw_content=llm_request,
4153+
is_truncated=tools_truncated,
40674154
event_data=EventData(
40684155
model=llm_request.model,
40694156
extra_attributes=attributes,

tests/unittests/plugins/test_bigquery_agent_analytics_plugin.py

Lines changed: 155 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1451,7 +1451,161 @@ async def test_before_model_callback_with_params_and_tools(
14511451
assert attributes["llm_config"]["temperature"] == 0.5
14521452
assert attributes["llm_config"]["top_p"] == 0.9
14531453
assert attributes["llm_config"]["top_p"] == 0.9
1454-
assert attributes["tools"] == ["tool1", "tool2"]
1454+
# Tools without a name/description/declaration fall back to just the key.
1455+
assert attributes["tools"] == [{"name": "tool1"}, {"name": "tool2"}]
1456+
1457+
@pytest.mark.asyncio
1458+
async def test_before_model_callback_logs_tool_declarations(
1459+
self,
1460+
bq_plugin_inst,
1461+
mock_write_client,
1462+
callback_context,
1463+
dummy_arrow_schema,
1464+
):
1465+
"""LLM_REQUEST tools carry name, description, and parameter schema."""
1466+
1467+
class _FakeTool(base_tool_lib.BaseTool):
1468+
1469+
def __init__(self, name, description, declaration):
1470+
super().__init__(name=name, description=description)
1471+
self._declaration = declaration
1472+
1473+
def _get_declaration(self):
1474+
return self._declaration
1475+
1476+
execute_sql = _FakeTool(
1477+
name="execute_sql",
1478+
description="Run a SQL query against BigQuery.",
1479+
declaration=types.FunctionDeclaration(
1480+
name="execute_sql",
1481+
description="Run a SQL query against BigQuery.",
1482+
parameters=types.Schema(
1483+
type=types.Type.OBJECT,
1484+
properties={
1485+
"query": types.Schema(
1486+
type=types.Type.STRING,
1487+
description="The SQL query to run.",
1488+
)
1489+
},
1490+
required=["query"],
1491+
),
1492+
),
1493+
)
1494+
# A tool without a declaration still contributes name + description.
1495+
list_datasets = _FakeTool(
1496+
name="list_dataset_ids",
1497+
description="List available datasets.",
1498+
declaration=None,
1499+
)
1500+
1501+
llm_request = llm_request_lib.LlmRequest(
1502+
model="gemini-pro",
1503+
contents=[types.Content(role="user", parts=[types.Part(text="hi")])],
1504+
)
1505+
llm_request.tools_dict = {
1506+
"execute_sql": execute_sql,
1507+
"list_dataset_ids": list_datasets,
1508+
}
1509+
bigquery_agent_analytics_plugin.TraceManager.push_span(callback_context)
1510+
await bq_plugin_inst.before_model_callback(
1511+
callback_context=callback_context, llm_request=llm_request
1512+
)
1513+
await asyncio.sleep(0.01)
1514+
log_entry = await _get_captured_event_dict_async(
1515+
mock_write_client, dummy_arrow_schema
1516+
)
1517+
_assert_common_fields(log_entry, "LLM_REQUEST")
1518+
attributes = json.loads(log_entry["attributes"])
1519+
tools_by_name = {t["name"]: t for t in attributes["tools"]}
1520+
1521+
assert tools_by_name["execute_sql"]["description"] == (
1522+
"Run a SQL query against BigQuery."
1523+
)
1524+
params = tools_by_name["execute_sql"]["parameters"]
1525+
assert params["type"] == "OBJECT"
1526+
assert params["properties"]["query"]["type"] == "STRING"
1527+
assert params["required"] == ["query"]
1528+
1529+
assert tools_by_name["list_dataset_ids"]["description"] == (
1530+
"List available datasets."
1531+
)
1532+
assert "parameters" not in tools_by_name["list_dataset_ids"]
1533+
1534+
def test_extract_tool_declarations_declaration_error_is_isolated(self):
1535+
"""A tool whose _get_declaration raises still yields name + description."""
1536+
1537+
class _RaisingTool(base_tool_lib.BaseTool):
1538+
1539+
def _get_declaration(self):
1540+
raise ValueError("boom")
1541+
1542+
class _OkTool(base_tool_lib.BaseTool):
1543+
1544+
def _get_declaration(self):
1545+
return None
1546+
1547+
result = bigquery_agent_analytics_plugin._extract_tool_declarations({
1548+
"raiser": _RaisingTool(name="raiser", description="Raises."),
1549+
"ok": _OkTool(name="ok", description="Fine."),
1550+
})
1551+
by_name = {t["name"]: t for t in result}
1552+
1553+
# The raising tool is not dropped; other tools are unaffected.
1554+
assert by_name["raiser"] == {"name": "raiser", "description": "Raises."}
1555+
assert by_name["ok"] == {"name": "ok", "description": "Fine."}
1556+
1557+
def test_extract_tool_declarations_parameters_serialization_error(self):
1558+
"""A parameters object that fails to serialize is dropped, not fatal."""
1559+
1560+
class _BadParams:
1561+
1562+
def model_dump(self, *args, **kwargs):
1563+
raise ValueError("cannot serialize")
1564+
1565+
class _BadDecl:
1566+
description = None
1567+
parameters = _BadParams()
1568+
1569+
class _BadParamTool(base_tool_lib.BaseTool):
1570+
1571+
def _get_declaration(self):
1572+
return _BadDecl()
1573+
1574+
result = bigquery_agent_analytics_plugin._extract_tool_declarations(
1575+
{"bad_params": _BadParamTool(name="bad_params", description="Bad.")}
1576+
)
1577+
1578+
# Name + description survive; the unserializable parameters key is omitted.
1579+
assert result == [{"name": "bad_params", "description": "Bad."}]
1580+
1581+
def test_extract_tool_declarations_uses_parameters_json_schema(self):
1582+
"""Declarations exposing parameters_json_schema log that raw schema."""
1583+
1584+
json_schema = {
1585+
"type": "object",
1586+
"properties": {"path": {"type": "string"}},
1587+
"required": ["path"],
1588+
}
1589+
1590+
class _JsonSchemaTool(base_tool_lib.BaseTool):
1591+
1592+
def _get_declaration(self):
1593+
return types.FunctionDeclaration(
1594+
name="read_file",
1595+
description="Read a file.",
1596+
parameters_json_schema=json_schema,
1597+
)
1598+
1599+
result = bigquery_agent_analytics_plugin._extract_tool_declarations(
1600+
{"read_file": _JsonSchemaTool(name="read_file", description="Read.")}
1601+
)
1602+
1603+
# parameters_json_schema is logged verbatim (preferred over `parameters`).
1604+
assert result == [{
1605+
"name": "read_file",
1606+
"description": "Read.",
1607+
"parameters": json_schema,
1608+
}]
14551609

14561610
@pytest.mark.asyncio
14571611
async def test_before_model_callback_with_full_config(

0 commit comments

Comments
 (0)