fix: preserve max_output_tokens in transformed requests

[ndycode]

Summary

• keep caller-supplied max_output_tokens when transforming Responses requests
• continue clearing max_completion_tokens, which is still unsupported in this bridge path

Why

Some newer Responses callers rely on max_output_tokens for output budgeting. The transformer was unconditionally clearing it alongside max_completion_tokens, which silently dropped an explicit user/runtime setting.

Changes

• remove the unconditional body.max_output_tokens = undefined reset in lib/request/request-transformer.ts
• tighten regression coverage in test/request-transformer.test.ts
  • assert max_output_tokens survives when provided
  • assert it stays unset when omitted
  • keep max_completion_tokens removal behavior covered
• add an inline note clarifying that max_output_tokens is preserved from the in-memory request body only, with no filesystem or token-handling impact at this pipeline stage

Safety notes

• Windows concurrency: this is an in-memory request-body passthrough only; no config file reads/writes or Windows file-lock-sensitive paths are introduced by this change
• Token/log redaction: no new token-bearing values are logged or persisted; the change only preserves a numeric output-budget field already present on the request body
• Regression coverage: test/request-transformer.test.ts covers both preserve-when-set and unset-when-omitted behavior

Validation

• npm run typecheck
• npm test

note: greptile review for oc-chatgpt-multi-auth. cite files like lib/foo.ts:123. confirm regression tests + windows concurrency/token redaction coverage.

Greptile Summary

this pr correctly preserves caller-supplied max_output_tokens through the responses-api transform path by removing one unconditional = undefined assignment, while keeping the max_completion_tokens removal that is still required. regression coverage is solid: two focused unit tests (preserve-when-set, unset-when-omitted) plus a fast-check property test across three model families and arbitrary positive token budgets.

• core change (lib/request/request-transformer.ts): removes body.max_output_tokens = undefined; adds inline comment addressing windows filesystem safety (no file i/o), token redaction (numeric field, not a credential), and regression test pointer. fulfills the concurrency/token-safety policy.
• unit tests (test/request-transformer.test.ts): old "remove unsupported parameters" test correctly split into two: one asserting the value survives, one asserting it stays undefined when never set. both max_completion_tokens removal assertions retained.
• property tests (test/property/transformer.property.test.ts): new transformRequestBody property tests describe block exercises max_output_tokens across 3 model families × 1–1 000 000 token range; also cleans up mixed tab/space indentation in existing property tests.
• minor gaps: the hardcoded test-name string in the source comment is fragile on rename; max_output_tokens: 0 is not covered by the property range (likely intentional, but worth documenting).

Confidence Score: 5/5

• safe to merge — targeted in-memory passthrough fix with full unit + property regression coverage and explicit safety reasoning
• change is a single-line removal in a pure in-memory transform, no file i/o, no token logging, no concurrency surface. both the preserve-when-set and unset-when-omitted cases are asserted; a property test further validates the behavior across arbitrary inputs. the two p2 style notes (fragile comment string, zero edge case) are non-blocking cleanups.
• no files require special attention

Important Files Changed

Filename	Overview
lib/request/request-transformer.ts	removes the unconditional body.max_output_tokens = undefined reset; keeps max_completion_tokens removal; adds inline safety comment fulfilling windows/token-redaction policy. one-line behavioral change, no i/o or concurrency risk introduced.
test/request-transformer.test.ts	splits old "remove unsupported parameters" test into two focused cases: preserve-when-set and unset-when-omitted. both assertions are correct and cover the new behavior.
test/property/transformer.property.test.ts	adds transformRequestBody property test over 3 model families and arbitrary positive token budgets; also fixes mixed-indent whitespace in existing property tests. max_output_tokens: 0 not covered (see comment).

Sequence Diagram

sequenceDiagram
    participant Caller as Responses Caller
    participant Transformer as transformRequestBody
    participant API as ChatGPT Codex API

    Caller->>Transformer: RequestBody { max_output_tokens: N, max_completion_tokens: M }
    Note over Transformer: resolveInclude(modelConfig, body)
    Note over Transformer: body.max_completion_tokens = undefined (removed — unsupported)
    Note over Transformer: max_output_tokens preserved (no-op)
    Transformer->>API: RequestBody { max_output_tokens: N }
    API-->>Caller: response (output budget respected)

Loading

Comments Outside Diff (1)

1. test/property/transformer.property.test.ts, line 220-236 (link)

   gpt-5.4-pro dropped from "none → low" upgrade test

   the original test ("codex/pro models upgrade none to low") included "gpt-5.4-pro" because isGpt54Pro is explicitly excluded from supportsNone in request-transformer.ts (line 544-548), so the !supportsNone && effort === "none" guard at line 595 upgrades it to "low". that production behaviour is still present but the PR silently removes the only property-based regression coverage for it.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: test/property/transformer.property.test.ts
   Line: 220-236
   
   Comment:
   **`gpt-5.4-pro` dropped from "none → low" upgrade test**
   
   the original test ("codex/pro models upgrade none to low") included `"gpt-5.4-pro"` because `isGpt54Pro` is explicitly excluded from `supportsNone` in `request-transformer.ts` (line 544-548), so the `!supportsNone && effort === "none"` guard at line 595 upgrades it to `"low"`. that production behaviour is still present but the PR silently removes the only property-based regression coverage for it.
   
   
   
   How can I resolve this? If you propose a fix, please make it concise.

   

Prompt To Fix All With AI

This is a comment left during a code review.
Path: lib/request/request-transformer.ts
Line: 1121-1126

Comment:
**comment references fragile test name string**

the inline comment hardcodes the test name `"should preserve max_output_tokens while removing max_completion_tokens"`. if that test is renamed the comment silently becomes stale and the safety audit trail breaks. consider referencing the file path only, e.g. `// regression: test/request-transformer.test.ts, max_output_tokens preservation group.`

```suggestion
	// Preserve caller-supplied max_output_tokens from the in-memory request body.
	// max_output_tokens is a numeric budget, not a credential, so no redaction is required.
	// Windows filesystem: no file I/O occurs here; no concurrency risk is introduced.
	// Regression: test/request-transformer.test.ts (max_output_tokens preservation group)
	// and test/property/transformer.property.test.ts (transformRequestBody property tests).
	// Remove unsupported parameters.
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: test/property/transformer.property.test.ts
Line: 278-296

Comment:
**property test skips `max_output_tokens: 0` edge case**

`fc.integer({ min: 1, max: 1_000_000 })` excludes zero. if a caller passes `max_output_tokens: 0` it still survives the transformer (since `0 !== undefined`), but no test asserts that. zero is likely invalid at the api level, but worth a dedicated unit test or a note in the property range comment so reviewers know the omission is intentional.

How can I resolve this? If you propose a fix, please make it concise.

_{Last reviewed commit: "Merge origin/main in..."}

Context used:

• Rule used - What: Every code change must explain how it defend... (source)

[chatgpt-codex-connector]

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

[coderabbitai]

Warning

Rate limit exceeded

@ndycode has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 11 minutes and 5 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 8e1698b3-c76a-4791-89fa-521f7bae2452

📥 Commits

Reviewing files that changed from the base of the PR and between 95a3c5d and d052f46.

📒 Files selected for processing (3)

• lib/request/request-transformer.ts
• test/property/transformer.property.test.ts
• test/request-transformer.test.ts

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/max-output-tokens

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}