Skip to content

fix: normalize tool arguments using parameter schemas#7980

Closed
whatevertogo wants to merge 1 commit intoAstrBotDevs:masterfrom
whatevertogo:codex/global-tool-arg-normalization
Closed

fix: normalize tool arguments using parameter schemas#7980
whatevertogo wants to merge 1 commit intoAstrBotDevs:masterfrom
whatevertogo:codex/global-tool-arg-normalization

Conversation

@whatevertogo
Copy link
Copy Markdown
Contributor

Draft discussion PR for implementing a second-stage fix after #7961.

The goal is to centralize tool argument normalization so provider/model outputs like JSON-stringified arrays or objects can be recovered before tool execution, without adding one-off parsing blocks inside individual tools.

Proposed direction

  • Add a schema-guided normalizer for tool arguments.
  • Coerce string values only when the tool parameter schema expects array or object.
  • Support nested properties, items, anyOf, and oneOf conservatively.
  • Do not parse parameters whose schema type is string, even if the string looks like JSON.
  • Keep invalid JSON strings unchanged so existing tool validation still reports the error.

Related cleanup

After the global path exists, the local send_message_to_user.messages JSON-string parsing workaround from #7978 can be removed and covered by shared normalizer tests.

Scope notes

This should address issues similar to #7961 where the expected structured argument is delivered as a JSON string. It is not intended to solve unrelated tool-call problems such as duplicated tool names from streamed chunks or cases where a model already emitted {} and the original content is lost.

Current status

This PR currently contains only an empty anchor commit so we can discuss the implementation shape before pushing code.

Validation plan

  • Unit tests for the normalizer:
    • array field from JSON string
    • object field from JSON string
    • nested object/list coercion
    • anyOf object/array coercion
    • string fields are not parsed
    • malformed JSON remains unchanged
  • Integration-style test for send_message_to_user.messages going through the common normalization path.
  • Existing targeted tests:
    • uv run pytest tests/unit/test_message_tools.py
    • targeted new normalizer tests
    • uv run ruff check ...
    • uv run ruff format --check ...

Open a dedicated discussion branch for centralizing tool argument normalization before implementation, so follow-up work can stay separate from the AstrBotDevs#7961 local fix.

Constraint: Keep this branch based on upstream/master and avoid mixing in PR AstrBotDevs#7978 changes.
Rejected: Stacking this discussion PR on the AstrBotDevs#7978 branch because it would make review diffs noisy.
Directive: Implement only schema-guided coercion, not blind recursive JSON parsing.
Confidence: medium
Scope-risk: medium
Reversibility: Drop this empty anchor commit if the design direction changes.
Tested: not run; discussion anchor only
Not-tested: implementation behavior because no code changed yet
Related: AstrBotDevs#7961
Co-authored-by: OmX <omx@users.noreply.github.com>
@gemini-code-assist
Copy link
Copy Markdown
Contributor

Note

Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported.

@whatevertogo
Copy link
Copy Markdown
Contributor Author

Closing this draft for now. The scoped #7961 fix remains in #7978, and the broader global tool-argument normalization does not look worth pursuing as a separate PR at this point.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant