Skip to content

feat: add GPT-5.4 support and docs refresh#65

Merged
ndycode merged 5 commits intomainfrom
feat/gpt-5.4-support-v5.4.2
Mar 6, 2026
Merged

feat: add GPT-5.4 support and docs refresh#65
ndycode merged 5 commits intomainfrom
feat/gpt-5.4-support-v5.4.2

Conversation

@ndycode
Copy link
Copy Markdown
Owner

@ndycode ndycode commented Mar 5, 2026

Summary

This PR updates the plugin to support the GPT-5.4 release while keeping rollout safe for existing users.

  • Adds runtime support for gpt-5.4 as the new default general-purpose model family.
  • Adds optional/manual support for gpt-5.4-pro (not enabled by default in shipped templates).
  • Adds a default fallback edge in fallback mode: gpt-5.4-pro -> gpt-5.4.
  • Keeps legacy fallback behavior unchanged (gpt-5.3-codex -> gpt-5.2-codex via existing compatibility toggle).
  • Refreshes config templates and docs so examples/default usage now point to GPT-5.4.
  • Expands automated tests to cover GPT-5.4 and GPT-5.4-pro normalization, reasoning handling, and fallback behavior.

Detailed Changes

Runtime/model handling

  • lib/request/helpers/model-map.ts
    • Added mappings for GPT-5.4 variants (none, low, medium, high, xhigh) -> gpt-5.4.
    • Added mappings for GPT-5.4 Pro variants (low, medium, high, xhigh) -> gpt-5.4-pro.
  • lib/request/request-transformer.ts
    • Added normalize logic for gpt-5.4 and gpt-5.4-pro.
    • Added reasoning-effort rules:
      • gpt-5.4 supports none and xhigh.
      • gpt-5.4-pro supports xhigh; none is upgraded to low.
  • index.ts
    • Updated quota probe model order to include gpt-5.4 first.

Fallback policy and entitlement behavior

  • lib/request/fetch-helpers.ts
    • Added default fallback chain edge for fallback mode:
      • gpt-5.4-pro -> gpt-5.4
    • Updated entitlement guidance message to include gpt-5.4 as latest general fallback option.

Prompt family/instructions

  • lib/prompts/codex.ts
    • Added gpt-5.4 model family detection.
    • Added cache file handling for GPT-5.4 instructions.
    • Mapped GPT-5.4 to current upstream prompt family file (gpt_5_2_prompt.md) per current Codex prompt layout.
    • Included GPT-5.4 in instruction prewarm candidates.

Templates/docs/versioning

  • Config templates now default to GPT-5.4:
    • config/opencode-modern.json
    • config/opencode-legacy.json
  • Documentation refreshed for GPT-5.4 defaults + optional/manual GPT-5.4 Pro usage:
    • README.md
    • config/README.md
    • docs/getting-started.md
    • docs/configuration.md
    • docs/troubleshooting.md
    • docs/index.md
    • docs/development/TESTING.md
  • Version and changelog:
    • package.json -> 5.4.2
    • CHANGELOG.md entry for 5.4.2

Test Coverage Added

  • Unit/integration coverage for GPT-5.4 and GPT-5.4-pro in:
    • test/model-map.test.ts
    • test/request-transformer.test.ts
    • test/fetch-helpers.test.ts
    • test/codex-prompts.test.ts
    • test/index.test.ts
  • Property-test input space expanded:
    • test/property/helpers.ts
    • test/property/transformer.property.test.ts

Validation

  • npm run typecheck passed
  • npm run lint passed
  • npm test passed
    • 56 test files
    • 1797 tests

Notes for Reviewers

  • gpt-5.4-pro remains opt-in/manual in templates to avoid entitlement surprises.
  • Existing legacy fallback path behavior is preserved.
  • PR scope intentionally includes runtime + tests + docs to keep model rollout consistent across code and user guidance.

Summary by CodeRabbit

  • New Features

    • Added support for GPT-5.4 and optional GPT-5.4-Pro models with normalization, reasoning behavior, and fallback handling.
  • Documentation

    • Updated configuration, getting-started, troubleshooting, and readme content to reflect GPT-5.4/5.4‑Pro availability and fallback mappings.
  • Tests

    • Expanded test coverage for normalization, reasoning configs, fallbacks, and extensive edge cases for the GPT-5.4 family.
  • Bug Fixes

    • Quota probing now prioritizes GPT-5.4 before legacy Codex models.
  • Chores

    • Version bumped to 5.4.2.

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 adds runtime support for gpt-5.4 and optional gpt-5.4-pro across model normalization, reasoning configuration, prompt-family detection, fallback policy, and documentation. the rollout is conservative: gpt-5.4-pro stays opt-in in shipped templates and a default fallback edge (gpt-5.4-pro → gpt-5.4) is wired into the existing DEFAULT_UNSUPPORTED_CODEX_FALLBACK_CHAIN. previously-flagged gaps (gpt-5.4-pro-none missing from MODEL_MAP, stripped gpt-5.2-codex normalization tests) have been addressed.

key changes:

  • model-map.ts — 12 new entries for gpt-5.4 and gpt-5.4-pro variants (including gpt-5.4-pro-none)
  • request-transformer.tsgpt-5.4-pro pattern checked before gpt-5.4; none→low upgrade for pro; supportsXhigh and supportsNone flags updated correctly
  • codex.ts"gpt-5.4" added as a ModelFamily; both gpt-5.4 and gpt-5.4-pro resolve to the same family (gpt_5_2_prompt.md / gpt-5.4-instructions.md), which is intentional and test-confirmed; prewarmCodexInstructions now includes gpt-5.4
  • fetch-helpers.tsgpt-5.4-pro → gpt-5.4 fallback edge added; entitlement error message updated
  • index.tsgpt-5.4 added first in quota probe list
  • config templates and all docs updated to default to gpt-5.4
  • 1797 tests across 56 files passing; new dedicated files test/gpt54-models.test.ts, test/edge-cases.test.ts, test/helper-utils.test.ts added

the model normalization logic is correct, regex priority ordering is right (pro before base), xhigh/none/upgrade rules are consistent, fallback chain is clean, and test coverage is broad. one test description needs clarity for readability.

Confidence Score: 4/5

  • safe to merge; model logic is correct with proper regex priority, fallback chain is clean, and test coverage is broad.
  • model normalization logic is correct, pro/base distinction is properly prioritized in regex patterns, reasoning effort configurations are appropriate (none→low upgrade for pro, both support xhigh), and fallback behavior is well-tested. one test description needs rewording for clarity. no safety or functional concerns.
  • test/gpt54-models.test.ts — test description naming (minor clarity issue)

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["incoming model string\n(e.g. openai/gpt-5.4-pro-high)"] --> B["strip provider prefix\n(split on '/')"]
    B --> C["MODEL_MAP lookup\ngetNormalizedModel()"]
    C -- hit --> D["normalized model ID\n(e.g. gpt-5.4-pro)"]
    C -- miss --> E["modelId.toLowerCase()"]
    E --> F{regex chain}
    F -- matches gpt-5.4-pro --> G["return 'gpt-5.4-pro'"]
    F -- matches gpt-5.4 --> H["return 'gpt-5.4'"]
    F -- matches other --> I["existing families\n(gpt-5.2, gpt-5-codex, etc.)"]
    F -- no match --> J["return 'gpt-5.1' (fallback)"]

    D --> K["getReasoningConfig()"]
    G --> K
    H --> K

    K --> L{isGpt54Pro?}
    L -- yes --> M["supportsNone = false\nnone → low upgrade\nsupportsXhigh = true"]
    L -- no --> N{isGpt54General?}
    N -- yes --> O["supportsNone = true\nsupportsXhigh = true"]

    K --> P["getModelFamily() for prompts"]
    P --> Q["both gpt-5.4 and gpt-5.4-pro\nmap to 'gpt-5.4' family\n→ gpt_5_2_prompt.md\n→ gpt-5.4-instructions.md"]

    G --> R{fallback policy?}
    R -- fallback enabled --> S["DEFAULT fallback chain:\ngpt-5.4-pro → gpt-5.4"]
    R -- strict --> T["entitlement error returned"]
Loading

Fix All in Codex

Last reviewed commit: bff8479

Add GPT-5.4 model normalization and reasoning support, plus optional GPT-5.4 Pro handling with fallback to GPT-5.4 under fallback policy.

Update templates and docs to use GPT-5.4 defaults, and expand unit/integration/property tests for 5.4 coverage.

Co-authored-by: Codex <noreply@openai.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 5, 2026

📝 Walkthrough

Walkthrough

This PR introduces support for GPT-5.4 as the new primary model, replacing GPT-5.2 across defaults and templates, and adds an optional GPT-5.4-Pro variant. Changes span configuration files, documentation updates, model mappings, request transformation logic, and comprehensive test coverage for normalization, reasoning configuration, and fallback behavior.

Changes

