fix(codex): preserve routed models in desktop picker - #1244
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughNative-alias combos are now opt-in and require supported native slugs plus display names. Catalog generation, visibility, recovery, routing, CLI, GUI editing, localized documentation, and regression coverage now support the behavior. ChangesNative alias compatibility
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant CLI_GUI
participant ComboConfig
participant CatalogSync
participant ModelVisibility
CLI_GUI->>ComboConfig: Set nativeAlias and displayName
ComboConfig->>CatalogSync: Provide normalized native-alias combo
CatalogSync->>ModelVisibility: Apply shadowing and disabled selectors
ModelVisibility-->>CLI_GUI: Return visible combo and native rows
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
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. Comment |
|
✅ Deterministic PR hygiene checks passed. |
✅ READY
Hygiene✅ Deterministic PR hygiene checks passed. |
|
@codex review Please review this replacement PR against current |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@coderabbitai review |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 25
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/maintainer-full-verify-1244.yml:
- Around line 3-6: Add a branch-scoped concurrency group to the workflow’s
top-level configuration so pushes to the same branch run serially. Update the
cleanup/removal step to tolerate the workflow already having been successfully
removed by an earlier run, while preserving normal failure reporting for other
errors.
- Around line 33-34: Update the high-severity dependency audit invoked by the
“High severity dependency audit” workflow step to also run `bun audit
--audit-level=high` from the `docs-site` directory, while preserving the
existing root and gui audits.
- Around line 16-21: Replace the mutable action references in the workflow’s
checkout and Bun setup steps with verified full commit SHA references, reusing
the corresponding SHA-pinned versions from the CI workflow while preserving
their existing configuration.
- Around line 8-9: The full verification job grants excessive repository write
access and persists checkout credentials. Change its permissions to contents:
read and configure actions/checkout with persist-credentials: false; move only
the trusted maintainer cleanup into a separate job with contents: write,
ensuring it does not execute repository-provided install, build, audit, or
verification code, and protect the maintainer/supersede-1056-native-alias branch
and its writers.
In `@docs-site/src/content/docs/zh-cn/guides/codex-app-models.md`:
- Around line 85-86: Update the native-alias behavior description in the Chinese
guide to state that shadowed bare native rows are omitted from the Models page
and have no native switch; only unshadowed native rows retain a switch and can
restore native metadata when re-enabled.
In `@gui/tests/combo-native-alias-editor.test.tsx`:
- Around line 86-95: Re-query the `#cwi-edit-display-name` input after
nativeAlias.click() and before calling setInputValue, while retaining the
existing pre-toggle expect(displayName).toBeTruthy() assertion. Use the
refreshed element for the post-toggle input event so the test targets the
current DetailPanel DOM node.
- Around line 96-98: Update ComboWorkspaceDetailPanel’s create and save action
buttons with stable selectors such as the specified IDs or data-testid
attributes, then revise the corresponding test queries around the create and
save clicks to use those selectors instead of rendered textContent.
In `@src/codex/catalog/aggregation.ts`:
- Around line 258-266: Update src/codex/catalog/aggregation.ts lines 258-266 in
the combo visibility logic to use only the canonical combo/<id> selector for
native-alias combos, while retaining the bare alias selector for native-row
visibility. Update src/codex/catalog/provider-fetch.ts lines 1199-1207 so native
target metadata remains available to native-alias combos when that bare alias is
disabled for the native row; add regression coverage for a disabled bare alias
with an openai/<alias> combo target.
In `@src/codex/catalog/metadata.ts`:
- Line 217: Update mergeCatalogEntriesForSync() so preserved native-alias
entries also pass through the exactComboCatalogSlugs() disabled-combo filtering
before or during the final merge. Ensure native aliases whose combo/<id> is in
disabledModels are removed from the transitory saved-route result instead of
remaining visible, while retaining enabled aliases.
In `@src/codex/catalog/sync.ts`:
- Around line 495-520: Update mergeCatalogModelsWithNativeRecovery to create a
fully independent clone of each recovered entry before adding it to merged,
replacing the selective supported_reasoning_levels copy. Use the existing
structured-clone mechanism available in the project so nested fields such as
input_modalities, service_tiers, and metadata cannot remain shared with
nativeRecoverySources.
- Around line 337-344: Update the nativeAliasesBySlug construction in the sync
flow to detect duplicate bare aliases among COMBO_NAMESPACE native aliases
instead of overwriting earlier models. Preserve the first occurrence for each
alias, add subsequent matching combo models to collisionSkipped, and ensure they
are excluded or warned consistently before routed aliases are emitted.
- Around line 662-665: Update the input_modalities filtering logic near the
preserved-entry check to retain bare native aliases whose slug is present in
exactComboSlugs, even when input_modalities is missing or empty. Ensure this
retention is applied consistently with retainedNativeAlias so these rows are not
dropped during disk-loaded catalog filtering.
- Line 976: Update buildCatalogEntries() to exclude suppressedBareNativeSlugs
from nativeEntries before creating account-bound selector/native-slug clones,
preventing disabled desktop-hidden native slugs from being emitted with list
visibility. Ensure clones for slugs removed from suppression are restored before
applyNativeVisibility() runs, while preserving existing behavior for
unsuppressed native entries.
In `@src/codex/convergence.ts`:
- Around line 186-195: Replace the intermediate preservedNativeAliases array
with direct construction of preservedNativeAliasSlugs from active.models,
retaining the existing preservingExistingRouted guard and both predicates. Leave
the independently pushed-row logic unchanged.
In `@src/server/management/combo-routes.ts`:
- Around line 127-138: Update the PUT response in the combo route to return the
persisted `stored` object instead of the pre-strip `normalized` object, while
preserving the existing storage behavior in the normalization block. This keeps
PUT responses consistent with GET responses and ensures both use the same
key-omitting shape.
In `@tests/codex-catalog.test.ts`:
- Line 424: Loosen the warning assertion in the test around the preservation
case so it checks only the stable message portion, such as the preservation
count and context, rather than exact singular wording. Update the expect call
for warning to avoid coupling it to pluralization while still verifying that one
existing routed entry is preserved.
- Around line 769-816: Update the native-alias assertions in the test to derive
context windows, input modalities, reasoning efforts, and default reasoning
efforts from the native snapshot helpers used by completeNativeAliasMembers,
including nativeOpenAiContextWindow, nativeInputModalities, and
nativeReasoningEfforts. Assert that each alias inherits those source values
rather than duplicating literals, while preserving the existing alias-specific
expectations.
- Around line 351-367: Refactor the test setup around mergeCatalogEntriesForSync
so this 15-argument call uses a test-local helper with named options. Ensure
disabledModels, goIds, gatheredProviderNames, and exactComboSlugs are explicitly
assigned to their intended parameters, preserving the gpt-5.6-sol survival
assertion as a disabled-model scenario rather than relying on positional set
ordering.
In `@tests/codex-convergence-contract.test.ts`:
- Around line 234-269: The test fixture does not prove that alias generation or
routed-entry backup handling executes. Update the convergence test around
liveConfig, gatherCodexCatalogCandidate, and commitCodexCatalogCandidate to
assert a generation-only field such as the routed description or distinct
priority, add a case with the active catalog cleared, and verify the published
backup; ensure the fixture exercises bare-slug alias routing and exposes
incorrect catalogHasRoutedEntries behavior.
- Line 274: Strengthen the gpt-5.5 assertion in the recovery test by locating
the matching entry and asserting its display_name equals the bundled fixture
value "GPT-5.5", rather than only checking that an entry exists. Keep the test
focused on metadata recovered through mergeCatalogModelsWithNativeRecovery.
- Line 219: Replace the `as never` cast in the call to
`setBundledCatalogCacheForTests` with the directly typed `bundled` value. If
needed, annotate `bundled` as `RawCatalog & { models: RawEntry[] }` at its
declaration, preserving the existing models data while allowing TypeScript to
validate future signature changes.
In `@tests/combo-management-api.test.ts`:
- Around line 338-357: Rename the test case around comboApi PUT validation to
“PUT rejects a native alias without a displayName,” matching the omitted
displayName and nativeAlias: true inputs. Optionally add a sibling test for a
native alias that omits nativeAlias while preserving the existing no-mutation
assertions.
In `@tests/model-visibility-management-api.test.ts`:
- Around line 283-289: Update the disable-step assertion in the test around put
and loadConfig to compare the complete disabledModels array with toEqual,
including both the preserved bare native key gpt-5.6-sol and the newly disabled
combo/nova entry in the handler’s actual order.
In `@tests/native-alias-maintainer-regressions.test.ts`:
- Around line 45-51: Update the test around mergeCatalogModelsWithNativeRecovery
to locate the recovered row by its expected slug instead of assuming merged[0]
is the target. Assert that the slug-matched row exists before accessing
supported_reasoning_levels, then keep the deep-copy and source-immutability
assertions unchanged.
In `@tests/native-profile-drain-server.test.ts`:
- Around line 290-293: Extract the repeated bounded polling logic into a shared
helper in the test module, preserving the 2,000 ms deadline and Bun.sleep(10)
interval. Replace both loops near the upstream-close checks with calls to that
helper, including the loops around the existing upstream close assertions, while
preserving their current conditions and behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 50ee5be5-27f1-4608-b56a-3c2f1b207cf0
📒 Files selected for processing (58)
.github/workflows/maintainer-full-verify-1244.ymldocs-site/src/content/docs/guides/codex-app-models.mddocs-site/src/content/docs/guides/combos.mddocs-site/src/content/docs/ja/guides/codex-app-models.mddocs-site/src/content/docs/ja/guides/combos.mddocs-site/src/content/docs/ja/reference/configuration/routing.mddocs-site/src/content/docs/ko/guides/codex-app-models.mddocs-site/src/content/docs/ko/guides/combos.mddocs-site/src/content/docs/ko/reference/configuration/routing.mddocs-site/src/content/docs/reference/cli/agents.mddocs-site/src/content/docs/reference/configuration/routing.mddocs-site/src/content/docs/ru/guides/codex-app-models.mddocs-site/src/content/docs/ru/guides/combos.mddocs-site/src/content/docs/ru/reference/configuration/routing.mddocs-site/src/content/docs/zh-cn/guides/codex-app-models.mddocs-site/src/content/docs/zh-cn/guides/combos.mddocs-site/src/content/docs/zh-cn/reference/cli/agents.mddocs-site/src/content/docs/zh-cn/reference/configuration/routing.mdgui/src/combo-workspace-data.tsgui/src/components/combo-workspace-detail-panel.tsxgui/src/i18n/de.tsgui/src/i18n/en.tsgui/src/i18n/ja.tsgui/src/i18n/ko.tsgui/src/i18n/ru.tsgui/src/i18n/zh.tsgui/tests/combo-native-alias-editor.test.tsxgui/tests/combo-workspace-dirty.test.tsxsrc/cli/combo.tssrc/codex/catalog.tssrc/codex/catalog/aggregation.tssrc/codex/catalog/effort.tssrc/codex/catalog/kinds.tssrc/codex/catalog/metadata.tssrc/codex/catalog/native-models.tssrc/codex/catalog/parsing.tssrc/codex/catalog/provider-fetch.tssrc/codex/catalog/sync.tssrc/codex/convergence.tssrc/combos/index.tssrc/combos/types.tssrc/server/index.tssrc/server/management/combo-routes.tssrc/server/management/model-routes.tssrc/server/management/model-rows.tssrc/types.tsstructure/03_catalog-and-subagents.mdtests/cli-headless-parity.test.tstests/codex-catalog-sync-hardening.test.tstests/codex-catalog.test.tstests/codex-convergence-contract.test.tstests/combo-management-api.test.tstests/combo-workspace-data.test.tstests/combos.test.tstests/model-visibility-management-api.test.tstests/native-alias-maintainer-regressions.test.tstests/native-model-toggle.test.tstests/native-profile-drain-server.test.ts
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2402848a83
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/codex/convergence.ts (1)
253-258: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winKeep preserved native aliases without
input_modalities.When routed discovery returns no rows, Lines 241-251 preserve exact native aliases from the active catalog. Lines 253-258 then remove those same rows if
input_modalitiesis absent or empty.A transient discovery failure can therefore delete a valid bare native alias from the converged catalog. Retain
isNativeAliasCatalogEntry(entry)rows whenpreservingExistingRoutedandexact.has(entry.slug)are both true. Add a regression test with an active native-alias row that has noinput_modalities.Proposed fix
if (!hasPhysicalComboProvider) { const exact = exactComboSlugs; - catalog.models = entries.filter(entry => ( - typeof entry.slug !== "string" || !exact.has(entry.slug) - || (Array.isArray(entry.input_modalities) && entry.input_modalities.length > 0) - )); + catalog.models = entries.filter(entry => { + const slug = typeof entry.slug === "string" ? entry.slug : ""; + const retainedNativeAlias = preservingExistingRouted + && isNativeAliasCatalogEntry(entry) + && exact.has(slug); + return retainedNativeAlias + || !exact.has(slug) + || (Array.isArray(entry.input_modalities) && entry.input_modalities.length > 0); + }); }This follows the preserved-alias contract in
src/codex/catalog/sync.tsLines 673-681.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/codex/convergence.ts` around lines 253 - 258, Update the filtering logic in the hasPhysicalComboProvider block of convergence so entries satisfying preservingExistingRouted, exact.has(entry.slug), and isNativeAliasCatalogEntry(entry) are retained even when input_modalities is missing or empty. Preserve the existing filtering behavior for other entries, and add a regression test covering an active bare native-alias catalog row during routed discovery failure.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/codex/convergence.ts`:
- Around line 253-258: Update the filtering logic in the
hasPhysicalComboProvider block of convergence so entries satisfying
preservingExistingRouted, exact.has(entry.slug), and
isNativeAliasCatalogEntry(entry) are retained even when input_modalities is
missing or empty. Preserve the existing filtering behavior for other entries,
and add a regression test covering an active bare native-alias catalog row
during routed discovery failure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2c7e602f-d8bc-4de4-a007-f02560c31361
📒 Files selected for processing (13)
docs-site/src/content/docs/zh-cn/guides/codex-app-models.mdgui/src/components/combo-workspace-detail-panel.tsxgui/tests/combo-native-alias-editor.test.tsxsrc/codex/catalog/provider-fetch.tssrc/codex/catalog/sync.tssrc/codex/convergence.tssrc/server/management/combo-routes.tstests/codex-catalog.test.tstests/codex-convergence-contract.test.tstests/combo-management-api.test.tstests/model-visibility-management-api.test.tstests/native-alias-maintainer-regressions.test.tstests/native-profile-drain-server.test.ts
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/codex/convergence.ts (1)
241-252: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
prepareCatalogdrops preserved native aliases that carry noinput_modalities;mergeCatalogEntriesForSynckeeps them.Line 245 preserves an alias row from the active catalog when its slug is in
exactComboSlugs. The filter at Line 253-258 then removes any entry whose slug is inexactComboSlugsunlessinput_modalitiesis a non-empty array. The preserved alias row has no exemption, so a disk row withoutinput_modalitiesis preserved and then discarded in the same function.
src/codex/catalog/sync.tsadded exactly that exemption at Line 673-681 (retainedNativeAlias || !exactComboSlugs.has(slug) || ...). The two catalog paths therefore disagree for the same input.tests/native-alias-maintainer-regressions.test.tsLine 90-104 covers only themergeCatalogEntriesForSyncpath, so the convergence path has no regression test for it.Apply the same retention rule here and add a matching case to
tests/codex-convergence-contract.test.ts.🐛 Proposed fix — mirror the sync.ts retention rule
if (!hasPhysicalComboProvider) { const exact = exactComboSlugs; catalog.models = entries.filter(entry => ( typeof entry.slug !== "string" || !exact.has(entry.slug) + || (preservingExistingRouted && isNativeAliasCatalogEntry(entry)) || (Array.isArray(entry.input_modalities) && entry.input_modalities.length > 0) )); } else {As per path instructions: "A behavior change in src/ should come with a focused regression test near the existing tests for that subsystem."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/codex/convergence.ts` around lines 241 - 252, Update prepareCatalog’s preserved-entry filtering to retain native alias entries in exactComboSlugs when they lack input_modalities, matching the retainedNativeAlias exemption used by mergeCatalogEntriesForSync; preserve existing filtering for other entries. Add a focused regression case to tests/codex-convergence-contract.test.ts covering a disk/active native alias without input_modalities and asserting it remains in the converged catalog.Source: Path instructions
src/codex/catalog/sync.ts (1)
663-681: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winAdd a test for generated exact combo aliases without
inputModalities.
deriveEntrysetspreserveExactInputModalitiesfor exact combo rows, andensureStrictCatalogFieldsonly normalizes to["text"]wheninput_modalitiesis absent if that option is false. Since the later filter keeps exact combo rows only wheninput_modalitiesis a non-empty array, a combo catalog model with noinputModalitiesdrops the generated alias after the existing bare native alias has been suppressed. Add a regression test that exercises a bare exact combo native alias withmodel.inputModalitiesmissing/empty.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/codex/catalog/sync.ts` around lines 663 - 681, Add a regression test covering a bare exact combo native alias where model.inputModalities is missing or empty. Exercise the sync flow through deriveEntry and ensureStrictCatalogFields, verify the existing bare native alias is suppressed, and assert the generated exact combo alias is retained despite lacking input_modalities.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs-site/src/content/docs/guides/combos.md`:
- Around line 101-104: Update the routing explanation around the exact bare slug
to state that only gpt-5.6-sol resolves to combo/nova-sol. Explicitly document
that openai-apikey/gpt-5.6-sol remains a provider-qualified API-key route and
never falls through to the native alias, while preserving the existing
account-qualified route behavior.
In `@docs-site/src/content/docs/reference/configuration/routing.md`:
- Around line 67-68: Update the routing configuration documentation around
nativeAlias to explicitly state that bare gpt-5.6-* aliases use Codex
Pool/Direct credentials, while openai-apikey/gpt-5.6-* uses API keys and never
falls through to the native alias; retain the existing distinction for
account-qualified and provider-qualified routes.
In `@docs-site/src/content/docs/ru/reference/configuration/routing.md`:
- Around line 67-68: Update the routing documentation at
docs-site/src/content/docs/ru/reference/configuration/routing.md lines 67-68 to
state that bare gpt-5.6-* IDs use Codex Pool/Direct credentials, while
openai-apikey/gpt-5.6-* uses API keys and never falls through. Apply the same
authentication and routing distinction at
docs-site/src/content/docs/ru/guides/combos.md lines 266-267, while clarifying
that account-qualified and provider-qualified identities remain unchanged.
In `@gui/src/i18n/de.ts`:
- Around line 1742-1743: Update the German translation value for
cws.field.nativeAliasHint to use the feminine article “diese Combo” instead of
“diesen Combo”, leaving the rest of the hint unchanged.
In `@gui/tests/combo-native-alias-editor.test.tsx`:
- Around line 9-29: Update the global-state setup in beforeEach and cleanup in
afterEach to save each key’s complete property descriptor via
Object.getOwnPropertyDescriptor(globalThis, key). Restore existing descriptors
with Object.defineProperty, and delete keys that had no original descriptor
instead of recreating them with undefined values. Keep the current testWindow
lifecycle and IS_REACT_ACT_ENVIRONMENT handling intact.
---
Outside diff comments:
In `@src/codex/catalog/sync.ts`:
- Around line 663-681: Add a regression test covering a bare exact combo native
alias where model.inputModalities is missing or empty. Exercise the sync flow
through deriveEntry and ensureStrictCatalogFields, verify the existing bare
native alias is suppressed, and assert the generated exact combo alias is
retained despite lacking input_modalities.
In `@src/codex/convergence.ts`:
- Around line 241-252: Update prepareCatalog’s preserved-entry filtering to
retain native alias entries in exactComboSlugs when they lack input_modalities,
matching the retainedNativeAlias exemption used by mergeCatalogEntriesForSync;
preserve existing filtering for other entries. Add a focused regression case to
tests/codex-convergence-contract.test.ts covering a disk/active native alias
without input_modalities and asserting it remains in the converged catalog.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ca79901d-7cd1-4806-b683-f19110beebda
📒 Files selected for processing (57)
docs-site/src/content/docs/guides/codex-app-models.mddocs-site/src/content/docs/guides/combos.mddocs-site/src/content/docs/ja/guides/codex-app-models.mddocs-site/src/content/docs/ja/guides/combos.mddocs-site/src/content/docs/ja/reference/configuration/routing.mddocs-site/src/content/docs/ko/guides/codex-app-models.mddocs-site/src/content/docs/ko/guides/combos.mddocs-site/src/content/docs/ko/reference/configuration/routing.mddocs-site/src/content/docs/reference/cli/agents.mddocs-site/src/content/docs/reference/configuration/routing.mddocs-site/src/content/docs/ru/guides/codex-app-models.mddocs-site/src/content/docs/ru/guides/combos.mddocs-site/src/content/docs/ru/reference/configuration/routing.mddocs-site/src/content/docs/zh-cn/guides/codex-app-models.mddocs-site/src/content/docs/zh-cn/guides/combos.mddocs-site/src/content/docs/zh-cn/reference/cli/agents.mddocs-site/src/content/docs/zh-cn/reference/configuration/routing.mdgui/src/combo-workspace-data.tsgui/src/components/combo-workspace-detail-panel.tsxgui/src/i18n/de.tsgui/src/i18n/en.tsgui/src/i18n/ja.tsgui/src/i18n/ko.tsgui/src/i18n/ru.tsgui/src/i18n/zh.tsgui/tests/combo-native-alias-editor.test.tsxgui/tests/combo-workspace-dirty.test.tsxsrc/cli/combo.tssrc/codex/catalog.tssrc/codex/catalog/aggregation.tssrc/codex/catalog/effort.tssrc/codex/catalog/kinds.tssrc/codex/catalog/metadata.tssrc/codex/catalog/native-models.tssrc/codex/catalog/parsing.tssrc/codex/catalog/provider-fetch.tssrc/codex/catalog/sync.tssrc/codex/convergence.tssrc/combos/index.tssrc/combos/types.tssrc/server/index.tssrc/server/management/combo-routes.tssrc/server/management/model-routes.tssrc/server/management/model-rows.tssrc/types.tsstructure/03_catalog-and-subagents.mdtests/cli-headless-parity.test.tstests/codex-catalog-sync-hardening.test.tstests/codex-catalog.test.tstests/codex-convergence-contract.test.tstests/combo-management-api.test.tstests/combo-workspace-data.test.tstests/combos.test.tstests/model-visibility-management-api.test.tstests/native-alias-maintainer-regressions.test.tstests/native-model-toggle.test.tstests/native-profile-drain-server.test.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs-site/src/content/docs/ru/guides/combos.md (1)
224-226: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDocument the existing clear-value syntax.
src/cli/combo.tssupports-to clear--effort,--alias, and--display-name(src/cli/combo.ts, Lines 68-99). The English page documents this behavior, but the Russian CLI section omits it.Add the equivalent Russian sentence:
Proposed documentation change
`set` также принимает `--strategy`, `--sticky`, `--effort`, `--alias`, `--native-alias`, `--display-name` и `--rename-from`. Для `--native-alias` нужны поддерживаемый сейчас bare native alias и непустой display name. +Значение `-` у `--effort`, `--alias` и `--display-name` очищает соответствующее поле.As per path instructions, localized documentation must stay synchronized with actual CLI/API behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs-site/src/content/docs/ru/guides/combos.md` around lines 224 - 226, Update the Russian CLI documentation near the `set` option description to state that `-` clears the `--effort`, `--alias`, and `--display-name` values, matching the behavior implemented in `src/cli/combo.ts` and documented on the English page.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@docs-site/src/content/docs/ru/guides/combos.md`:
- Around line 224-226: Update the Russian CLI documentation near the `set`
option description to state that `-` clears the `--effort`, `--alias`, and
`--display-name` values, matching the behavior implemented in `src/cli/combo.ts`
and documented on the English page.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: acb35fdb-e3f1-4dda-8f23-edb66940c93e
📒 Files selected for processing (6)
docs-site/src/content/docs/guides/combos.mddocs-site/src/content/docs/reference/configuration/routing.mddocs-site/src/content/docs/ru/guides/combos.mddocs-site/src/content/docs/ru/reference/configuration/routing.mdgui/src/i18n/de.tsgui/tests/combo-native-alias-editor.test.tsx
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/codex/convergence.ts (1)
253-258: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRetain exact native aliases during fallback filtering.
When routed discovery is empty, this function preserves an active native-alias row. The following filter removes that same row if its on-disk
input_modalitiesarray is absent or empty.Apply the same retained-native-alias exception used by
mergeCatalogEntriesForSync. Add a regression case with an exact native alias and noinput_modalities.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/codex/convergence.ts` around lines 253 - 258, The fallback filter in the convergence logic should retain exact native-alias entries even when input_modalities is absent or empty. Update the filtering condition under !hasPhysicalComboProvider to apply the same native-alias exception as mergeCatalogEntriesForSync, and add a regression case covering an exact native alias without input_modalities.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs-site/src/content/docs/reference/cli/agents.md`:
- Around line 60-63: The native-alias documentation must distinguish credential
behavior: in docs-site/src/content/docs/reference/cli/agents.md:60-63, state
that bare gpt-5.6-* native aliases use Codex Pool/Direct credentials, while
openai-apikey/gpt-5.6-* uses its configured API key and never falls through to
the native alias; add the same distinction in Chinese at
docs-site/src/content/docs/zh-cn/reference/cli/agents.md:53-56.
In `@gui/src/components/combo-workspace-detail-panel.tsx`:
- Around line 285-298: Update the display-name onChange handler in the combo
workspace detail panel to trim the entered value before storing it, converting
an empty trimmed result to null. Preserve the existing draft update behavior
while ensuring whitespace-only display names are treated as unset for
native-alias validation and saving.
In `@gui/tests/combo-native-alias-editor.test.tsx`:
- Around line 100-112: Add a third test case alongside the existing combo alias
editor tests that starts from the edit baseline, changes the alias through the
alias input to a non-native value such as vendor/custom, and creates the item.
Assert the saved result has nativeAlias false and displayName null, covering the
family-exit reset performed by updateComboAliasDraft.
In `@src/codex/catalog/provider-fetch.ts`:
- Around line 1229-1231: The contextCapped flag in the member context mapping
must reflect whether applying contextCap actually changes the pre-cap context
window, not whether contextWindow is below nativeContextWindow. Update the
relevant provider-fetch logic to preserve the pre-cap window, cap contextWindow
only when the cap is lower, and set contextCapped by comparing the capped value
with that original window, matching the explicit provider-hint behavior.
In `@src/codex/catalog/sync.ts`:
- Around line 553-558: Preserve trusted native source entries in a separate map
before the suppression applied in the native catalog construction, while keeping
the bare native row suppressed by configured aliases. Update alignment for
<selector>/<native-slug> rows to resolve metadata from this pre-suppression map
instead of the post-suppression nativeBySlug data, and add a regression case
covering an account selector with a native alias and model-specific metadata.
---
Outside diff comments:
In `@src/codex/convergence.ts`:
- Around line 253-258: The fallback filter in the convergence logic should
retain exact native-alias entries even when input_modalities is absent or empty.
Update the filtering condition under !hasPhysicalComboProvider to apply the same
native-alias exception as mergeCatalogEntriesForSync, and add a regression case
covering an exact native alias without input_modalities.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 92361ba9-eebc-4cdb-b045-1c6a1dd6f3a9
📒 Files selected for processing (57)
docs-site/src/content/docs/guides/codex-app-models.mddocs-site/src/content/docs/guides/combos.mddocs-site/src/content/docs/ja/guides/codex-app-models.mddocs-site/src/content/docs/ja/guides/combos.mddocs-site/src/content/docs/ja/reference/configuration/routing.mddocs-site/src/content/docs/ko/guides/codex-app-models.mddocs-site/src/content/docs/ko/guides/combos.mddocs-site/src/content/docs/ko/reference/configuration/routing.mddocs-site/src/content/docs/reference/cli/agents.mddocs-site/src/content/docs/reference/configuration/routing.mddocs-site/src/content/docs/ru/guides/codex-app-models.mddocs-site/src/content/docs/ru/guides/combos.mddocs-site/src/content/docs/ru/reference/configuration/routing.mddocs-site/src/content/docs/zh-cn/guides/codex-app-models.mddocs-site/src/content/docs/zh-cn/guides/combos.mddocs-site/src/content/docs/zh-cn/reference/cli/agents.mddocs-site/src/content/docs/zh-cn/reference/configuration/routing.mdgui/src/combo-workspace-data.tsgui/src/components/combo-workspace-detail-panel.tsxgui/src/i18n/de.tsgui/src/i18n/en.tsgui/src/i18n/ja.tsgui/src/i18n/ko.tsgui/src/i18n/ru.tsgui/src/i18n/zh.tsgui/tests/combo-native-alias-editor.test.tsxgui/tests/combo-workspace-dirty.test.tsxsrc/cli/combo.tssrc/codex/catalog.tssrc/codex/catalog/aggregation.tssrc/codex/catalog/effort.tssrc/codex/catalog/kinds.tssrc/codex/catalog/metadata.tssrc/codex/catalog/native-models.tssrc/codex/catalog/parsing.tssrc/codex/catalog/provider-fetch.tssrc/codex/catalog/sync.tssrc/codex/convergence.tssrc/combos/index.tssrc/combos/types.tssrc/server/index.tssrc/server/management/combo-routes.tssrc/server/management/model-routes.tssrc/server/management/model-rows.tssrc/types.tsstructure/03_catalog-and-subagents.mdtests/cli-headless-parity.test.tstests/codex-catalog-sync-hardening.test.tstests/codex-catalog.test.tstests/codex-convergence-contract.test.tstests/combo-management-api.test.tstests/combo-workspace-data.test.tstests/combos.test.tstests/model-visibility-management-api.test.tstests/native-alias-maintainer-regressions.test.tstests/native-model-toggle.test.tstests/native-profile-drain-server.test.ts
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/scripts/pr1244_followup_patch.py:
- Around line 62-69: Normalize the GUI test file’s final newline in the read
path and preserve it when writing. In .github/scripts/pr1244_followup_patch.py
lines 62-69, strip trailing newline characters before the endswith guard; in
lines 70-102, terminate the appended template with a newline after the final
`});`.
- Around line 164-166: Update the nativeSourceBySlug Map construction to spread
only nativeSourceEntries, removing the redundant native collection while
preserving string-slug filtering and metadata lookup behavior.
- Around line 4-10: Refactor replace_exact so it validates and stages each
replacement without writing files immediately, then add a single final flush()
call after all planned edits have been validated. Preserve the
exactly-one-target validation and ensure no files are modified if any target is
missing or duplicated, allowing safe re-runs.
- Around line 70-102: Preserve the end-of-file newline in the appended test by
adding a newline after the final `});` before the closing triple-quoted string
in the `gui_test.write_text` block. Keep the existing `rstrip("\n")` behavior
unchanged.
In @.github/workflows/one-shot-run-pr1244-followup-patch.yml:
- Around line 3-6: Add a workflow-level concurrency group to serialize runs of
this patch workflow, using a stable key tied to the workflow or target branch
and cancelling or queuing overlapping runs consistently. Keep the existing
replace_exact and push behavior unchanged.
- Around line 3-9: Scope the workflow’s default permissions to contents: read,
then grant contents: write only within the patch job’s permissions block and
document that elevated access with an explanatory comment. Add a descriptive
name to the patch job so runs are identifiable in the checks UI, while
preserving the existing pinned actions and job behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c4bcfc19-5dd5-4c79-8636-24a87dc0ed47
📒 Files selected for processing (2)
.github/scripts/pr1244_followup_patch.py.github/workflows/one-shot-run-pr1244-followup-patch.yml
| def replace_exact(path_str: str, old: str, new: str) -> None: | ||
| path = Path(path_str) | ||
| text = path.read_text() | ||
| count = text.count(old) | ||
| if count != 1: | ||
| raise SystemExit(f"{path}: expected exactly one replacement target, found {count}") | ||
| path.write_text(text.replace(old, new, 1)) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
replace_exact leaves the working tree half-patched when a later target is missing.
The script performs 12 independent mutations across docs, GUI, src, and tests. Each call writes to disk immediately. If call 5 raises SystemExit, calls 1 through 4 have already written their files. The caller workflow aborts before git commit, so the remote branch stays clean, but any local or re-run invocation starts from a partially patched tree. The second run then fails with found 0 on the already-applied targets, which makes the failure hard to diagnose.
Collect the planned edits first, validate every target, then write. This makes the script idempotent-safe on re-run.
♻️ Proposed two-phase apply
-from pathlib import Path
+from pathlib import Path
+
+_PENDING: dict[Path, str] = {}
def replace_exact(path_str: str, old: str, new: str) -> None:
path = Path(path_str)
- text = path.read_text()
+ text = _PENDING.get(path) or path.read_text()
count = text.count(old)
if count != 1:
raise SystemExit(f"{path}: expected exactly one replacement target, found {count}")
- path.write_text(text.replace(old, new, 1))
+ _PENDING[path] = text.replace(old, new, 1)
+
+
+def flush() -> None:
+ for path, text in _PENDING.items():
+ path.write_text(text)Then call flush() once at the end of the script.
🧰 Tools
🪛 Ruff (0.16.1)
[warning] 9-9: Avoid specifying long messages outside the exception class
(TRY003)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/scripts/pr1244_followup_patch.py around lines 4 - 10, Refactor
replace_exact so it validates and stages each replacement without writing files
immediately, then add a single final flush() call after all planned edits have
been validated. Preserve the exactly-one-target validation and ensure no files
are modified if any target is missing or duplicated, allowing safe re-runs.
| gui_test.write_text( | ||
| gui_text | ||
| + """ | ||
|
|
||
| test("edit clears native-alias metadata when alias leaves native family", async () => { | ||
| const baseline: ComboItem = { | ||
| id: "nova", | ||
| model: "gpt-5.6-sol", | ||
| alias: "gpt-5.6-sol", | ||
| nativeAlias: true, | ||
| displayName: "Nova Sol", | ||
| strategy: "failover", | ||
| stickyLimit: 1, | ||
| defaultEffort: null, | ||
| targets: target, | ||
| }; | ||
| let saved: ComboItem | undefined; | ||
| const { container, root } = await renderPanel(baseline, false, item => { saved = item; }); | ||
| const alias = container.querySelector<HTMLInputElement>("#cwi-edit-alias")!; | ||
| await act(async () => { setInputValue(alias, "vendor/custom"); }); | ||
| const save = container.querySelector<HTMLButtonElement>("#cwi-edit-save"); | ||
| expect(save).toBeTruthy(); | ||
| await act(async () => { save!.click(); }); | ||
| expect(saved).toMatchObject({ | ||
| alias: "vendor/custom", | ||
| model: "vendor/custom", | ||
| nativeAlias: false, | ||
| displayName: null, | ||
| }); | ||
| await act(async () => root.unmount()); | ||
| container.remove(); | ||
| });""" | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the GUI test file exists and expose its helpers plus its exact tail.
set -euo pipefail
f=$(fd --iglob 'combo-native-alias-editor.test.tsx' . || true)
echo "resolved: ${f:-NOT FOUND}"
[ -n "$f" ] || exit 0
echo "--- imports and helper declarations ---"
rg -nP '^(import|const|let|function|async function)\b' "$f"
echo "--- declarations of target/targets/renderPanel/setInputValue ---"
rg -nP -C3 '\b(targets?|renderPanel|setInputValue)\b\s*[:=(]' "$f"
echo "--- last 6 lines, with byte-level view of EOF ---"
tail -n 6 "$f" | cat -ARepository: lidge-jun/opencodex
Length of output: 339
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "--- git status/stat ---"
git diff --stat || true
echo "--- locate script and test file ---"
fd -a 'pr1244_followup_patch.py|combo-native-alias-editor.test.tsx' . || true
echo "--- tracked files matching names ---"
git ls-files | awk '/(^|\/)pr1244_followup_patch.py$|(^|\/)combo-native-alias-editor.test.tsx$/ {print}'
echo "--- inspect candidate files with cat ---"
for f in $(git ls-files | awk '/(^|\/)pr1244_followup_patch.py$|(^|\/)combo-native-alias-editor.test.tsx$/ {print}'); do
echo "===== $f ====="
wc -l "$f"
sed -n '1,140p' "$f"
done
echo "--- references to target/targets/helper symbols in repository (focused) ---"
rg -n --glob '*.tsx' --glob '*.ts' --glob '*.py' 'ComboItem|renderPanel|setInputValue|act|targets?:\s*target|const[\s\S]{0,80}target[\s\S]{0,120}={|function[\s\S]{0,80}target|targets' .Repository: lidge-jun/opencodex
Length of output: 6448
🏁 Script executed:
#!/bin/bash
set -euo pipefail
f=$(git ls-files | awk '/(^|\/)gui\/tests\/combo-native-alias-editor.test.tsx$/ {print; found=1} END {if (!found) exit 1}')
echo "target_file=$f"
echo "--- last 8 lines with trailing newline markers ---"
tail -n 8 "$f" | cat -A
printf '--- byte-level tail ---\n'
tail -c 200 "$f" | od -An -tx1 -c | tr '\n' ' '; echo
echo "--- script file candidates ---"
git ls-files '.github/scripts/*' | sed -n '1,120p'
echo "--- focused references to target target renderPanel setInputValue and combo workspace data exports ---"
rg -n --glob '*.tsx' --glob '*.ts' --glob '*.py' 'ComboItem|renderPanel|setInputValue|const target|exports|type ComboItem|interface ComboItem|targets' gui .github/scripts 2>/dev/null || trueRepository: lidge-jun/opencodex
Length of output: 538
Preserve the end-of-file newline after the appended test.
The appended test ends with }); and leaves gui/tests/combo-native-alias-editor.test.tsx without an EOF newline. This will fail GUI linting. Add the newline break just before the closing """ for the block, and keep the existing rstrip("\n") change if it removes the preceding test’s newline.
The referenced helper identifiers (ComboItem, renderPanel, setInputValue, act, and target) are already available through existing imports and declarations; targets: target matches the existing usage in this file.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/scripts/pr1244_followup_patch.py around lines 70 - 102, Preserve the
end-of-file newline in the appended test by adding a newline after the final
`});` before the closing triple-quoted string in the `gui_test.write_text`
block. Keep the existing `rstrip("\n")` behavior unchanged.
| """ const nativeSourceBySlug = new Map([...nativeSourceEntries, ...native].flatMap(entry => | ||
| typeof entry.slug === "string" ? [[entry.slug, entry] as const] : [] | ||
| ));""", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
The nativeSourceBySlug map spreads a subset of its own source.
Line 153 defines native as a filtered subset of nativeSourceEntries. Line 164 then spreads both lists into one Map. Every entry from native is already present in nativeSourceEntries and is the same object reference, so the second spread only rewrites identical keys with identical values.
The first spread alone preserves the intent, which is to keep suppressed bare native entries reachable as a metadata source for Line 171.
♻️ Proposed simplification
- const nativeSourceBySlug = new Map([...nativeSourceEntries, ...native].flatMap(entry =>
+ const nativeSourceBySlug = new Map(nativeSourceEntries.flatMap(entry =>
typeof entry.slug === "string" ? [[entry.slug, entry] as const] : []
));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| """ const nativeSourceBySlug = new Map([...nativeSourceEntries, ...native].flatMap(entry => | |
| typeof entry.slug === "string" ? [[entry.slug, entry] as const] : [] | |
| ));""", | |
| """ const nativeSourceBySlug = new Map(nativeSourceEntries.flatMap(entry => | |
| typeof entry.slug === "string" ? [[entry.slug, entry] as const] : [] | |
| ));""", |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/scripts/pr1244_followup_patch.py around lines 164 - 166, Update the
nativeSourceBySlug Map construction to spread only nativeSourceEntries, removing
the redundant native collection while preserving string-slug filtering and
metadata lookup behavior.
| on: | ||
| push: | ||
| branches: | ||
| - maintainer/supersede-1056-native-alias |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
Add a concurrency group. Overlapping runs corrupt the patch.
The workflow has no concurrency key. Two pushes in quick succession start two runs. Both check out the branch, and both execute replace_exact against the same targets.
The failure mode is specific. The first run pushes the patched commit. The second run then finds found 0 for every already-applied target and exits non-zero, or its non-force git push on Line 31 is rejected because the branch has moved. Either way the run fails for a reason unrelated to the patch content.
♻️ Proposed concurrency group
on:
push:
branches:
- maintainer/supersede-1056-native-alias
+
+concurrency:
+ group: one-shot-pr1244-followup-patch
+ cancel-in-progress: false📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| on: | |
| push: | |
| branches: | |
| - maintainer/supersede-1056-native-alias | |
| on: | |
| push: | |
| branches: | |
| - maintainer/supersede-1056-native-alias | |
| concurrency: | |
| group: one-shot-pr1244-followup-patch | |
| cancel-in-progress: false |
🧰 Tools
🪛 zizmor (1.29.0)
[warning] 3-6: insufficient job-level concurrency limits (concurrency-limits): workflow is missing concurrency setting
(concurrency-limits)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/one-shot-run-pr1244-followup-patch.yml around lines 3 - 6,
Add a workflow-level concurrency group to serialize runs of this patch workflow,
using a stable key tied to the workflow or target branch and cancelling or
queuing overlapping runs consistently. Keep the existing replace_exact and push
behavior unchanged.
Source: Linters/SAST tools
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/codex/catalog/provider-fetch.ts (1)
1211-1231: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPreserve provider-cap state for native-alias members.
The provider-cap regression at
tests/codex-catalog.test.tsLine 914 fails because the returned combo hascontextWindow: 350_000but nocontextCappedvalue. This branch only emits the flag whenmember.contextCapexists. The provider-level cap has already reduced the explicit 400,000-token target window before this function runs, but its cap provenance is absent here.Set
contextCappedwhen the provider cap is first applied, or propagate the applied cap into the member before this completion step. Preserve that state in the returned combo metadata.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/codex/catalog/provider-fetch.ts` around lines 1211 - 1231, Update the member completion logic around applyProviderContextCap so provider-level caps preserve their provenance for native-alias members. Set or propagate the applied cap state before constructing the returned member, and ensure contextCapped is emitted when the provider cap reduced the requested context window even when member.contextCap is undefined.Source: Pipeline failures
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/codex/catalog/provider-fetch.ts`:
- Around line 1211-1231: Update the member completion logic around
applyProviderContextCap so provider-level caps preserve their provenance for
native-alias members. Set or propagate the applied cap state before constructing
the returned member, and ensure contextCapped is emitted when the provider cap
reduced the requested context window even when member.contextCap is undefined.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 55767c48-5e42-44d3-8833-319d3695c8c9
📒 Files selected for processing (8)
docs-site/src/content/docs/reference/cli/agents.mddocs-site/src/content/docs/zh-cn/reference/cli/agents.mdgui/src/components/combo-workspace-detail-panel.tsxgui/tests/combo-native-alias-editor.test.tsxsrc/codex/catalog/provider-fetch.tssrc/codex/catalog/sync.tstests/codex-catalog-sync-hardening.test.tstests/codex-catalog.test.ts
3924bea to
67842aa
Compare
Squash-rebased PR #1244 onto current dev after conflict reconciliation and verification.
b79a526 to
15545b3
Compare
|
@Wibias Thanks for rebasing this onto CI is red at
Two shapes of fix are available and the choice is yours, since these are your call sites: default the destructured input, or update every caller to pass it. Defaulting is smaller but hides a future missing caller; updating callers is louder. Whichever you pick, the same class of gap may exist for the other inputs added in the same commit, so it's worth grepping rather than fixing just this one. Holding the merge until Cross-platform CI is green at the exact head. Ping me when it is and I'll take another look. |
|
@Wibias CI is green at And your fix was better than what I suggested. I proposed either defaulting the destructured input at Reviewed the whole diff on your head. The native-alias design is well covered — actual Three things before I merge, none of them large: 1. The branch is 20 commits behind 2. One locale is out of sync. English documents that 3. CI evidence standard, and this one is about our infrastructure rather than your patch. I filed #1302 today: Linux test shards intermittently hang for ~15 minutes and get killed, reporting One request rather than a blocker: the screenshot in the description is carried forward from #1056, and this branch is a reconstruction rather than that code. A fresh Desktop picker capture at the current head would be worth more — bare native alias routing to the combo, account-qualified natives staying separate, and provider-qualified OpenAI not falling through. |
2 participants
Summary
Supersedes #1056, which had diverged 362 commits behind
dev.This maintainer rebuild transplants the original native-alias feature onto current
devwith a true three-way merge, then hardens the resulting catalog/routing behavior against the still-valid bot/reviewer findings and current-dev integration changes.available_modelsfiltering. Bare supported native IDs route through the configured combo; account-qualified/provider-qualified OpenAI routes remain separate.nativeAliasanddisplayNamein the dashboard editor with localized labels and create/edit/save coverage.Maintainer verification so far
dev(9326b851) and verified the branch is 0 commits behind.UI verification
The original implementation was verified in an isolated Codex Desktop instance; the routed native aliases remained in the picker while native OpenAI rows were suppressed as intended. This screenshot is carried forward from #1056 because #1244 preserves that user-facing behavior while rebuilding the implementation on current
dev.Security / integrity review
Manual diff review covers alias validation and namespace boundaries, bare-vs-qualified routing separation, catalog backup/recovery trust, disabled-model migration, live/provider discovery isolation, duplicate/suppressed native rows, metadata mutation, and model-ranking behavior. No new secret-bearing or authentication material is introduced by this feature.
Review readiness
This replacement remains draft until the full repository gate, fresh CI, and all current reviewer/bot findings are clean.
Supersedes #1056.
Summary by CodeRabbit