Skip to content

fix(google): enforce response part field contract - #2246

Merged
lidge-jun merged 1 commit into
devfrom
ingw/fix-google-part-field-contract
Aug 21, 2026
Merged

fix(google): enforce response part field contract#2246
lidge-jun merged 1 commit into
devfrom
ingw/fix-google-part-field-contract

Conversation

@Ingwannu

@Ingwannu Ingwannu commented Aug 21, 2026

Copy link
Copy Markdown
Owner

Summary

Fixes #2233 after the project owner policy decision:

  • reject a present Google functionCall unless it is an object with a nonblank string name
  • terminate with a structured adapter error before thought-signature observation or tool dispatch
  • drop non-string optional part.text values without coercion
  • preserve provider-native functionCall.args exactly as before
  • enforce identical behavior in streaming and buffered response parsing

Why this disposition

Gemini delivers a complete function call in one response part, so a missing or malformed name cannot be repaired by a later delta. Passing or coercing it can dispatch an unidentified tool. Optional malformed text is different: dropping only that field avoids fabricating assistant output while preserving the rest of the response.

Verification

  • Google hardening suite: 78/78 passed
  • Google adjacent adapter suites: 186/186 passed
  • bun run typecheck: passed
  • bun run privacy:scan: passed
  • git diff --check: passed
  • full repository attempt: 13,717 pass / 15 skip; local environment failures were unrelated to this diff. Missing isolated GUI dependencies were resolved by linking gui/node_modules, and service-token shim failures passed after unsetting the host token environment. The remaining translator contract failure is the installed TypeScript rejecting the repository test option --ignoreConfig before checking the fixture.

Decision Log

  • Purpose: keep malformed Google-compatible fields from violating the internal string-only event contract or dispatching a nameless tool.
  • Existing constraint: container validation did not validate values inside a valid part; Gemini function calls are atomic, while arguments are structured provider-native values.
  • Alternatives considered: pass through, coerce, silently drop all malformed fields, or terminate every malformed field.
  • Chosen approach: terminal error for invalid call identity, field-only drop for non-string text, unchanged argument serialization.
  • Tradeoff: malformed third-party Google-compatible text is ignored, while malformed tool identity ends the turn visibly and safely.

Review readiness checklist

  • All focused tests and static checks are green.
  • The branch is based on the latest dev at creation time.
  • The owner Grok review policy is implemented exactly.
  • Ready for another maintainer review; the author will not self-approve or self-merge.

Summary by CodeRabbit

  • Bug Fixes

    • Improved handling of malformed Google responses during streaming and standard requests.
    • Invalid or incomplete function calls now produce clear protocol errors.
    • Malformed text values are ignored rather than emitted incorrectly.
    • Function arguments are preserved in their original provider format.
  • Documentation

    • Added documentation describing Google response validation behavior.

@coderabbitai

coderabbitai Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: d2d411bd-a436-4689-8f7f-48f269fbf364

📥 Commits

Reviewing files that changed from the base of the PR and between 08cc2ac and 0103abd.

📒 Files selected for processing (3)
  • src/adapters/google.ts
  • structure/04_transports-and-sidecars.md
  • tests/google-hardening.test.ts

📝 Walkthrough

Walkthrough

The Google adapter validates function-call payloads and names before emitting tool-call events. Streaming and buffered parsers share the validation path. Non-string text values are ignored. Tests and documentation cover these behaviors.

Changes

Google response field validation

Layer / File(s) Summary
Function-call and text validation contract
src/adapters/google.ts
The adapter defines a validated function-call shape, rejects invalid payloads and names, and accepts only non-empty string text values.
Streaming and buffered parser integration
src/adapters/google.ts
Both parsers validate function calls before dispatch and use the validated call for tool-call event emission.
Validation coverage and response-part documentation
tests/google-hardening.test.ts, structure/04_transports-and-sidecars.md
Tests cover malformed function calls and non-string text in both parsing paths. Documentation describes the validation behavior and preservation of structured arguments.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: lidge-j, devmello

✨ 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 ingw/fix-google-part-field-contract

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.

@github-actions

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 57 / 80