Cohort / File(s) Summary
Version & Changelog
package.json, CHANGELOG.md
Bump version to 5.4.2 and add changelog entry detailing GPT-5.4 runtime support, model-map normalization, and request-transform coverage for gpt-5.4 variants and fallback edges.
Configuration Files
config/opencode-modern.json, config/opencode-legacy.json
Rename model keys from gpt-5.2-* to gpt-5.4-* (none, low, medium, high, xhigh) with corresponding display name updates; gpt-5.2 → gpt-5.4 in modern template.
Configuration Documentation
config/README.md, docs/configuration.md
Update model references from gpt-5.2 to gpt-5.4; add gpt-5.4-pro as optional model; document new default fallback chain and unsupported codex fallback mappings (gpt-5.4-pro → gpt-5.4).
Product Documentation
README.md, docs/getting-started.md, docs/index.md, docs/troubleshooting.md
Replace gpt-5.2 references with gpt-5.4; introduce gpt-5.4-pro in examples and tables; update fallback descriptions and entitlement guidance for optional IDs.
Core Model Logic
lib/prompts/codex.ts, lib/request/helpers/model-map.ts, lib/request/fetch-helpers.ts, index.ts
Extend ModelFamily union and MODEL_FAMILIES array with gpt-5.4; add prompt/cache file mappings for gpt-5.4; add gpt-5.4 and gpt-5.4-pro entries to MODEL_MAP; add gpt-5.4-pro fallback chain mapping; prioritize gpt-5.4 in quota probe models list.
Request Transformation Logic
lib/request/request-transformer.ts
Expand normalizeModel to recognize gpt-5.4 and gpt-5.4-pro variants; introduce isGpt54Pro and isGpt54General boolean flags; update reasoning config logic to handle gpt-5.4 family constraints (disallow none for pro, support xhigh) and adjust default/upgrade rules.
Testing - Core Coverage
test/codex-prompts.test.ts, test/model-map.test.ts, test/fetch-helpers.test.ts
Add test cases for gpt-5.4 family in MODEL_FAMILIES, getModelFamily recognition, prompt mappings, MODEL_MAP entries, and gpt-5.4-pro fallback resolution behavior.
Testing - GPT-5.4 Specific
test/gpt54-models.test.ts
Comprehensive test suite covering normalization (all reasoning variants), MODEL_MAP validation, model family detection, reasoning configuration, edge cases, and interactions with other GPT-5 models.
Testing - Comprehensive
test/edge-cases.test.ts, test/helper-utils.test.ts, test/index.test.ts, test/request-transformer.test.ts
Large-scale addition of edge-case tests for normalizeModel, filterInput, getModelConfig, fallback resolution, token refresh, URL handling, and request transformation; add integration test for gpt-5.4-pro fallback flow; expand request-transformer tests with gpt-5.4 presets and configurations.
Testing - Property-Based & Helpers
test/property/helpers.ts, test/property/transformer.property.test.ts
Add gpt-5.4 and gpt-5.4-pro to arbitrary model/quota-key generators; update reasoning-config property tests to include gpt-5.4-pro variants and adjust reasoning-effort expectations.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Poem

🐰 A hop toward newer heights, so bright!
GPT-5.4 takes the stage,
With Pro variants turning the page,
Fallbacks and presets shine in light,
Tests cascade, making all things right! 🚀

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the primary changes: adding GPT-5.4 runtime support and refreshing documentation.
Docstring Coverage ✅ Passed Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/gpt-5.4-support-v5.4.2

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

@ndycode ndycode marked this pull request as ready for review March 5, 2026 23:31
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 5, 2026

Note

Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it.


Generating unit tests... This may take up to 20 minutes.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 5, 2026

@ndycode: Sure! I'll kick off a review of this PR right away.

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@ndycode
Copy link
Copy Markdown
Owner Author

ndycode commented Mar 5, 2026

@coderabbitai review

@greptile-ai please review this PR for runtime/model-routing correctness and test coverage completeness.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 5, 2026

@ndycode: Sure! I'll kick off a review of this PR right away.

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

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

⚠️ Outside diff range comments (1)
test/property/transformer.property.test.ts (1)

219-232: ⚠️ Potential issue | 🟡 Minor

Strengthen assertion to match the test’s stated contract (none -> low).

Current assertion only checks “not none,” which is too permissive for this behavior.

🔧 Proposed test fix
-          expect(result.effort).not.toBe("none");
+          expect(result.effort).toBe("low");
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/property/transformer.property.test.ts` around lines 219 - 232, The test
currently only asserts that getReasoningConfig(model, { reasoningEffort: "none"
}).effort is not "none"; change the assertion to assert it equals "low" so it
matches the test contract ("none -> low"). Update the assertion in the
"codex/pro models upgrade none to low" case to expect result.effort === "low"
for the models passed into getReasoningConfig.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@CHANGELOG.md`:
- Line 28: The changelog entry header "## [5.4.2] - 2026-03-06" uses a future
date; update the header in CHANGELOG.md by either moving the entry under the
"Unreleased" section (rename to "## [5.4.2]" or place its content under the
existing "Unreleased" heading) or change the date to the actual published date
(e.g., "2026-03-05" or the real release date) so the heading no longer shows a
future date.

In `@docs/development/TESTING.md`:
- Around line 620-625: The document's example outputs and test snippets still
show the older fallback "gpt-5" and must be updated to the new fallback
"gpt-5.1": search for occurrences in this file that reference normalizeModel
outputs or unit-test examples expecting "gpt-5" (including cases for undefined
or unknown inputs like "random-model") and replace them with "gpt-5.1" (also
update variants like "GPT 5" examples to match normalized forms such as
"gpt-5.1" or "gpt-5.4-pro" as appropriate); ensure every mention of
normalizeModel, its example results, and any future unit test snippets
consistently reflect the new fallback value "gpt-5.1".

---

Outside diff comments:
In `@test/property/transformer.property.test.ts`:
- Around line 219-232: The test currently only asserts that
getReasoningConfig(model, { reasoningEffort: "none" }).effort is not "none";
change the assertion to assert it equals "low" so it matches the test contract
("none -> low"). Update the assertion in the "codex/pro models upgrade none to
low" case to expect result.effort === "low" for the models passed into
getReasoningConfig.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 4139efe6-534d-4935-89a0-9fffc1df8e82

📥 Commits

Reviewing files that changed from the base of the PR and between 9e4fb2b and 0fbd443.

📒 Files selected for processing (23)
  • CHANGELOG.md
  • README.md
  • config/README.md
  • config/opencode-legacy.json
  • config/opencode-modern.json
  • docs/configuration.md
  • docs/development/TESTING.md
  • docs/getting-started.md
  • docs/index.md
  • docs/troubleshooting.md
  • index.ts
  • lib/prompts/codex.ts
  • lib/request/fetch-helpers.ts
  • lib/request/helpers/model-map.ts
  • lib/request/request-transformer.ts
  • package.json
  • test/codex-prompts.test.ts
  • test/fetch-helpers.test.ts
  • test/index.test.ts
  • test/model-map.test.ts
  • test/property/helpers.ts
  • test/property/transformer.property.test.ts
  • test/request-transformer.test.ts

Resolve changelog date, restore missing GPT-5.2 codex normalization coverage, add gpt-5.4-pro-none model mapping, and tighten property assertion semantics.

Align testing-guide normalizeModel examples with current gpt-5 -> gpt-5.1 fallback behavior.

Co-authored-by: Codex <noreply@openai.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

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)
docs/configuration.md (1)

349-354: ⚠️ Potential issue | 🟡 Minor

Add a language to the fenced log snippet (markdownlint MD040).

The code fence under “look for:” is missing a language identifier and will trigger lint warnings.

🛠️ Suggested doc-lint fix
 look for:
-```
+```text
 [openai-codex-plugin] Model config lookup: "gpt-5.4" → normalized to "gpt-5.4" for API {
   hasModelSpecificConfig: true,
   resolvedConfig: { ... }
 }
</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against the current code and only fix it if needed.

In @docs/configuration.md around lines 349 - 354, The fenced log snippet under
"look for:" is missing a language identifier and triggers markdownlint MD040;
update the fenced code block that contains the line beginning with
"[openai-codex-plugin] Model config lookup: "gpt-5.4" → normalized..." by adding
a language tag (e.g., use "text") after the opening triple backticks so the
block becomes text ... to satisfy the linter.


</details>

</blockquote></details>

</blockquote></details>
♻️ Duplicate comments (1)
docs/development/TESTING.md (1)

608-625: ⚠️ Potential issue | 🟡 Minor

Normalization docs are still internally inconsistent with current fallback outputs.

The updated examples here are correct, but later snippets still assert old outputs (e.g., Line 726, Line 729-730, Line 744-747). Please align those future-unit-test expectations to current behavior (gpt-5.1 fallback and gpt-5.1-codex-mini).

📝 Suggested doc fix (future-unit-test snippet)
-    expect(normalizeModel('gpt-5')).toBe('gpt-5')
+    expect(normalizeModel('gpt-5')).toBe('gpt-5.1')
     expect(normalizeModel('gpt-5-codex')).toBe('gpt-5-codex')
