Skip to content

fix(hy3): repair collapsed tool calls and Guardian routing - #83

Draft
jatmn wants to merge 4 commits into
mainfrom
fix/hy3-tool-calls-guardian
Draft

fix(hy3): repair collapsed tool calls and Guardian routing#83
jatmn wants to merge 4 commits into
mainfrom
fix/hy3-tool-calls-guardian

Conversation

@jatmn

@jatmn jatmn commented Aug 23, 2026

Copy link
Copy Markdown
Owner

Summary

  • repair Hy3 chat streams that collapse several same-function invocations into adjacent JSON argument objects under one tool-call index
  • emit distinct Responses calls with stable/unique IDs in streaming and non-streaming paths
  • add Hy3 Guardian auto-review metadata and provider-local model localization groundwork
  • keep repair opt-in to the Hy3 family, preserve exact argument text, and bound recovery to 64 calls and 1 MiB per source call

Draft status: known issues / not merge-ready

This PR is intentionally a draft. Final-head review found the following unresolved branch-owned issues:

  1. Response-wide call amplification: the 64-call and 1 MiB limits apply independently to each upstream source call. A response containing many tool-call indexes can therefore expand every index by up to 64×. Streaming conversion materializes all generated SSE events in ChatAccum::finish(), and non-streaming conversion builds the complete expanded output array, so a bounded upstream response can still cause excessive CPU/memory use or terminate the proxy. The repair needs a response-scoped recovery/output budget shared by both paths.
  2. Truncated or filtered output can become executable: repair currently runs for terminal finish_reason values including length and content_filter. Two complete objects observed before truncation/filtering can be converted from one invalid call into multiple valid executable calls. Expansion must be limited to a successful tool-call completion state.
  3. Streaming function identity is not stable: if an upstream reuses one tool-call index but later changes its function name, Warp appends all argument fragments while retaining the last name. Repair can then execute earlier arguments under the later function. The accumulator must detect name/identity changes and suppress recovery for that index.
  4. Static Guardian aliases can cross providers: provider-local catalog lookup does not prove final ownership of a globally colliding model slug. With multiple configured providers, a Hy3 model can advertise a bare review alias owned by another provider, sending the Guardian approval transcript to the wrong gateway. Hy3 review routing must self-localize to the visible model or otherwise verify final global route ownership.

The branch is also behind current main, with overlapping model/configuration changes. GitHub currently reports the PR as conflicting. These issues must be fixed, current main integrated, and the complete validation/review campaign rerun before the PR is marked ready.

Investigation

A recent concentrate.ai/hy3 Codex session stopped after Codex rejected one exec_command call with trailing JSON. Correlated Warp debug frames showed that the upstream stream reused tool-call index 0 while emitting two complete argument objects. Warp accumulated those fragments faithfully, producing one invalid argument string; the model then ended the turn. Replaying that malformed history can also trigger an upstream 400.

Historical codex-auto-review routing failures occurred before the session-aware fallback from #58. Current clients should also receive a concrete Hy3 auto_review_model_override; live-catalog aliases now localize to their exact discovered route instead of an unroutable bare hy3 name. Static-catalog collision handling remains an unresolved issue described above.

The installed operator configuration inspected during diagnosis predates the repository's Hy3 family defaults. It will need a separate config refresh after this change merges; this PR does not modify a running deployment.

Implementation

The new model-family transform parses only a complete sequence of two or more top-level JSON objects. Single objects, partial JSON, arrays/scalars, and trailing junk are forwarded unchanged. Original trimmed object spans are retained so large integers, exponent spellings, and duplicate keys are not rewritten. Recovery falls back to the original single call above 64 recovered objects or 1 MiB of arguments per source call.

The same repair and ID rules apply to streaming and non-streaming Chat Completions conversion. Native Responses behavior and unrelated model families are unchanged.

Validation

  • focused config, model-catalog, streaming, non-streaming, exact-text, ID, byte-bound, and count-bound regression tests
  • bash scripts/ci-preflight.sh, repeated by commit and pre-push hooks on head 3981e9d9b2c4a3b95f78b4fe8f850eff22054879
    • Clippy with warnings denied
    • 764 tests passed
    • build, rustdoc, CLI and diff checks passed
    • 30 diff mutants caught; 2 unviable; 0 missed
    • cargo-deny passed; cargo-audit completed as the repository's non-blocking scan

Passing validation does not clear the known design/lifecycle defects above.

Research

  • Tencent's Hy3 serving examples require the dedicated hy_v3 reasoning/tool parser and automatic tool choice
  • vLLM's official Hy3 parser assigns separate indexes and IDs as new tool calls begin
  • Tencent TokenHub documents standard OpenAI tool_calls arrays and role-tool continuations

Review in cubic

Summary by Sourcery

Repair Hy3 tool-call conversion and Guardian routing so repeated invocations remain valid and provider-routable.

Bug Fixes:

  • Repair Hy3 tool-call streams and responses that concatenate multiple JSON argument objects under a single call index.
  • Ensure repaired and missing tool calls receive distinct, stable non-empty call IDs across streaming and non-streaming conversions.
  • Route Guardian auto-review requests to provider-local Hy3 model identifiers, including live-discovered aliases.

Enhancements:

  • Keep argument repair narrowly scoped to Hy3 models, preserve original JSON text, and cap recovery at 64 calls and 1 MiB of arguments.
  • Expose and document the opt-in transform for splitting concatenated tool-call arguments.

Documentation:

  • Document Hy3 concatenated tool-call recovery behavior and add the corresponding model-family catalog option.

Tests:

  • Add regression coverage for Hy3 routing, streaming and non-streaming repair, exact argument preservation, unique IDs, opt-in behavior, and recovery bounds.

@jatmn jatmn self-assigned this Aug 23, 2026
@jatmn
jatmn marked this pull request as ready for review August 23, 2026 14:46

@sourcery-ai sourcery-ai 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.

Hey - I've found 1 issue

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="src/response_codec.rs" line_range="2453-2467" />
<code_context>
                     .and_then(Value::as_str)
                     .unwrap_or("{}");
                 let call_id = call.get("id").and_then(Value::as_str).unwrap_or("call");
-                output.push(tool_call_item(
-                    name,
-                    arguments,
</code_context>
<issue_to_address>
**issue (bug_risk):** When a repaired tool call has no upstream `id`, the first emitted call uses the literal fallback ID `"call"`; any other call without an ID in the same response also uses `"call"`. This produces duplicate Responses call IDs instead of the stable, unique IDs promised by the repair path.

**Triggers:** When Hy3 omits tool-call IDs, or when multiple tool-call entries without IDs are present in one completion.

**Suggested fix:** Generate an ID for the first emitted call whenever `call_id` is missing, rather than preserving the literal `"call"` fallback; use the same generated-ID policy for every emitted call that lacks an upstream ID.
</issue_to_address>

Sourcery assessment

Needs a human reviewer. 1 finding to address first, and if the repair misparses a Hy3 argument stream, it can turn one malformed tool call into multiple executable calls, including extra shell or other agent actions; those effects may already occur before a revert. The Hy3-only scope bounds the exposure, and reverting restores the previous routing and parsing behavior but cannot undo actions or review decisions already made.

Blocking findings: src/response_codec.rs:2467


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.

Comment thread src/response_codec.rs Outdated
@jatmn
jatmn marked this pull request as draft August 23, 2026 16:38
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