#2233 본체임. 컨테이너는 이미 #2232가 dev에 들어감. 지금 HEAD 826a1b7a4 googlePartTextEvent (src/adapters/google.ts:368-372)가 if (!part.text)만 봄. 숫자 5, 객체, 배열은 truthy라 text_delta.text / reasoning_raw_delta에 비문자열이 들어감. 스트림 :838 / 버퍼 :1095if (part.functionCall)만. 문자열/숫자/배열이 truthy라 .name이 undefined인 tool_call_start가 브릿지로 감. AdapterEvent는 name/text를 string으로 선언함. throw는 없음. 조용히 계약이 깨짐. #2233 본문 14케이스랑 맞음.

이 PR이 그 정책을 그대로 박음. 있는 functionCall은 객체 + nonblank string name. 아니면 터미널 에러. cannot dispatch. openai-chat unnamedToolCallEvent (src/adapters/openai-chat.ts) 톤. 진단이 파트 전체 순회를 시그니처 관찰보다 먼저. 리플레이 상태에 이름 없는 콜이 안 들어감. 비문자열 text는 그 필드만 drop. coerce 없음. functionCall.args는 그대로 JSON.stringify. 스트림/버퍼 동일. #2233 리뷰가 시킨 그거임. ㅇㅇ

테스트가 tests/google-hardening.test.ts에 콜 7종 × 양 경로 + 비문자열 text 4종. 메시지 exact. src/만 되돌리면 새 케이스가 깨짐. structure/04_transports-and-sidecars.md Decision Log도 정책이랑 맞음. draft임. #2232에 접지 말 것. 그 PR은 컨테이너고 이미 머지됨. 여기 안쪽 필드를 거기 되돌리면 리뷰가 다시 열림.

