Skip to content

fix(models): report truncated Apigee tool calls as max tokens - #7686

Open
n1shanthb wants to merge 1 commit into
google:mainfrom
n1shanthb:fix/apigee-length-tool-call-7648
Open

n1shanthb wants to merge 1 commit into
google:mainfrom
n1shanthb:fix/apigee-length-tool-call-7648

Conversation

@n1shanthb

Copy link
Copy Markdown

Link to Issue or Description of Change

Closes #7648
Related: #7645

Problem:

When an Apigee Chat Completions stream ends with finish_reason="length", the response handler attempts to parse accumulated tool-call arguments before handling the provider’s finish reason. Truncated JSON can therefore raise ValueError, preventing a terminal MAX_TOKENS response from reaching the caller.

Additionally, the existing argument-merging fallback can discard an incomplete fragment following a complete JSON object and expose a function call whose arguments were truncated.

Solution:

Validate accumulated tool-call arguments before finalizing responses that terminate with length.

The change:

  • Returns a terminal MAX_TOKENS error without final executable function calls if any call is incomplete.
  • Discards the entire final tool-call batch when one call is incomplete.
  • Treats empty arguments as incomplete while preserving explicit {}.
  • Preserves valid completed calls and existing complete-object-per-delta merging.
  • Preserves accompanying text and response metadata.
  • Counts token usage once when text and the terminal error are emitted separately.
  • Leaves normal partial streaming updates and other finish reasons unchanged.

The implementation is limited to the Apigee response handler and its regression tests.

Testing Plan

Unit Tests:

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

Regression coverage includes:

  • Truncated, missing, empty, and whitespace-only arguments.
  • Explicit {} and valid fragmented JSON.
  • Complete-object-per-delta fallback compatibility.
  • Complete JSON followed by an incomplete fragment, and the reverse order.
  • Sparse, interleaved tool calls with whole-batch rejection.
  • Unchanged handling of non-length finish reasons.
  • Mixed text, response metadata, and token-usage accounting.
  • Mocked SSE through generate_content_async(stream=True), with termination in the same chunk or a subsequent chunk.

Validation matrix

Validation performed on Ubuntu under WSL 2.

Check Python Passed Failed Skipped Xfailed Xpassed Result
Focused regression suite 3.11.17 1,006 0 0 0 0 PASS
Full unit suite 3.11.17 20,861 0 94 18 2 PASS
Full unit suite 3.12.15 20,852 0 95 18 2 PASS
Full unit suite 3.13.17 20,852 0 95 18 2 PASS
Full unit suite 3.14.4 20,852 1 94 18 2 FAIL
Unpatched baseline full suite 3.14.4 20,821 1 94 18 2 FAIL
A2A SDK 0.3.26 compatibility Each of 3.11–3.14 1,051 0 29 0 0 PASS
MCP 2.3.0 compatibility Each of 3.11–3.14 618 0 0 0 0 PASS
Latest main + this patch + #7645: focused and Runner tests 3.11 1,239 0 0 0 0 PASS

xfailed denotes expected failures, separate from the Failed column. xpassed denotes tests marked as expected failures that passed.

Python 3.14 full-suite validation: 20,852 passed, 1 failed, 94 skipped, 18 xfailed, 2 xpassed.

The sole failure, test_sequential_tools_add_up, measures elapsed time against a narrow tolerance. It failed in two full-suite runs, including a rerun without other test jobs, and reproduced on the unpatched baseline. The test passed when run individually (1 passed, exit 0).

The full Python 3.14 suite therefore did not pass. No patch-specific regression was identified from this failure.

Full suites used:

python -m pytest tests/unittests -n 4 -q
Additional check Result Notes
Configured Linux pre-commit run --all-files PASS Optional addlicense hook used its configured skip behavior
Comparative all-extras CI mypy on Linux PASS Python 3.11–3.13: 268 baseline errors, 268 patched errors, 0 new normalized errors
Live Apigee manual E2E NOT RUN Automated mocked SSE integration coverage passed

Manual End-to-End (E2E) Tests:

No live Apigee manual E2E test was performed.

Automated integration tests use httpx.MockTransport to supply SSE responses through ApigeeLlm.generate_content_async(stream=True). They verify that truncated arguments produce terminal MAX_TOKENS errors without final executable function calls, while preserving text and metadata and counting token usage once.

To reproduce the mocked streaming integration tests:

uv sync --extra test --no-install-package lancedb
uv run pytest tests/unittests/models/test_completions_http_client.py \
  -k apigee_streaming_truncation_with_mocked_sse -q

The lancedb installation exclusion follows the repository’s CI configuration.

Checklist

  • I have read the [CONTRIBUTING.md] document.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added tests that prove my fix is effective.
  • New and existing unit tests pass locally with my changes.
  • I have manually tested my changes end-to-end.
  • No dependent downstream changes are required.

Additional context

PR #7645 addresses missing-index accumulation in the same handler. Both patches applied cleanly together on upstream main at fb197fa. Combined focused and Runner validation passed: 1,239 tests. The complete multi-version suite was not rerun on that combined checkout.

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 raises ValueError instead of reporting MAX_TOKENS when streamed tool arguments are cut off

2 participants