Skip to content

fix(catalog): hide custom models when provider is disabled - #4199

Draft
leonclab wants to merge 1 commit into
lidge-jun:devfrom
leonclab:fix/catalog-custom-models-disabled-provider
Draft

fix(catalog): hide custom models when provider is disabled#4199
leonclab wants to merge 1 commit into
lidge-jun:devfrom
leonclab:fix/catalog-custom-models-disabled-provider

Conversation

@leonclab

@leonclab leonclab commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Summary

  • When a provider is disabled (config.providers[name].disabled === true), any manually configured custom models (config.customModels) under that provider are now hidden from catalog visibility and client export instead of remaining visible in the Codex model picker.
  • filterCatalogVisibleModels now checks config.providers[m.provider]?.disabled === true alongside existing blocklists and allowlists.
  • gatherRoutedModelsUncached guards the raw provider resolution so custom models under disabled providers do not inherit live/effective provider metadata.

Verification

  • Added unit tests in tests/codex-integration/selected-models.test.ts verifying that models (including custom models) belonging to disabled providers are omitted by filterCatalogVisibleModels.
  • Ran bun test tests/codex-integration/selected-models.test.ts (15/15 passed).
  • Ran bun test tests/codex-integration/codex-catalog.test.ts (311/311 passed).

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Agent Provenance

Agent Role Contribution Profile Model Evidence
codex main, implementer Fix custom model visibility on disabled providers and add regression tests unverified google-antigravity/gemini-3.8-flash git commit ca561b2

Review readiness checklist

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

  • All CI tests are green on my local testing.

  • I pushed my PR to the latest dev commit.

  • I resolved all correct Codex and CodeRabbit findings.

  • My PR is ready for review.

Summary by CodeRabbit

  • Bug Fixes
    • Models from disabled providers are no longer shown in the model catalog.
    • Custom models associated with disabled providers are also excluded.
    • Disabled providers no longer contribute configuration-based limits or capabilities to custom model entries.

@github-actions

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

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

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

⏳ DRAFT

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

What to do

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

Review readiness checklist

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

0/4 boxes ticked.

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

@github-actions
github-actions Bot marked this pull request as draft September 10, 2026 12:34
@coderabbitai

coderabbitai Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 68199790-fc80-46cd-ad6e-1096b4473d96

📥 Commits

Reviewing files that changed from the base of the PR and between 6d3ad12 and ca561b2.

📒 Files selected for processing (2)
  • src/codex/catalog/provider-fetch.ts
  • tests/codex-integration/selected-models.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

Changes

Disabled Provider Catalog Behavior

Layer / File(s) Summary
Catalog filtering and custom-model synthesis
src/codex/catalog/provider-fetch.ts, tests/codex-integration/selected-models.test.ts
filterCatalogVisibleModels excludes models from providers with disabled: true. Custom-model synthesis ignores configuration from disabled providers. Tests cover standard and custom models.
Estimated code review effort: 1 (Trivial) ~5 minutes

Merge Risk: ⚪ Minimal · up to ca561

Disabled-provider models will no longer appear in catalog results or client exports, and their settings will not shape custom catalog entries. Focused tests pass, so the change is mergeable.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: hiding custom catalog models when their provider is disabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 58 / 80

이 PR은 Codex 모델 목록에서 프로바이더를 끈 뒤에도 그 프로바이더 아래 수동 커스텀 모델이 피커에 남는 구멍을 막습니다. 지금 dev(HEAD 6d3ad12e3, 2.51.0)의 src/codex/catalog/provider-fetch.ts를 보면, 라이브 디스커버리 쪽은 이미 captureGatherFlight에서 provider.disabled !== true인 프로바이더만 모읍니다. 그런데 gatherRoutedModelsUncached 안의 config.customModels 매핑은 그 필터를 안 타고, filterCatalogVisibleModels도 프로바이더 disabled를 보지 않습니다. 그래서 디스커버리 행은 사라지는데 커스텀 행만 Codex/Grok/Claude Desktop 내보내기(filterCatalogVisibleModels를 거치는 src/grok/catalog.ts, src/cli/claude-desktop.ts, src/codex/convergence.ts 등)에 남는 불일치가 납니다. 이번 변경은 가시성 필터에 config.providers[m.provider]?.disabled === true 가드를 넣고, gather 쪽에서는 disabled 프로바이더의 rawProviderundefined로 두어 커스텀 행이 산 프로바이더 메타데이터를 물려받지 않게 합니다. 범위가 provider-fetch.ts + selected-models.test.ts 두 파일이고 +28/−1이라 2.50.0 릴리즈 트레인 문서 마감 이후의 dev 방향과도 잘 맞습니다.