1st-party Google이 이 셰이프를 안 낸다는 말은 여전함. 노출은 호환 baseUrl / 이상한 CCA 엔벨로프. 그래서 점수가 컨테이너 구멍(#2231/#2232)보다 한 칸 낮음. 그래도 이름 없는 콜이 브릿지로 가면 잘못된 툴이 실행될 수 있음. ㅋㅋ 그게 제일 큼. #2216 프리폼이랑 섞지 말 것. args 손대지 말 것.

src/types.ts / src/types/config.ts 안 건드림. 스플릿이 어댑터 파서를 옮기면 리베이스하지 말고 닫고 다시 짜라. 지금은 그 정도 아님. #2188 사이드카, #2190 x_search, #2217/#2227 와이어 기본 무관. 2.28 블로커 아님. 닫을 중복 아님. #2233은 머지될 때까지 열어둠. 클로징 키워드는 머지 커밋에만.

해결방안: 체크리스트 채우고 draft 해제 후 dev 머지. 정책 바꾸지 말 것 — 콜은 terminate, text는 drop, args는 유지. #2232 되돌리거나 한 PR로 합치지 말 것. 스플릿이 google.ts 파서를 옮기면 닫고 다시 짜라.

이 댓글은 grok-bot이 작성했습니다

@snowyukitty snowyukitty left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed at exact head cb4fbc1c, rebuilt locally and run through bun scripts/test.ts.

Declaring my position: I filed #2233, so this is the reporter confirming the implementation
matches the report and does not break anything adjacent — not an independent maintainer approval.
Per MAINTAINERS.md the merge still needs one of you; GitHub also refuses an APPROVE from an
account without repository access, so this is submitted as a comment with the verdict stated at the
bottom.

All fourteen reported shapes behave per the stated policy

# parts element On cb4fbc1c
A–D functionCall is "x" / 5 / [], streaming and buffered terminal invalid function call (function_call_not_object; partIndex=0; valueType=…) — cannot dispatch
E, F, I name absent / 5, streaming and buffered terminal function_call_name_invalid
G, H name is "" / " " terminal function_call_name_blank
J–N text is 5 / {a:1} / [1,2] / 5+thought:true, streaming and buffered dropped, turn completes
O control: [{text:"hi"},{functionCall:{name:"lookup",args:{q:1}}}] unchanged

Seventeen adjacent cases I probed for a gap, and did not find one

The ones that could plausibly have gone wrong:

  • Fail-closed lands before any content in the same chunk escapes. [{text:"hello"},{functionCall:{}}]
    emits no text_delta — the whole parts array is validated first, matching how #2232 validates
    the container before iterating. Same on the buffered path, and [{functionCall:{name:"ok"}},{functionCall:5}]
    emits no tool call either.
  • Content already delivered in an earlier chunk is kept: chunk 1 text, chunk 2 malformed call
    [text("hello"), error(…)]. That is the right split and it is the #1219 line.
  • partIndex is accurate — a bad call at index 2 reports partIndex=2, not 0.
  • name: "\t\n " is caught by the blank check, not just space.
  • functionCall: null is absence, not corruption[{functionCall:null,text:"hi"}] still emits
    text("hi") and completes. Consistent with how parts: null and content: null are read.
  • args missing and args: null both still serialize to {} — unchanged, as intended.
  • A non-string thoughtSignature on an otherwise valid call is inert. thoughtSignature: 5
    emits the call with no providerMetadata; a real 20-char base64 signature still attaches. Both
    consumers were already typeof === "string"-guarded — isLikelyRealThoughtSignature
    (src/adapters/google-antigravity-wire.ts:30-31) and extractSignature
    (src/adapters/google-antigravity-replay.ts:484-490) — so widening text/functionCall to
    unknown does not expose them.
  • CCA rejects before replay observation. A response-wrapped malformed call terminates without
    entering the replay cache, which is the part of the docstring I most wanted to see hold.
  • applyAntigravityReplay is a cleared site — it reads request contents, not response parts,
    and already guards typeof fc.name === "string" (:707).
  • #2232's container guards are intact, checked on this head: parts: {} still
    invalid content parts, buffered candidates: [null] still invalid candidates, and the
    mid-stream {"candidates":null} frame still completes with its text rather than terminating.

The one design refinement worth calling out

The owner review on #2233 said "비문자열 text는 파트 drop". This implements field-drop, not
part-drop, and the docstring says why: "Dropping only this optional field preserves the rest of the
part". That is a better reading and it is load-bearing — [{text:5, functionCall:{name:"ok",args:{}}}]
still dispatches the call. Part-drop would have silently discarded a valid, named tool call over a
malformed sibling field, which is the exact "strand a tool turn" outcome #1332 was written to
prevent. Worth keeping the sentence in structure/04_transports-and-sidecars.md as the record of it.

One non-blocking observation

A part whose only content is a malformed text now yields a completed, empty turn
([{text:5}]heartbeat, done). That is the chosen policy working as specified and I am not
asking for a change — terminating on optional malformed text would discard otherwise usable
responses, exactly as the Decision Log argues. Recording it because the net underneath it is
guardEmptyCompletionEventStream, not the adapter: the retry is what keeps this from being a silent
loss, so it is worth knowing that the two are now coupled for this shape.

Smaller, and genuinely cosmetic: GoogleResponsePart widens text and functionCall to unknown
but leaves thoughtSignature?: string / thought_signature?: string declaring string. Runtime is
safe for the reason above. Only mentioning it because the PR's own thesis is that this interface
should not claim what the wire does not guarantee.

Verification on this head

All through the repository runner, bun scripts/test.ts:

  • 660 pass / 0 fail across 28 files — google-hardening, google-adapter, google-empty-content,
    google-buffered-stop-reason, the three Vertex suites, the three Antigravity suites,
    google-signature-history-roundtrip, google-claude-prefill-guard, google-wire-compiler,
    google-tool-schema, sse-null-data-frame, both adapter-conformance suites,
    openai-chat-hardening, images/gemini-inline, server-images,
    compatibility-provider-equivalence.
  • bun x tsc --noEmit — output identical to dev, both carrying only the pre-existing
    Cannot find module '@napi-rs/keyring'.

No change requested from me. The three-way split — terminate on call identity, drop on optional
text, leave args provider-native — is the right set of lines, and it holds on every shape I could
construct.

@lidge-jun
lidge-jun marked this pull request as ready for review August 21, 2026 06:29
@lidge-jun
lidge-jun force-pushed the ingw/fix-google-part-field-contract branch from cb4fbc1 to 0103abd Compare August 21, 2026 06:29
@lidge-jun
lidge-jun merged commit 1f2615e into dev Aug 21, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants