Skip to content

fix(google): clamp max output tokens per model - #2512

Closed
Hsia97 wants to merge 2 commits into
lidge-jun:devfrom
Hsia97:fix/google-output-clamp
Closed

Hsia97 wants to merge 2 commits into
lidge-jun:devfrom
Hsia97:fix/google-output-clamp

Conversation

@Hsia97

@Hsia97 Hsia97 commented Aug 25, 2026 •

Copy link
Copy Markdown
Contributor

Split from #2470 (closed). Scope: maxOutputTokens clamping only.

Changes:

  • Add maxOutputTokensForGoogleModel / clampGoogleMaxOutputTokens.
  • Clamp requested maxOutputTokens per model (Flash 65536, Pro 65535, Claude 64000, GPT-OSS 32768, default 16384).
  • No thinkingBudget coupling in this PR; the clamp is purely a downward cap on the requested value.

Verification:

  • �un test tests/google-output-clamp.test.ts -> 4 pass / 0 fail
  • �un run typecheck -> 0 errors

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

  • All CI tests are green on my local testing.

  • I pushed my PR to the latest dev commit.

  • I resolved all correct Codex and CodeRabbit findings.

  • My PR is ready for review.

@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Aug 25, 2026
@github-actions

Copy link
Copy Markdown
Contributor

⏳ DRAFT

  • review readiness checklist open (0/4 boxes ticked).

What to do

  • Tick all four boxes in the PR description once you're done (currently 0/4).

Review readiness checklist

  • ⬜ All CI tests are green on my local testing.
  • ⬜ I pushed my PR to the latest dev commit.
  • ⬜ I resolved all correct Codex and CodeRabbit findings.
  • ⬜ My PR is ready for review.

0/4 boxes ticked.

This PR stays in draft until every box above is ticked.

@github-actions
github-actions Bot marked this pull request as draft August 25, 2026 02:40
@coderabbitai

coderabbitai Bot commented Aug 25, 2026 •

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@Ingwannu Ingwannu left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Please do not merge hardcoded substring caps in this form. The values are not derived from provider/model metadata, several aliases can match the wrong branch, and every unknown Google-routed model is silently clamped to 16,384 tokens. That can truncate models which support more output and makes future model additions depend on editing this adapter table. The cap needs an authoritative per-model source (live metadata or the existing catalog/config capability path), a conservative no-cap behavior when unknown, and regressions for aliases plus unknown/custom models. Keep this draft while the contract is redesigned.

@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 52 / 80

설명: 이 풀은 구글 어댑터가 내보내는 maxOutputTokens 를 모델마다 아래로만 자르려는 드래프트다. 2470 에서 잘랐다. 범위는 클램프만이다. thinkingBudget 과 묶지 않는다. 지금 CURRENT dev HEAD 는 bb89eaf 이다. origin/dev 는 이번 시간에 안 움직였다. 베이스는 dev 이고 MERGEABLE 이다. 머지 상태는 BLOCKED 다. 드래프트다. 점검이 비어 있다.

지금 HEAD 의 src/adapters/google.ts 653줄은 요청값이 있으면 그대로 generationConfig.maxOutputTokens 에 넣는다. 이 파일이 그 값을 쓰는 곳은 그 한 줄뿐이다. createGoogleAdapter 한 길이라서 다이렉트와 버텍스와 클라우드 코드 어시스트가 같이 잘린다. 이 풀은 maxOutputTokensForGoogleModel 과 clampGoogleMaxOutputTokens 를 같은 파일 위에 넣는다. flash 는 65536, pro 는 65535, claude 는 64000, gpt-oss 또는 oss 는 32768, gemini 로 시작하면 65536, 나머지는 16384 다. 요청이 없거나 0 이하면 칸 자체를 안 넣는다.

부분 문자열 매칭이 너무 넓다. includes("oss") 는 gpt-oss 만이 아니다. 아이디 어디에 oss 세 글자가 있으면 32768 로 떨어진다. includes("pro") 는 flash 다음이라 gemini-3.7-flash 는 산다. 그러나 이름에 pro 가 들어간 다른 아이디도 65535 가 된다. includes("claude") 는 안티그래비티 클로드 경로를 노린 것이다. 기본 16384 는 모르는 아이디를 너무 낮게 자를 수 있다. 시험 tests/google-output-clamp.test.ts 는 헬퍼만 잠근다. 어댑터가 실제로 generationConfig 에 넣는지는 잠그지 않는다. 라이브 한도 측정도 본문에 없다.

2470 은 이미 닫혔다. 이 풀로 다시 열거나 다른 구글 구멍을 닫지 말 것. 2510 의 429 분류, 2513 의 생각 서명 재생과 겹치지 않는다. 한 줄로 묶지 말 것. types.ts/config.ts 가르기를 건드리지 않는다. 닫고 다시 짜라고 하지 않는다. 다만 드래프트이고 점검이 비어 있으니 지금 합치지 않는다.