-    expect(normalizeModel('gpt-5-codex-mini')).toBe('codex-mini-latest')
-    expect(normalizeModel('gpt-5-mini')).toBe('gpt-5')
-    expect(normalizeModel('gpt-5-nano')).toBe('gpt-5')
+    expect(normalizeModel('gpt-5-codex-mini')).toBe('gpt-5.1-codex-mini')
+    expect(normalizeModel('gpt-5-mini')).toBe('gpt-5.1')
+    expect(normalizeModel('gpt-5-nano')).toBe('gpt-5.1')
...
-    expect(normalizeModel('openai/gpt-5-codex-mini-high')).toBe('codex-mini-latest')
-    expect(normalizeModel('gpt-5-high')).toBe('gpt-5')
+    expect(normalizeModel('openai/gpt-5-codex-mini-high')).toBe('gpt-5.1-codex-mini')
+    expect(normalizeModel('gpt-5-high')).toBe('gpt-5.1')
...
-    expect(normalizeModel(undefined)).toBe('gpt-5')
-    expect(normalizeModel('codex-mini-latest')).toBe('codex-mini-latest')
-    expect(normalizeModel('random')).toBe('gpt-5')
+    expect(normalizeModel(undefined)).toBe('gpt-5.1')
+    expect(normalizeModel('codex-mini-latest')).toBe('gpt-5.1-codex-mini')
+    expect(normalizeModel('random')).toBe('gpt-5.1')
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/development/TESTING.md` around lines 608 - 625, The later
"future-unit-test" examples in TESTING.md are still using old expected outputs
for normalizeModel; update those assertions to match the current behavior: use
"gpt-5.1" as the generic fallback for undefined/random inputs and use
"gpt-5.1-codex-mini" for codex-mini mappings (e.g., replace any old expectations
that assert "gpt-5" or other legacy strings with "gpt-5.1", and replace
codex-mini legacy expectations with "gpt-5.1-codex-mini"), ensuring all
normalizeModel examples and test snippets consistently match the examples shown
earlier.
🧹 Nitpick comments (1)
test/index.test.ts (1)

1401-1403: Tighten this test’s isolation from legacy fallback toggles.

This case is validating gpt-5.4-pro -> gpt-5.4, but Line 1402 also enables the GPT-5.3 legacy fallback toggle. Keeping that toggle off here makes the assertion more regression-proof for the 5.4 path.

♻️ Suggested test tightening
 		vi.mocked(configModule.getFallbackOnUnsupportedCodexModel).mockReturnValueOnce(true);
-		vi.mocked(configModule.getFallbackToGpt52OnUnsupportedGpt53).mockReturnValueOnce(true);
+		vi.mocked(configModule.getFallbackToGpt52OnUnsupportedGpt53).mockReturnValueOnce(false);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/index.test.ts` around lines 1401 - 1403, The test is enabling the
GPT-5.3 legacy fallback toggle which blurs validation of the gpt-5.4-pro ->
gpt-5.4 path; update the mock for getFallbackToGpt52OnUnsupportedGpt53 to return
false
(vi.mocked(configModule.getFallbackToGpt52OnUnsupportedGpt53).mockReturnValueOnce(false))
so only getFallbackOnUnsupportedCodexModel remains true and other mocks (e.g.,
fetchHelpers.transformRequestForCodex) stay unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@docs/configuration.md`:
- Around line 349-354: The fenced log snippet under "look for:" is missing a
language identifier and triggers markdownlint MD040; update the fenced code
block that contains the line beginning with "[openai-codex-plugin] Model config
lookup: "gpt-5.4" → normalized..." by adding a language tag (e.g., use "text")
after the opening triple backticks so the block becomes ```text ... ``` to
satisfy the linter.

---

Duplicate comments:
In `@docs/development/TESTING.md`:
- Around line 608-625: The later "future-unit-test" examples in TESTING.md are
still using old expected outputs for normalizeModel; update those assertions to
match the current behavior: use "gpt-5.1" as the generic fallback for
undefined/random inputs and use "gpt-5.1-codex-mini" for codex-mini mappings
(e.g., replace any old expectations that assert "gpt-5" or other legacy strings
with "gpt-5.1", and replace codex-mini legacy expectations with
"gpt-5.1-codex-mini"), ensuring all normalizeModel examples and test snippets
consistently match the examples shown earlier.

---

Nitpick comments:
In `@test/index.test.ts`:
- Around line 1401-1403: The test is enabling the GPT-5.3 legacy fallback toggle
which blurs validation of the gpt-5.4-pro -> gpt-5.4 path; update the mock for
getFallbackToGpt52OnUnsupportedGpt53 to return false
(vi.mocked(configModule.getFallbackToGpt52OnUnsupportedGpt53).mockReturnValueOnce(false))
so only getFallbackOnUnsupportedCodexModel remains true and other mocks (e.g.,
fetchHelpers.transformRequestForCodex) stay unchanged.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: dbfe1c40-56af-48a2-986e-2143bc56e7e9

📥 Commits

Reviewing files that changed from the base of the PR and between 9e4fb2b and 0fbd443.

📒 Files selected for processing (23)
  • CHANGELOG.md
  • README.md
  • config/README.md
  • config/opencode-legacy.json
  • config/opencode-modern.json
  • docs/configuration.md
  • docs/development/TESTING.md
  • docs/getting-started.md
  • docs/index.md
  • docs/troubleshooting.md
  • index.ts
  • lib/prompts/codex.ts
  • lib/request/fetch-helpers.ts
  • lib/request/helpers/model-map.ts
  • lib/request/request-transformer.ts
  • package.json
  • test/codex-prompts.test.ts
  • test/fetch-helpers.test.ts
  • test/index.test.ts
  • test/model-map.test.ts
  • test/property/helpers.ts
  • test/property/transformer.property.test.ts
  • test/request-transformer.test.ts

@ndycode
Copy link
Copy Markdown
Owner Author

ndycode commented Mar 5, 2026

Addressed all review findings in commit beb88bd.

Also fixed the outside-diff review note by tightening the property test assertion in test/property/transformer.property.test.ts from not.toBe("none") to toBe("low") for the codex/pro none -> low contract.

Validation rerun after fixes:

  • npm run typecheck
  • npm run lint
  • npm test (56 files, 1798 tests)

@ndycode
Copy link
Copy Markdown
Owner Author

ndycode commented Mar 5, 2026

@greptile-ai please re-review latest commit �eb88bd.

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 5, 2026

@ndycode: Thanks for the detailed update! I'll kick off a fresh incremental review of the latest commit (beb88bd) now. The tightened property-test assertion (toBe("low") instead of not.toBe("none")) is a good improvement — it makes the none → low upgrade contract explicit and future-proof.

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Add language tag to configuration log snippet and isolate GPT-5.4-pro fallback test from legacy GPT-5.3 fallback toggle.

Co-authored-by: Codex <noreply@openai.com>
@ndycode
Copy link
Copy Markdown
Owner Author

ndycode commented Mar 5, 2026

Followed up on review #3900326193 in commit d9f8617.

Changes applied:

  • docs/configuration.md: changed the "look for:" fence from totext to satisfy MD040.
  • test/index.test.ts: set getFallbackToGpt52OnUnsupportedGpt53 mock to false in the GPT-5.4-pro fallback test to isolate that behavior from legacy GPT-5.3 fallback toggles.

Validation after this commit:

  • npm run typecheck
  • npm run lint
  • npm test (56 files, 1798 tests)

@ndycode
Copy link
Copy Markdown
Owner Author

ndycode commented Mar 5, 2026

@coderabbitai review

@greptile-ai please re-review latest commit d9f8617.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 5, 2026

✅ Unit tests committed locally. Commit: e26b0ee33a3798522660116bb3fbdc6e5a2aeaed

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 5, 2026

@ndycode: Thanks for the follow-up commit! I'll kick off an incremental review of commit d9f8617 now. The two targeted changes look clean — fixing the bare code fence in docs/configuration.md to text for MD040 compliance, and tightening the test/index.test.ts mock isolation for the GPT-5.4-pro fallback test are both good hygiene improvements.

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

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

⚠️ Outside diff range comments (1)
docs/configuration.md (1)

38-52: ⚠️ Potential issue | 🟡 Minor

Clarify none coercion for gpt-5.4-pro explicitly.

The table is correct to omit none for gpt-5.4-pro, but the explanatory bullet currently mentions only codex. Adding pro here will better match runtime behavior.

📝 Suggested wording update
-- `none` - no reasoning phase (base models only, auto-converts to `low` for codex)
+- `none` - no reasoning phase (base models only; auto-converts to `low` for codex/pro families, including `gpt-5.4-pro`)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/configuration.md` around lines 38 - 52, Update the explanatory bullet
about "none" so it explicitly states that "none" is coerced to "low" not only
for codex models but also for pro models such as gpt-5.4-pro; reference the
table entries like `gpt-5.4-pro` and `gpt-5-codex` and change the line "- `none`
- no reasoning phase (base models only, auto-converts to `low` for codex)" to
indicate it auto-converts to `low` for both codex and pro models (e.g.,
`gpt-5-codex`, `gpt-5.4-pro`).
🧹 Nitpick comments (2)
lib/request/request-transformer.ts (1)

82-93: Tighten GPT-5.4/GPT-5.4-pro normalization patterns.

These includes(...) checks are overly broad and can normalize unintended model IDs that merely contain gpt-5.4 as a substring.

🔧 Suggested safer normalization patterns
-	if (
-		normalized.includes("gpt-5.4-pro") ||
-		normalized.includes("gpt 5.4 pro")
-	) {
+	if (/\bgpt(?:-| )5\.4(?:-| )pro(?:\b|[- ])/.test(normalized)) {
 		return "gpt-5.4-pro";
 	}
@@
-	if (normalized.includes("gpt-5.4") || normalized.includes("gpt 5.4")) {
+	if (/\bgpt(?:-| )5\.4(?:\b|[- ])/.test(normalized)) {
 		return "gpt-5.4";
 	}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/request/request-transformer.ts` around lines 82 - 93, The current
includes(...) checks for "gpt-5.4-pro" and "gpt-5.4" are too permissive and may
match unintended substrings; update the checks in request-transformer.ts to use
stricter matching on the normalized string (e.g., a regex or tokenized check)
that enforces separators or boundaries around the model id so only exact IDs
like "gpt-5.4-pro" or "gpt-5.4" (and their space variants) match; keep the "pro"
branch checked first (the existing normalized.includes("gpt-5.4-pro") /
normalized.includes("gpt 5.4 pro") logic) and replace the includes(...) calls
with a regex.test(...) or a token-array lookup against normalized to avoid
matching names that merely contain "gpt-5.4" as a substring.
lib/prompts/codex.ts (1)

123-125: Use token-aware matching for GPT-5.4 family detection.

includes("gpt-5.4") can also match unrelated version strings (e.g., gpt-5.40). Prefer boundary-aware matching to avoid accidental family routing.

♻️ Suggested matcher refinement
-	if (normalizedModel.includes("gpt-5.4") || normalizedModel.includes("gpt 5.4")) {
+	if (/\bgpt(?:-| )5\.4(?:\b|[- ])/i.test(normalizedModel)) {
 		return "gpt-5.4";
 	}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/prompts/codex.ts` around lines 123 - 125, The current family detection
uses normalizedModel.includes("gpt-5.4") which can false-match strings like
"gpt-5.40"; change the check on normalizedModel to a token/boundary-aware match
(e.g., use a regex with word boundaries such as /\bgpt[- ]5\.4\b/i or split
tokens and compare exact token values) so only the exact "gpt-5.4" family (or
"gpt 5.4") is matched; update the conditional that references normalizedModel in
the block returning "gpt-5.4".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@test/gpt54-models.test.ts`:
- Around line 64-75: Add explicit assertions for the missing "gpt-5.4-pro-none"
variant in the test block that checks pro reasoning-effort normalization: update
the loop (or add an assertion) so that normalizeModel("gpt-5.4-pro-none") and
getNormalizedModel("gpt-5.4-pro-none") both expect "gpt-5.4-pro" to prevent a
regression if the map key is removed; apply the same explicit check in the other
similar test locations noted (lines around 94-100 and 282-287) referencing the
normalizeModel and getNormalizedModel functions.

---

Outside diff comments:
In `@docs/configuration.md`:
- Around line 38-52: Update the explanatory bullet about "none" so it explicitly
states that "none" is coerced to "low" not only for codex models but also for
pro models such as gpt-5.4-pro; reference the table entries like `gpt-5.4-pro`
and `gpt-5-codex` and change the line "- `none` - no reasoning phase (base
models only, auto-converts to `low` for codex)" to indicate it auto-converts to
`low` for both codex and pro models (e.g., `gpt-5-codex`, `gpt-5.4-pro`).

---

Nitpick comments:
In `@lib/prompts/codex.ts`:
- Around line 123-125: The current family detection uses
normalizedModel.includes("gpt-5.4") which can false-match strings like
"gpt-5.40"; change the check on normalizedModel to a token/boundary-aware match
(e.g., use a regex with word boundaries such as /\bgpt[- ]5\.4\b/i or split
tokens and compare exact token values) so only the exact "gpt-5.4" family (or
"gpt 5.4") is matched; update the conditional that references normalizedModel in
the block returning "gpt-5.4".

In `@lib/request/request-transformer.ts`:
- Around line 82-93: The current includes(...) checks for "gpt-5.4-pro" and
"gpt-5.4" are too permissive and may match unintended substrings; update the
checks in request-transformer.ts to use stricter matching on the normalized
string (e.g., a regex or tokenized check) that enforces separators or boundaries
around the model id so only exact IDs like "gpt-5.4-pro" or "gpt-5.4" (and their
space variants) match; keep the "pro" branch checked first (the existing
normalized.includes("gpt-5.4-pro") / normalized.includes("gpt 5.4 pro") logic)
and replace the includes(...) calls with a regex.test(...) or a token-array
lookup against normalized to avoid matching names that merely contain "gpt-5.4"
as a substring.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 39a27cbd-54df-486d-9c50-fe29fe854561

📥 Commits

Reviewing files that changed from the base of the PR and between 9e4fb2b and e26b0ee.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (26)
  • CHANGELOG.md
  • README.md
  • config/README.md
  • config/opencode-legacy.json
  • config/opencode-modern.json
  • docs/configuration.md
  • docs/development/TESTING.md
  • docs/getting-started.md
  • docs/index.md
  • docs/troubleshooting.md
  • index.ts
  • lib/prompts/codex.ts
  • lib/request/fetch-helpers.ts
  • lib/request/helpers/model-map.ts
  • lib/request/request-transformer.ts
  • package.json
  • test/codex-prompts.test.ts
  • test/edge-cases.test.ts
  • test/fetch-helpers.test.ts
  • test/gpt54-models.test.ts
  • test/helper-utils.test.ts
  • test/index.test.ts
  • test/model-map.test.ts
  • test/property/helpers.ts
  • test/property/transformer.property.test.ts
  • test/request-transformer.test.ts

- make gpt-5.4/pro family matching boundary-aware in runtime + prompt family detection
- clarify docs for none->low coercion across codex/pro families
- add regression tests for gpt-5.40/gpt-5.4pro non-matches and gpt-5.4-pro-none coverage

Co-authored-by: Codex <noreply@openai.com>
@ndycode
Copy link
Copy Markdown
Owner Author

ndycode commented Mar 6, 2026

Addressed remaining review issues in bff8479 with targeted hardening and regression coverage.

What was fixed

  • Replaced broad substring checks with boundary-aware matching for GPT-5.4 family detection:
    • lib/request/request-transformer.ts
    • lib/prompts/codex.ts
  • Clarified docs for reasoning effort coercion:
    • docs/configuration.md now explicitly states none -> low applies to codex/pro families (including gpt-5.4-pro).
  • Expanded regression tests to close model-id edge cases and map coverage:
    • test/gpt54-models.test.ts
      • Added explicit gpt-5.4-pro-none assertions.
      • Tightened pro-variant count expectation to include none.
      • Updated edge-case expectation so gpt-5.40/gpt-5.44 do not match gpt-5.4.
    • test/request-transformer.test.ts
      • Added checks that gpt-5.40 and gpt-5.4pro do not normalize to gpt-5.4.
    • test/codex-prompts.test.ts
      • Added check that gpt-5.40 is not classified as gpt-5.4 family.

Validation

  • npm run typecheck
  • npm run lint
  • npm test ✅ (59 files, 1935 tests)

@coderabbitai review
@greptile-ai please re-review latest commit bff8479

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 6, 2026

@ndycode: Thanks for the thorough hardening in bff8479 — the changes look well-targeted. A few highlights worth noting:

  • Boundary-aware matching in lib/request/request-transformer.ts and lib/prompts/codex.ts is the right call — substring checks for model IDs with numeric suffixes (like 5.4) are a common source of false positives, and the added regression tests for gpt-5.40, gpt-5.44, and gpt-5.4pro give solid proof the boundaries hold.
  • gpt-5.4-pro-none map entry + assertions directly closes the Greptile-flagged gap about getNormalizedModel("gpt-5.4-pro-none")undefined.
  • Pro-variant count tightening (including none) makes the model-map completeness check self-enforcing going forward.
  • The jump from 1798 → 1935 tests across 59 files is a meaningful coverage expansion, not just noise.

I'll kick off an incremental review of bff8479 now.

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@lib/request/request-transformer.ts`:
- Around line 445-453: The model-name checks using string.includes on
normalizedName (for isGpt54Pro and isGpt54General) are too permissive and can
mis-set capability flags like supportsXhigh/supportsNone; update the logic in
request-transformer.ts to perform boundary-safe matching (e.g., use a regex with
word boundaries or tokenized comparisons) when computing isGpt54Pro and
isGpt54General so only exact family names like "gpt-5.4", "gpt-5.4-pro", "gpt
5.4", or "gpt 5.4 pro" match; apply the same boundary-safe fix to the other
gpt-5.4 checks in this file that influence reasoning detection (the similar
checks referenced near the second occurrence). Ensure the changed predicates
still respect the existing negation (!isGpt54Pro) for isGpt54General.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 9a7325c6-1070-41b2-ad34-79cf0fb132c7

📥 Commits

Reviewing files that changed from the base of the PR and between e26b0ee and bff8479.

📒 Files selected for processing (6)
  • docs/configuration.md
  • lib/prompts/codex.ts
  • lib/request/request-transformer.ts
  • test/codex-prompts.test.ts
  • test/gpt54-models.test.ts
  • test/request-transformer.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
  • test/codex-prompts.test.ts
  • test/gpt54-models.test.ts
  • test/request-transformer.test.ts
  • lib/prompts/codex.ts

@ndycode ndycode merged commit 15b7f01 into main Mar 6, 2026
3 checks passed
@ndycode ndycode deleted the feat/gpt-5.4-support-v5.4.2 branch March 6, 2026 04:57
ndycode added a commit that referenced this pull request Apr 6, 2026
feat: add GPT-5.4 support and docs refresh
ndycode added a commit that referenced this pull request Apr 6, 2026
feat: add GPT-5.4 support and docs refresh
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