refactor(combos): isolate combo identifier helpers (split S11 L1/5)

[lidge-jun]

Summary

• Pure move: the combo identifier helpers of src/combos/types.ts (COMBO_NAMESPACE, id pattern/validation, targetKey, parseComboModelId/comboModelId/comboPublicModelId/comboDisabledModelId/comboDisabledModelSelectors, resolveComboId, native-alias predicate — lines 13–22, 54–124, 396–399) move verbatim to src/combos/identifiers.ts (89 lines). types.ts keeps validation/normalization and re-exports all 11 moved values, so all 22 previously exported names stay importable from combos/types and the combos/index.ts facade is untouched (19 direct importers unchanged).
• Why: 423-line file over the 400-line module limit; identifiers are a dependency-free leaf that validation consumes, so the split removes an implicit forward reference inside the file. Zero behavior change.
• Plan and evidence: devlog/_plan/260905_now_split_train/320_combos_types.md; rules 003_parent_decisions.md (PURE-MOVE-SIZE-01, TYPE-CYCLE-01).

Stack (S11 codex-misc — independent layers, each based on dev; no cascade between them):

#	PR	Branch	Base	Review focus
5	TBD	codex/split-oauth-github-copilot	dev	github-copilot
4	TBD	codex/split-routing-trace	dev	routing/trace
3	TBD	codex/split-codex-cli-install-provenance	dev	cli-install-provenance
2	TBD	codex/split-codex-subagent-defaults	dev	subagent-defaults
1	this PR	codex/split-combos-types ← you are here	dev	identifiers leaf

Base: dev. Review this PR's diff only (3 files, +118/−94; non-move diff: 2 leaf imports, 5 facade wiring lines, 25 test lines). Move-aware view: git diff --color-moved=dimmed-zebra dev...HEAD.

Verification

• bun run typecheck → exit 0
• Focused (combos, codex-catalog, combo-management-api, provider-id-rewrite) → 372 pass / 0 fail
• tests/lab/core-lab-boundary.test.ts → 17 pass / 0 fail
• Cycle gate including inline import("…") type edges: 18 files walked, no path returns to types.ts/index.ts/the leaf; the three pre-existing type cycles under src/types are unchanged.
• Red-drives, then restored: wrapping comboModelId at the facade fails the new identity assertion; breaking comboDisabledModelSelectors in the leaf fails combos.test.ts:193.
• bun run privacy:scan → passed; git diff --check dev...HEAD clean.
• New test: identity of all 11 moved values via facade vs leaf; leaf has no back-edge.
• Full suite on the remote CI host (lidge) at this exact SHA: recorded in the devlog doc.

Checklist

• Scope stays focused and avoids unrelated cleanup.
• Docs or release notes were updated when needed (devlog unit records the layer; no user-facing change).
• Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults (combo id validation moved byte-for-byte).

Summary by CodeRabbit

• New Features
  • Added consistent combo model identifiers and aliases for selecting configured combo models.
  • Added support for resolving combo models by canonical ID or exact configured alias.
  • Improved handling of disabled combo model selectors, including native OpenAI aliases.
  • Added validation and parsing for combo model IDs, with malformed identifiers handled safely.
• Bug Fixes
  • Preserved physical combo provider behavior when no configured combos are present.

[github-actions]

✅ Deterministic PR hygiene checks passed.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Team

Run ID: db823168-3710-4e9e-9064-ee14930928da

📥 Commits

Reviewing files that changed from the base of the PR and between 3c920af and 0c914bf.

📒 Files selected for processing (3)

• src/combos/identifiers.ts
• src/combos/types.ts
• tests/codex-integration/combos.test.ts

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

---

📝 Walkthrough

Walkthrough

The change extracts combo identifier helpers into src/combos/identifiers.ts, preserves their public exports through src/combos/types.ts, and adds integration coverage for export identity and leaf-module import boundaries.

Changes

Combo identifier handling

Layer / File(s)	Summary
Identifier primitives src/combos/identifiers.ts	Adds namespace handling, native alias detection, combo identifier formatting, parsing, and validation.
Selector and configuration resolution src/combos/identifiers.ts	Adds disabled-model selector handling and canonical-or-alias combo resolution.
Public export wiring and validation src/combos/types.ts, tests/codex-integration/combos.test.ts	Re-exports the helpers and verifies export identity plus leaf-module import boundaries.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 0c914

Combo identifier helpers are now housed in a dependency-free module while existing public exports remain available. The change includes compatibility and boundary coverage, with no remaining merge-readiness risk identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 41.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 3 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 accurately describes the main change: moving combo identifier helpers into an isolated module. It is concise and specific, although the split milestone suffix is supplementary.
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 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/split-combos-types

---

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

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-05T03:15:50.815545Z	0c914bf	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[lidge-jun]

리뷰 · 우선순위 61 / 80

이 PR은 combos 모듈 스플릿 트레인 S11의 1/5층입니다. 베이스는 dev, 헤드는 codex/split-combos-types, GitHub 상태는 MERGEABLE, 라벨은 chore입니다. 하는 일은 단순합니다. 지금 dev의 src/combos/types.ts는 423줄로 모듈 400줄 한도를 넘깁니다. 그중 콤보 식별자 헬퍼(COMBO_NAMESPACE, id 패턴/검증, targetKey, parseComboModelId/comboModelId/comboPublicModelId/comboDisabledModelId/comboDisabledModelSelectors, resolveComboId, native-alias 판별 등)를 새 파일 src/combos/identifiers.ts(약 89줄)로 그대로 옮기고, types.ts는 validation/normalization을 남긴 채 11개 심볼을 re-export합니다. combos/index.ts 파사드와 기존 import 경로는 안 바꿉니다. 동작 변경 없는 pure move입니다.

왜 지금이냐면, 메인테이너가 걸어 둔 types/config 대형 스플릿 캠페인과 같은 규칙입니다(PURE-MOVE-SIZE-01, TYPE-CYCLE-01). 식별자 리프는 의존성이 거의 없고 validation이 그걸 쓰므로, 파일 안 forward reference를 줄이면서 한도만 맞춥니다. 계획 문서는 devlog/_plan/260905_now_split_train/320_combos_types.md에 있습니다. 같은 S11의 다른 층(oauth-github-copilot, routing-trace, cli-install-provenance, subagent-defaults)은 각각 dev 기준 독립 층이라 이 PR과 캐스케이드하지 않습니다.

검증 메모도 스플릿 PR답게 짧고 확실합니다. typecheck 통과, combos/codex-catalog/combo-management-api/provider-id-rewrite 포커스 372 pass, cycle gate에서 identifiers → types.ts/index.ts 역엣지 없음, facade vs leaf export 동일성 테스트 추가. CI는 gates/hygiene/linux 쪽이 통과이고 macos/keyring 일부는 pending입니다. dev HEAD 3c920af5f 기준으로 충돌 없이 올릴 수 있는 형태입니다.

라인 / 경로로 보면 아래를 보면 됩니다.

경로 src/combos/identifiers.ts - ../types(전역 타입)와 ../codex/catalog/native-models만 import한다. ./types나 ./index로 돌아가지 않는다. cycle gate 주장과 맞다
경로 src/combos/types.ts - 이동 후 re-export 한 줄 + COMBO_NAMESPACE/isValidComboId/targetKey를 leaf에서 가져와 validation에 쓴다. SUPPORTED_NATIVE_OPENAI_SLUGS import는 types 쪽 validation에 남아 있어 죽은 import가 아니다
경로 tests/codex-integration/combos.test.ts - public facade와 leaf의 11개 export가 같은 참조인지, leaf 소스에 ./types|./index back-edge가 없는지 잠근다. 스플릿 회귀에 맞는 최소 테스트다
경로 S11 나머지 층 - 이 PR과 파일 겹침이 거의 없다. 닫고 리베이스할 대상이 아니라, 각각 dev에 독립 merge 가능한 chore다
경로 대형 types.ts/config.ts 스플릿 규칙 - 이 PR은 그 캠페인의 정상 층이다. “스플릿에 무효화되니 닫아라”가 아니라 “스플릿을 진행하라” 쪽이다

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

• S11 1/5를 지금 dev에 바로 올릴지, 다른 급한 bug/enhancement(feat(quota): support Google Antigravity weekly quota and Ollama Cloud quota (carry of #3447) #3587 quota 캐리 등) 뒤에 둘지
• identifiers leaf를 index에서 직접 re-export할지, 지금은 types facade만 유지할지(추천: 지금처럼 types re-export 유지)
• 나머지 S11 네 층을 이 PR 머지 직후 연속으로 올릴지

너의 추천
exact-head CI(특히 남은 macos)가 초록이면 merge 하세요. pure move라 사용자 동작 위험이 작고, 423줄 한도 위반을 바로 깎습니다. 머지 후 열린 중복·무효 PR은 없고, leftover close 대상도 아닙니다. 이어서 S11 다음 층을 같은 방식으로 올리면 됩니다. types/config 스플릿 캠페인에 맞는 정상 chore입니다.

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

[lidge-jun]

리뷰 · 우선순위 74 / 80

설명

이 PR은 S11(codex-misc) 스플릿 기차의 1번째 칸이다. 지금 dev HEAD는 3191fe1aa이고, 방금 #3600으로 oauth 쿼럼 테스트가 이름대로 account-removal 경로를 타게 고친 직후다. 그 위에는 이미 #3587 / #3592 / #3588 쿼터 스택이 올라가 있다. 이 PR은 그 쿼터 일과는 겹치지 않는다. 하는 일은 하나다. src/combos/types.ts 안에 붙어 있던 콤보 식별자 도우미들(COMBO_NAMESPACE, id 패턴/isValidComboId, targetKey, parseComboModelId / comboModelId / comboPublicModelId / comboDisabledModelId / comboDisabledModelSelectors, resolveComboId, isNativeAliasCombo, preservesPhysicalComboProvider)을 새 파일 src/combos/identifiers.ts로 그대로 옮긴다.

왜 옮기냐면 types.ts가 약 423줄로 400줄 모듈 한도를 넘었기 때문이다. 식별자 쪽은 다른 combos 검증 로직에 의존하지 않는 잎(leaf)이다. 검증/normalize가 이 잎을 소비하는 방향이 맞고, 파일 안에 앞으로 참조하던 느낌이 사라진다. types.ts는 검증·정규화만 남기고, 옮긴 11개 이름을 전부 다시 export 한다. 그래서 combos/types나 combos/index를 쓰던 기존 importer 22곳은 손대지 않아도 된다. 행동 변화는 없다. 순수 이동이다.

diff는 파일 3개(+117/−93)다. 새로 생긴 identifiers.ts 89줄, types.ts에서 해당 블록 삭제 후 re-export 한 줄과 잎 import 몇 줄, 그리고 tests/codex-integration/combos.test.ts에 facade vs leaf 동일성 + 잎이 ./types/./index를 다시 import하지 않는지 검사하는 테스트 한 덩어리다. 로컬에서 보면 이동 후 types.ts는 333줄로 한도 아래로 내려온다. SUPPORTED_NATIVE_OPENAI_SLUGS import는 types.ts 검증(nativeAlias 검사)과 identifiers.ts의 isNativeAliasCombo 양쪽에 여전히 쓰이므로 죽은 import가 아니다.

지금 dev가 최적화하는 방향(쿼터 스택 마무리 + 스플릿 기차 진행)과 맞물린다. S11은 레이어가 서로 cascade 없이 각자 dev 위에 올라가는 독립 칸이라고 PR 본문에 적혀 있다. 이 L1이 먼저 들어가면 뒤에 올 oauth-github-copilot / routing-trace / cli-install-provenance / subagent-defaults 칸이 같은 파일 충돌 없이 갈 수 있다. 베이스는 예전 3c920af5f(Windows shard bump 근처)라 현재 HEAD보다 뒤처져 있지만, 건드리는 파일이 쿼터/#3600과 겹치지 않아 리베이스 압박은 작다.

라인 85 - src/combos/identifiers.ts에서 resolveComboId 끝과 isValidComboId 사이에 빈 줄이 두 줄이다다. 세 번째 커밋이 trailing blank를 정리했다고 했는데 한 줄이 더 남았다. 동작엔 영향 없다. 다만 순수 이동 PR이라 이런 자잘한 공백까지 맞추는 편이 리뷰 신호가 깨끗하다.

경로/심볼 base vs HEAD - 브랜치가 현재 dev(3191fe1aa)의 조상이 아니다. 충돌 가능성은 낮지만 merge 직전에 dev로 ff/rebase 한 번만 확인하면 된다. CodeRabbit docstring coverage 경고(41% vs 80%)는 기존에 docstring이 없던 헬퍼를 그대로 옮긴 결과라서, 이 PR 범위에서 docstring을 새로 쓰기보다는 무시하거나 후속 문서 PR로 미루는 쪽이 스플릿 규칙(PURE-MOVE)과 맞다.

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

• S11 L2~L5를 이 PR merge 직후 연속으로 열지, 아니면 L1만 먼저 랜딩하고 CI 안정화를 볼지
• docstring coverage 경고를 스플릿 기차 전체에서 일괄 ignore할지, leaf마다 최소 JSDoc을 채울지
• 베이스를 merge 전에 반드시 3191fe1aa로 맞출지, 충돌 없으면 그대로 squash/merge할지

너의 추천
merge. 순수 이동 + facade 동일성 테스트 + cycle/leaf back-edge 가드가 있고, 현재 dev의 쿼터/#3600 일과 파일 겹침이 없다. merge 전 identifiers.ts 이중 빈 줄만 한 줄로 줄이거나, 무시하고 바로 랜딩해도 된다. S11 다음 칸은 이 PR이 dev에 들어간 뒤에 여는 게 안전하다.

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

[lidge-jun]

Superseded by aggregate PR #3668, admin-merged into dev as 6585e6a after exact-head CI and tested-tree verification. This original PR was not individually merged; its rebased content and historical records were consolidated in #3668. The original branch is preserved. Further unimplemented debt layers remain deferred.

[lidge-jun]

Landed via #3668 at 6585e6a