Skip to content

feat(core): normalize nullable unions in Google tool schemas#8667

Open
SunmiJJW wants to merge 2 commits into
AstrBotDevs:masterfrom
SunmiJJW:feat/google-schema-nullable-unions
Open

feat(core): normalize nullable unions in Google tool schemas#8667
SunmiJJW wants to merge 2 commits into
AstrBotDevs:masterfrom
SunmiJJW:feat/google-schema-nullable-unions

Conversation

@SunmiJJW

@SunmiJJW SunmiJJW commented Jun 8, 2026

Copy link
Copy Markdown

Modifications / 改动点

This PR improves Google/Gemini tool schema conversion for common nullable JSON Schema unions.

本 PR 改进 Google/Gemini 工具 schema 转换,对常见 nullable JSON Schema union 做兼容归一化。

  • Collapse nullable anyOf / oneOf forms such as [{type: string}, {type: null}] into a typed schema with nullable: true.
  • Mark type: ["string", "null"] schemas as nullable: true while keeping the selected non-null type.
  • Preserve supported metadata such as description and enum.
  • Avoid forwarding unsupported default values in the converted Google schema.
  • Add focused regression tests in tests/unit/test_tool_google_schema.py.
  • This is NOT a breaking change. / 这不是一个破坏性变更。

Screenshots or Test Results / 运行截图或测试结果

Verification:

python -m pytest tests\unit\test_tool_google_schema.py -q
...                                                                      [100%]
3 passed in 0.51s

python -m ruff check astrbot\core\agent\tool.py tests\unit\test_tool_google_schema.py
All checks passed!

python -m ruff format --check astrbot\core\agent\tool.py tests\unit\test_tool_google_schema.py
2 files already formatted

git diff --check
# no whitespace errors

Smoke output for a nullable recency enum:

{'type': 'string', 'enum': ['day', 'week', 'month', 'year'], 'nullable': True, 'description': 'Optional recency filter'}

Duplicate check:

gh pr list --repo AstrBotDevs/AstrBot --state open --limit 100 --json number,title,files,url
# One open PR (#8617) also touches astrbot/core/agent/tool.py, but only adds a FunctionTool permission field.
# It does not modify ToolSet.google_schema() or tests/unit/test_tool_google_schema.py.

Checklist / 检查清单

  • 😊 No user-facing feature is added; this checklist item is N/A.
    / 未新增面向用户的功能;此项不适用。

  • 👀 My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
    / 我的更改经过了良好的测试,并已在上方提供了“验证步骤”和“运行截图”

  • 🤓 I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in requirements.txt and pyproject.toml.
    / 我确保没有引入新依赖库,或者引入了新依赖库的同时将其添加到 requirements.txtpyproject.toml 文件相应位置。

  • 😮 My changes do not introduce malicious code.
    / 我的更改没有引入恶意代码。

Summary by Sourcery

Normalize nullable union types when converting JSON Schemas to Google/Gemini tool schemas.

Enhancements:

  • Collapse nullable anyOf/oneOf unions that combine a single non-null type with null into a nullable typed schema while preserving supported metadata.
  • Treat schemas with type lists that include "null" as nullable while keeping the non-null type in the converted schema.
  • Centralize and reuse logic for propagating only supported fields from source schemas into converted Google schemas, avoiding unsupported defaults.

Tests:

  • Add regression tests covering nullable anyOf unions and type lists including null in Google tool schema conversion.

@dosubot dosubot Bot added size:M This PR changes 30-99 lines, ignoring generated files. area:core The bug / feature is about astrbot's core, backend labels Jun 8, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request enhances the Google schema conversion logic in astrbot/core/agent/tool.py to handle anyOf and oneOf unions by collapsing them into a single nullable type when they contain only one non-null branch. It also marks list types containing "null" as nullable and adds corresponding unit tests. The reviewer suggested a code improvement to simplify the union collapsing logic and support single-element unions without null branches.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread astrbot/core/agent/tool.py Outdated
Comment on lines +283 to +289
if len(non_null_branches) == 1 and len(non_null_branches) < len(
converted_branches
):
result = non_null_branches[0].copy()
result["nullable"] = True
apply_supported_fields(result, schema)
return result

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

We can simplify the condition and also support collapsing single-element anyOf/oneOf arrays (e.g., [{"type": "string"}]) by checking len(non_null_branches) == 1 and conditionally setting nullable only if there are other (null) branches in the union. Since Gemini does not support anyOf/oneOf at all, collapsing single-element unions to their underlying type is always a safe and beneficial normalization.

Suggested change
if len(non_null_branches) == 1 and len(non_null_branches) < len(
converted_branches
):
result = non_null_branches[0].copy()
result["nullable"] = True
apply_supported_fields(result, schema)
return result
if len(non_null_branches) == 1:
result = non_null_branches[0].copy()
if len(converted_branches) > 1:
result["nullable"] = True
apply_supported_fields(result, schema)
return result

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've left some high level feedback:

  • In the anyOf/oneOf handling, non-dict branches are silently dropped (if isinstance(item, dict)); if the source schema ever includes literal values or refs there, you may want to preserve those branches unchanged rather than omitting them to avoid losing parts of the schema.
  • The new nullable-collapse logic only treats branches with type == "null" as the null case; if you expect schemas that encode nullability differently (e.g., via enum: [null, ...] or $ref to a null type), consider whether those should also be normalized for consistency.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In the anyOf/oneOf handling, non-dict branches are silently dropped (`if isinstance(item, dict)`); if the source schema ever includes literal values or refs there, you may want to preserve those branches unchanged rather than omitting them to avoid losing parts of the schema.
- The new nullable-collapse logic only treats branches with `type == "null"` as the null case; if you expect schemas that encode nullability differently (e.g., via `enum: [null, ...]` or `$ref` to a null type), consider whether those should also be normalized for consistency.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:core The bug / feature is about astrbot's core, backend size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant