Skip to content

fix(models): reject non-object Apigee tool-call arguments - #7774

Open
n1shanthb wants to merge 1 commit into
google:mainfrom
n1shanthb:fix/apigee-nonobject-tool-args-7740
Open

n1shanthb wants to merge 1 commit into
google:mainfrom
n1shanthb:fix/apigee-nonobject-tool-args-7740

Conversation

@n1shanthb

Copy link
Copy Markdown

Problem:

Apigee Chat Completions tool-call arguments containing valid non-object JSON could raise exceptions or be silently converted into dictionaries in non-streaming mode. Streaming could expose those values directly as FunctionCall.args.

For example, JSON values such as [1, 2], "x", 5, and null were handled inconsistently between streaming and non-streaming responses.

Solution:

Validate completed tool-call arguments as JSON objects before assigning them to FunctionCall.args.

  • Reject invalid completed calls individually and report MALFORMED_FUNCTION_CALL.
  • Preserve valid sibling calls, text, usage, and response metadata.
  • Preserve valid JSON objects, malformed-JSON exception behavior, and partial streaming.
  • Retain provider MAX_TOKENS and SAFETY finish-reason precedence.
  • Apply the same validation to legacy non-streaming function calls.
  • Prevent silent coercion of non-object arguments into empty or otherwise unintended dictionaries.

The per-call rejection policy follows existing LiteLLM behavior. Unlike coercing invalid arguments to {}, it prevents rejected calls from executing with missing arguments or defaults.

Testing Plan

Unit Tests:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.

Focused regression testing:

The new regression suite covers non-object JSON values, valid JSON objects, streaming and non-streaming paths, finish-reason precedence, valid sibling preservation, legacy function calls, fragmented arguments, metadata preservation, and downstream Runner execution.

Results:

  • Original implementation: 77 failed, 20 passed, 0 skipped, 28 deselected.
  • Patched implementation: 97 passed, 0 failed, 0 skipped, 28 deselected.
  • Broader relevant model, parser, Event, and tool-flow suites: 1,228 passed, 0 failed.

The 77 failures against unchanged upstream demonstrate that the new regression tests detect the original behavior.

Full multi-version tox validation:

The repository's configured tox matrix was executed using Python 3.11 through 3.14.

Python Passed Failed Skipped
3.11.17 21,608 1 97
3.12.15 21,599 1 98
3.13.17 21,599 1 98
3.14.4 21,600 2 96

Each completed run additionally reported:

  • 12 xfailed
  • 2 xpassed
  • 0 deselected

The full tox matrix completed but did not pass.

Failure investigation:

  1. A2A custom-factory failure (Python 3.11–3.14)

    All four versions failed:

    tests/unittests/a2a/agent/test_remote_a2a_agent.py::TestRemoteA2aAgentResolution::test_ensure_resolved_uses_custom_factory

    The failure is an AttributeError involving CustomFactory._httpx_client.

    Targeted comparisons reproduced the same failure on unchanged upstream using identical dependencies. The resolved a2a-sdk version is 1.3.0, where the relevant client is stored under _config.httpx_client.

    No A2A source or test files are modified by this PR.

  2. Instrumentation timing failure (Python 3.14 only)

    The full suite additionally failed:

    test_sequential_tools_add_up

    The measured time was approximately 0.843 seconds, compared with the expected 0.20 ± 0.05 seconds.

    The test passed in isolated runs against both patched and baseline source. However, its full-suite failure remains unresolved and is not claimed to be baseline-only.

Python 3.14 completed using an external diagnostic-only pytest logger. No tests were filtered or excluded.

Additional validation:

  • Ruff passed.
  • Pyink formatting checks passed.
  • isort passed.
  • Repository compliance checks passed.
  • Scoped mypy passed with no reported issues.
  • git diff --check passed.
  • The configured pre-commit checks passed in a staged Linux snapshot.

Compatibility testing:

The patch was also tested in disposable integration snapshots:

These snapshots required documented merge-conflict resolutions, including preservation of #7686's incomplete-argument handling and finish-reason precedence.

These results demonstrate compatibility in the resolved test combinations, not conflict-free application of the patches.

Manual End-to-End (E2E) Tests

No live Apigee/manual E2E testing was performed.

Automated integration tests use synthetic Chat Completions responses with real local Runner and FunctionTool execution.

Under the tested default Runner configuration, these tests verify that:

  • Valid sibling calls remain available for execution.
  • Retained valid sibling calls execute successfully.
  • Rejected malformed calls never execute.
  • The structured malformed-function-call error remains observable.

Configured callbacks, plugins, or retry policies may alter downstream execution behavior.

To reproduce the relevant Runner tests:

PYTHONPATH=src python -m pytest \
  tests/unittests/models/test_completions_http_client.py \
  -k valid_sibling_executes_with_malformed_response -q

Checklist

  • I have read the CONTRIBUTING.md document.
  • I have performed an automated self-review of the patch.
  • I have commented non-obvious validation and fallback behavior.
  • I have added tests that demonstrate the fix.
  • New and existing unit tests all pass locally.
  • I have manually tested my changes end-to-end.
  • Any dependent changes have been merged and published in downstream modules.

The downstream-dependency item is not applicable: no downstream dependency changes are required.

Additional Context

Alternative implementation:

Open PR #7763 (addresses the same issue by coercing non-object JSON arguments to an empty dictionary.

This PR takes a more defensive approach by rejecting invalid completed arguments individually with MALFORMED_FUNCTION_CALL, consistent with the existing LiteLLM convention.

In addition to preserving valid sibling calls, explicit rejection avoids silently substituting {} for malformed arguments, which could otherwise allow tools to execute with unintended default parameters. This provides a clearer validation boundary between model-generated arguments and tool execution.

Both approaches address the original crash and type-consistency issue. Maintainer guidance on the preferred behavior is welcome.

AI Assistance:

OpenAI Codex (GPT-6.1 sol) assisted in the implementation and regression tests and performed automated review and validation.

Acknowledgments:

Thanks to @surajksharma07 for identifying the related argument-handling issue during the #7686 discussion and clarifying the need for consistent behavior across streaming and non-streaming paths.

Thanks to @Somay-kousis for reporting #7740 and providing the detailed reproduction.

Their feedback helped inform the investigation and the scope of this fix.

Validate completed Chat Completions arguments before assigning them to
FunctionCall.args. Drop invalid calls individually and report a structured
MALFORMED_FUNCTION_CALL error while preserving valid siblings, text, and
metadata. Preserve malformed-JSON exceptions and partial streaming, with
provider MAX_TOKENS and SAFETY reasons retaining precedence.

Fixes google#7740

AI assistance: OpenAI Codex generated the implementation and regression
tests and performed automated review and validation.
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.

ApigeeLlm: tool call arguments that are valid JSON but not an object crash the non-streamed path and pass through as args when streamed

2 participants