라인 2148 (필터) - filterCatalogVisibleModels에 프로바이더 disabled 조기 탈락을 넣은 방향은 맞습니다. 다만 config.providers[m.provider]아예 없는 커스텀 행은 ?.disabled === true가 거짓이라 계속 보입니다. 의도(고아 커스텀은 유지)인지, 아니면 미등록 프로바이더도 숨길지 한 줄로 밝히면 좋습니다.

경로 gatherRoutedModelsUncached / customModels map - disabled일 때 rawProvider만 빼고 행 자체는 여전히 customModels 배열에 남습니다. 공개 피커는 대체로 필터를 거치지만, gather 결과를 필터 없이 쓰는 경로가 있으면 커스텀 행이 다시 노출될 수 있습니다. 디스커버리와 맞추려면 disabled 프로바이더 커스텀은 map에서 아예 생략(또는 map 직후 제거)하는 편이 더 안전합니다.

테스트 selected-models.test.ts - filterCatalogVisibleModels 회귀 두 개는 좋고 로컬 15/15·catalog 311/311 보고도 신뢰할 만합니다. 그런데 PR이 손댄 gather 가드(rawProvider undefined)에 대한 직접 테스트는 없습니다. disabled 프로바이더 커스텀이 gather 결과에서 메타데이터를 안 물려받는지(또는 아예 빠지는지) 한 케이스만 더 있으면 리뷰 부담이 줄어듭니다.

경로/심볼 initialModelSelectionPending 근처 순서 - disabled 체크가 pending 체크 바로 다음에 온 것은 읽기 좋습니다. disabledModels 블록리스트·selectedModels 허용 목록과의 우선순위도 현재 코드 흐름상 자연스럽고, 기존 슬래시 id 허용 목록 계약과도 충돌하지 않습니다.

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

  • gather에서 disabled 커스텀을 메타데이터만 제거하고 행은 남길지, 디스커버리처럼 행을 아예 뺄지
  • 프로바이더 키가 config에 없는 커스텀 모델을 계속 노출할지 말지
  • 이 구멍을 가리키는 오픈 이슈가 없으면 이 PR만으로 닫을지, 짧게 이슈를 남기고 closes로 묶을지

너의 추천
gather에서 disabled 프로바이더 커스텀 행을 생략하도록 한 줄 더 다듬고(또는 그게 의도적으로 남기는 거면 PR 본문에 이유를 한 문장), gather 쪽 단위 테스트 하나를 추가한 뒤 merge하세요. 타입/설정 분할에 무효화될 규모도 아니고 중복 PR도 아닙니다. CI 그린 확인 후 랜딩하면 됩니다.

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

@Ingwannu Ingwannu left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

The disabled-provider filter closes the direct visibility gap. The second source change is separate: gatherRoutedModelsUncached stops inheriting provider metadata for custom rows while that provider is disabled. The two new tests only call filterCatalogVisibleModels, so they do not observe that gather/export behavior.

Please add a gather-to-visible-export regression with a real customModels entry and disabled/enabled controls, asserting the saved custom configuration is retained and becomes visible with the same intended metadata after re-enabling. Also document the disabled-provider/custom-model visibility contract; no documentation file changes despite the checked docs box. I have not changed provider settings or executed local product tests. This remains Draft for complete exact-head checks and final acceptance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants