Skip to content

feat(llm): support Z.ai structured output - #372

Merged
yyiilluu merged 1 commit into
mainfrom
codex/support-glm
Jul 21, 2026
Merged

feat(llm): support Z.ai structured output#372
yyiilluu merged 1 commit into
mainfrom
codex/support-glm

Conversation

@yyiilluu

@yyiilluu yyiilluu commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Make Z.ai/GLM viable for Reflexio's structured roles by using the provider's supported JSON-object transport with an explicit schema instruction and local Pydantic validation.
  • Default zai/* completions to the Z.ai coding API while preserving custom-endpoint and per-call overrides.
  • Enable the live-verified zai/glm-5.2 extraction-agent tool loop despite LiteLLM 1.82.2's stale capability result.
  • Reject structured-output fallback ladders whose providers require incompatible wire formats instead of silently reusing an invalid primary request.

Changes

Z.ai request strategy

  • Add prompt-backed structured-output strategy selection for Z.ai.
  • Preserve response_format={"type":"json_object"} through LiteLLM's drop_params=True behavior.
  • Keep tool turns unconstrained and require schema-conforming JSON only on the terminal turn.
  • Treat empty structured content as a retryable parse failure.

Endpoint and capability handling

  • Use https://api.z.ai/api/coding/paas/v4 as the built-in zai/* completion endpoint.
  • Add an exact zai/glm-5.2 function-calling override backed by dependent live tool-call evidence.

Safety and regression coverage

  • Preserve provider-safe schema validation on the prompt-backed path.
  • Fail closed for mixed structured-output fallback strategies while leaving text fallback unchanged.
  • Cover endpoint precedence, request shaping, caller-message immutability, tool turns, fallback compatibility, empty content, and the exact capability override.

Test Plan

  • uv run ruff check on changed Python sources and tests
  • uv run pyright on changed LLM sources and tests
  • uv run pytest -q --no-cov tests/server/llm/test_litellm_client_unit.py tests/server/llm/test_tools.py -k 'SupportsToolCallingOverrides or MaybeParseStructuredOutput or StrictStructuredOutputRequest' — 39 passed
  • Live Reflexio-client probe against zai/glm-5.2, omitting --base-url — single tool call and dependent get_weather -> convert_temperature -> parsed terminus sequence both passed

Summary by CodeRabbit

  • New Features

    • Added support for Z.ai structured-output requests, including automatic endpoint handling and JSON-object responses.
    • Improved structured-output handling with provider-specific schema instructions and validation.
    • Added support for selected models with native tool calling.
  • Bug Fixes

    • Empty structured responses now raise a clear parsing error.
    • Fallback models must use compatible structured-output strategies.

Use the coding API by default, preserve JSON-object mode through LiteLLM, and enable the live-verified GLM-5.2 extraction tool loop while rejecting incompatible structured fallback ladders.
@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Changes

Adds Z.ai coding-endpoint routing and structured-output transport handling, including prompt-based JSON schema instructions, fallback strategy validation, stricter empty-response parsing, and exact tool-calling overrides. Documentation and unit tests cover the new behavior.

Z.ai structured-output support

Layer / File(s) Summary
Structured-output strategy and prompt directives
reflexio/server/llm/_litellm_structured_output.py, reflexio/server/llm/llm_utils.py, tests/server/llm/test_litellm_client_unit.py
Structured-output transport is selected between passthrough, prompt JSON-object, and native JSON-schema modes; schema instructions are generated and empty content raises StructuredOutputParseError.
Z.ai completion transport and fallback routing
reflexio/server/llm/_litellm_text_generation.py, reflexio/server/README.md, tests/server/llm/test_litellm_client_unit.py
Z.ai models use the coding endpoint by default, structured requests inject schema instructions and configure response formats, explicit endpoints are preserved, and incompatible fallbacks are rejected.
Z.ai tool-calling overrides
reflexio/server/llm/tools.py, tests/server/llm/test_tools.py
Exact model-name overrides allow zai/glm-5.2 tool calling when LiteLLM reports unsupported capability.
Estimated code review effort: 4 (Complex) ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant _build_completion_params
  participant StructuredOutputMixin
  participant ZaiCodingEndpoint
  Client->>_build_completion_params: provide model, messages, and response schema
  _build_completion_params->>StructuredOutputMixin: select and apply transport strategy
  StructuredOutputMixin->>ZaiCodingEndpoint: route request with JSON-object format and schema instruction
  ZaiCodingEndpoint-->>Client: return structured completion
Loading

Possibly related PRs

Suggested reviewers: guangyu-reflexio, yilu331

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: adding Z.ai structured-output support in the LLM layer.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/support-glm

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
reflexio/server/llm/_litellm_text_generation.py (1)

424-441: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Z.ai’s forced api_base should not leak into cross-provider fallbacks. params["api_base"] = _ZAI_CODING_API_BASE is set once on the shared params dict, and LiteLLM plain-string fallbacks reuse that same kwargs set for every rung. A zai/* primary with an openai/* fallback will send the fallback attempt to the Z.ai endpoint instead of the target provider, defeating failover. Use per-fallback overrides or avoid forcing api_base when the ladder spans other providers.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@reflexio/server/llm/_litellm_text_generation.py` around lines 424 - 441,
Prevent the Z.ai-specific api_base in the shared params from being reused by
cross-provider fallback attempts. Update the fallback handling around
fallback_models and the shared params construction so each fallback receives its
provider-appropriate api_base, or omit the forced Z.ai api_base whenever
fallback_models includes a different provider such as openai/*. Preserve the
Z.ai endpoint for the primary and same-provider fallbacks.
🧹 Nitpick comments (1)
tests/server/llm/test_litellm_client_unit.py (1)

1056-1061: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

LGTM! Solid coverage for the new zai transport and fallback-compatibility behavior. One gap worth closing: test_text_fallback_does_not_require_matching_transport (Lines 1406-1418) only asserts the returned fallback list, not params["api_base"] — it doesn't catch the cross-provider api_base leak described in the _litellm_text_generation.py review (a zai primary forces api_base onto the whole ladder, including this exact fallback scenario).

Also applies to: 1276-1419

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/server/llm/test_litellm_client_unit.py` around lines 1056 - 1061,
Extend test_text_fallback_does_not_require_matching_transport to assert
params["api_base"] for the fallback request, ensuring a zai primary does not
leak its api_base to the text fallback ladder. Preserve the existing
fallback-list assertions and cover the expected cross-provider behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@reflexio/server/llm/_litellm_text_generation.py`:
- Around line 424-441: Prevent the Z.ai-specific api_base in the shared params
from being reused by cross-provider fallback attempts. Update the fallback
handling around fallback_models and the shared params construction so each
fallback receives its provider-appropriate api_base, or omit the forced Z.ai
api_base whenever fallback_models includes a different provider such as
openai/*. Preserve the Z.ai endpoint for the primary and same-provider
fallbacks.

---

Nitpick comments:
In `@tests/server/llm/test_litellm_client_unit.py`:
- Around line 1056-1061: Extend
test_text_fallback_does_not_require_matching_transport to assert
params["api_base"] for the fallback request, ensuring a zai primary does not
leak its api_base to the text fallback ladder. Preserve the existing
fallback-list assertions and cover the expected cross-provider behavior.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 745e691c-d50a-49a3-876c-63c07cedeae8

📥 Commits

Reviewing files that changed from the base of the PR and between bbfc4ca and 81ed179.

📒 Files selected for processing (7)
  • reflexio/server/README.md
  • reflexio/server/llm/_litellm_structured_output.py
  • reflexio/server/llm/_litellm_text_generation.py
  • reflexio/server/llm/llm_utils.py
  • reflexio/server/llm/tools.py
  • tests/server/llm/test_litellm_client_unit.py
  • tests/server/llm/test_tools.py

@yyiilluu
yyiilluu merged commit 6876318 into main Jul 21, 2026
1 check passed
yyiilluu added a commit that referenced this pull request Jul 21, 2026
## Why

`main` is red.
`test_consolidator_calls_litellm_with_fallback_configured` has been
failing since #372 landed:

```
E   ValueError: Structured-output fallback models must use the same transport
    strategy as 'gpt-test' (pydantic_passthrough); incompatible fallbacks: ['gpt-5.4-mini']
```

## Root cause

`_build_real_client_consolidator` patched the generation site var to a
fake
`"gpt-test"`, so the `LiteLLMConfig(model=...)` each test passed never
reached the
request — `PlaybookConsolidator` reads its request model from the site
var
(`deduplication_utils.py:191`), not from the client config, and passes
it as
`model=self.model_name` (`components/consolidator.py:743`).

#372 added `_validate_structured_fallback_strategies`, which resolves a
transport
strategy per model and rejects a ladder whose fallbacks disagree with
the primary.
Measured strategies:

| model | provider | strategy |
|---|---|---|
| `gpt-test` | *(unresolvable)* | `pydantic_passthrough` |
| `gpt-5.4-mini` | openai | `native_json_schema` |
| `minimax/MiniMax-M3` | minimax | `native_json_schema` |

The fake model made the pairing artificially incompatible. The guard
itself is
correct — the test was asserting against a model it never intended to
use.

## Fix

Pin the patched site var to `config.model`. The fallback-configured case
now
exercises a realistic production pairing (MiniMax primary + OpenAI
fallback, both
`native_json_schema`). The second test (`claude-code/...`, no fallbacks)
never
reaches the guard and is unaffected.

Also included: a `style:` commit applying `ruff format` to
`client/client.py`,
the only file in the package `ruff format --check` flagged. No behavior
change.

## Verification

- `pytest
tests/server/services/playbook/test_playbook_consolidator_integration.py`
— 14 passed (1 failed before)
- `ruff format --check reflexio tests` — 821 files already formatted
- `ruff check` + `pyright` clean on both touched files


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Bug Fixes**
* Improved filtering of outdated operation status updates, helping
prevent stale results from being processed.

* **Tests**
* Updated integration coverage to validate structured-output fallback
behavior using the configured primary model.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
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