src/adapters/google.ts 653 - HEAD 는 요청값을 그대로 넣는다. 이 풀의 유일한 클램프 자리다
maxOutputTokensForGoogleModel includes("oss") - gpt-oss 외 아이디까지 32768 로 떨어질 수 있다
maxOutputTokensForGoogleModel includes("pro") - 이름에 pro 가 있으면 65535 다. 카탈로그 한도가 아니다
기본값 16384 - 모르는 아이디를 너무 낮게 자를 수 있다
tests/google-output-clamp.test.ts - 헬퍼만 잠근다. 와이어 본문을 안 본다

메인테이너의 판단이 필요한 지점

  • 드래프트를 유지할지. 유지하는 편이 맞다
  • 부분 문자열 대신 카탈로그 한도를 쓸지. 쓰는 편이 맞다
  • 기본 16384 를 요청값 통과로 둘지. 모르는 아이디는 자르지 않는 편이 안전하다
  • 지금 머지할지. 하지 말 것

너의 추천
드래프트로 둔다. 머지하지 않는다. 부분 문자열 맵을 카탈로그 한도로 바꾸고, 어댑터 와이어 시험을 넣은 뒤에 레디로 올린다. 2510 2513 과 묶지 말 것. 라벨은 그대로 둔다.

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

lidge-jun added a commit that referenced this pull request Aug 25, 2026
…eiling for unknown ids (rebase of #2512) (#2576)

* fix(google): clamp max output tokens per model

* fix(google): clamp max output tokens per model

* fix(google): do not invent an output ceiling for unrecognized models

The clamp matched by substring and fell back to 16,384 for anything
unmatched, so an alias, a gateway id, or any model newer than the table
was silently truncated to 16,384 regardless of what the operator asked
for. structure/02_config-and-codex-home.md is explicit that an explicit
request value wins.

Unknown ids now return undefined and pass the request through untouched;
the upstream stays the authority on its own limit. Matching is also
prefix/family based, because includes(pro) matched my-prototype-model and
includes(oss) matched crossover-v2.

---------

Co-authored-by: Hsia97 <xjxj1997@163.com>
@lidge-jun

Copy link
Copy Markdown
Owner

Landed on dev as #2576 with your commits rebased onto the current head and authorship preserved.

I folded in the review findings rather than sending them back. Two things: the unmatched fallback to 16,384 silently truncated aliases, gateway-prefixed ids, and any model newer than the table regardless of what the operator requested — structure/02_config-and-codex-home.md says an explicit request value wins, so unknown ids now return undefined and pass through untouched. And the substring match fired on incidental text (includes(pro) matched my-prototype-model, includes(oss) matched crossover-v2), so matching is now prefix/family based.

The original test asserted the 16,384 fallback, which locked the defect in; it now pins the passthrough contract and the substring cases instead. 16 pass across the two Google suites, typecheck clean. Thanks for the fix.

@lidge-jun lidge-jun closed this Aug 25, 2026
tarunravi pushed a commit to tarunravi/opencodex that referenced this pull request Sep 14, 2026
…eiling for unknown ids (rebase of lidge-jun#2512) (lidge-jun#2576)

* fix(google): clamp max output tokens per model

* fix(google): clamp max output tokens per model

* fix(google): do not invent an output ceiling for unrecognized models

The clamp matched by substring and fell back to 16,384 for anything
unmatched, so an alias, a gateway id, or any model newer than the table
was silently truncated to 16,384 regardless of what the operator asked
for. structure/02_config-and-codex-home.md is explicit that an explicit
request value wins.

Unknown ids now return undefined and pass the request through untouched;
the upstream stays the authority on its own limit. Matching is also
prefix/family based, because includes(pro) matched my-prototype-model and
includes(oss) matched crossover-v2.

---------

Co-authored-by: Hsia97 <xjxj1997@163.com>
agentHits pushed a commit to agentHits/opencodex that referenced this pull request Sep 17, 2026
…eiling for unknown ids (rebase of lidge-jun#2512) (lidge-jun#2576)

* fix(google): clamp max output tokens per model

* fix(google): clamp max output tokens per model

* fix(google): do not invent an output ceiling for unrecognized models

The clamp matched by substring and fell back to 16,384 for anything
unmatched, so an alias, a gateway id, or any model newer than the table
was silently truncated to 16,384 regardless of what the operator asked
for. structure/02_config-and-codex-home.md is explicit that an explicit
request value wins.

Unknown ids now return undefined and pass the request through untouched;
the upstream stays the authority on its own limit. Matching is also
prefix/family based, because includes(pro) matched my-prototype-model and
includes(oss) matched crossover-v2.

---------

Co-authored-by: Hsia97 <xjxj1997@163.com>
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