Skip to content

fix(usage): restrict legacy -main provider collapse to Codex labels - #5283

Closed
luvs01 wants to merge 1 commit into
lidge-jun:devfrom
luvs01:fix/provider-main-label-scope
Closed

luvs01 wants to merge 1 commit into
lidge-jun:devfrom
luvs01:fix/provider-main-label-scope

Conversation

@luvs01

@luvs01 luvs01 commented Sep 20, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • baseProviderLabel stripped a -main suffix from ANY provider name, so a configured provider like openrouter-main or azure-main silently merged into the base provider's usage row and pricing fallback.
  • The -main compatibility mapping is now restricted to the known Codex provider labels (openai-main, chatgpt-main, openai-multi-main), matching the codexAccountProviders identity map. Configured providers whose names end in -main stay distinct.

Verification

  • bun test tests/usage/usage-provider-label.test.ts tests/usage/usage-summary.test.ts tests/usage/usage-cost.test.ts — 180 pass, 0 fail
  • bun x tsc --noEmit — clean

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.

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
    • Preserved distinct provider names ending in -main for unrecognized providers, such as OpenRouter and Azure.
    • Continued consolidating recognized legacy OpenAI-related labels while retaining their canonical provider identity.
    • Updated usage summaries so providers with -main suffixes remain separate from their base providers.
    • Restricted historical pricing fallback behavior to recognized legacy suffixes.

@coderabbitai

coderabbitai Bot commented Sep 20, 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: Repository: lidge-jun/opencodex/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: cdba8431-8635-4ce3-8bd7-9e924da1ef55

📥 Commits

Reviewing files that changed from the base of the PR and between 447ac22 and 49e82dc.

📒 Files selected for processing (4)
  • src/providers/label.ts
  • tests/usage/usage-cost.test.ts
  • tests/usage/usage-provider-label.test.ts
  • tests/usage/usage-summary.test.ts

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


📝 Walkthrough

Walkthrough

The change restricts legacy -main collapsing to recognized OpenAI and ChatGPT labels. Unknown providers such as openrouter-main and azure-main remain distinct. Usage tests cover label normalization, summary rows, and legacy pricing fallback behavior.

Changes

Provider label normalization

Layer / File(s) Summary
Restrict recognized legacy labels
src/providers/label.ts:8-9, src/providers/label.ts:34-41
Adds an explicit recognized-label set. baseProviderLabel collapses -main only for openai-main, chatgpt-main, and openai-multi-main.
Update usage coverage
tests/usage/usage-provider-label.test.ts:28-37, tests/usage/usage-summary.test.ts:1189-1198, tests/usage/usage-cost.test.ts:1513-1519
Tests preserve unknown -main provider names, keep separate usage rows, and exclude legacy-main from the historical pricing fallback.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

🚥 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 3 functions across 4 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: it restricts legacy -main provider collapsing to Codex-related labels. It is concise, specific, and consistent with the implementation and tests.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

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.

@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 20, 2026
@github-actions

github-actions Bot commented Sep 20, 2026 •

Copy link
Copy Markdown
Contributor

✅ READY

  • all PR quality gates passed; the review readiness checklist is complete.

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.

✅ 4/4 boxes ticked.

Automatic ready-for-review conversion failed; please mark the pull request ready manually if it is still a draft.
The review-ready label marks this PR as ready; review automation runs independently.
Maintainers notified: @lidge-jun @Ingwannu

@github-actions
github-actions Bot marked this pull request as draft September 20, 2026 07:29
@luvs01
luvs01 marked this pull request as ready for review September 20, 2026 07:35
@github-actions
github-actions Bot marked this pull request as draft September 20, 2026 07:36
@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 24 / 80

이 PR은 usage 쪽에서 쓰는 baseProviderLabel의 -main 처리를 좁히는 버그 수정입니다. 예전에는 이름 끝이 -main이면 무조건 앞부분만 남겨서, 설정에 openrouter-main이나 azure-main처럼 진짜로 -main으로 끝나는 프로바이더가 있으면 usage 요약·가격 fallback에서 기본 이름(openrouter, azure)과 한 줄로 합쳐졌습니다. 지금은 Codex 계열로 알려진 openai-main, chatgpt-main, openai-multi-main만 접미사를 떼고 openai로 모읍니다. 이 세 이름은 codexAccountProviders가 openai/chatgpt/openai-multi에 main identity를 붙이는 맵과 맞습니다. 테스트도 라벨 단위, usage summary 행 분리, 가격 fallback에서 legacy-main 제외까지 같이 손봤고, base는 dev입니다.

라인 - src/providers/label.ts / LEGACY_MAIN_ACCOUNT_PROVIDER_LABELS: 예전 테스트는 codex-main → codex로 접미사를 뗐는데, 이번 허용 목록에는 codex-main이 없습니다. codexAccountProviders에도 codex 계열은 없어서 방향은 맞지만, 예전 로그에 codex-main이 남아 있으면 이제 별도 provider 행으로 남을 수 있습니다. PR 본문에 이 부작용이 안 적혀 있습니다.
라인 - 리뷰 readiness: draft이고 체크리스트 0/4라 merge 게이트 기준으로는 아직 “리뷰 준비됨”이 아닙니다. 코드 방향과 별개로 머지 판단은 박스 채운 뒤가 맞습니다.
라인 - LEGACY_MAIN_ACCOUNT_PROVIDER_LABELS와 codexAccountProviders가 서로 다른 파일에 하드코딩되어 있어, 나중에 Codex base 이름이 늘면 한쪽만 고칠 위험이 있습니다. 지금 범위에서는 과한 리팩터는 아니지만, drift 가능성은 알아두면 좋습니다.

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

과거 usage 로그에 codex-main(또는 Codex 허용 목록 밖 -main) 행이 실제로 남아 있는지, 그걸 계속 base로 합칠 필요가 있는지 한 번만 확인해 주세요. 필요 없다면 지금 allowlist가 맞고, 필요하면 목록에만 추가하면 됩니다.

너의 추천

방향은 맞고 범위도 작습니다. draft 체크리스트를 채운 뒤, codex-main 과거 로그 이슈만 확인되면 머지해도 됩니다. CI hygiene/label/enforce-target은 통과한 상태입니다.

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

A -main suffix was stripped from ANY provider name, so a configured provider like openrouter-main silently merged into openrouter in usage summaries and pricing fallback. Restrict the compatibility mapping to the known Codex provider labels (openai/chatgpt/openai-multi-main), matching codexAccountProviders.
@luvs01

luvs01 commented Sep 22, 2026

Copy link
Copy Markdown
Collaborator Author

Consolidated into #5556 as a single related-function aggregate.

Source head: 89a927228c3e30f330f313ab8b2af37199892afd. Replacement head: d3589638a877530f89c111d3dba69d7a76908939.

The complete provider-label change matches carried 0f0ef96 by stable patch ID, with author/date/provenance preserved. Configured providers ending in -main remain distinct while the recognized legacy Codex aliases retain their existing behavior. Its regressions are preserved in the registered provider-label sibling rather than exceeding the existing summary-file cap. Final latest-dev integration preserves the tested contribution files; full final suite, exact-head hosted CI and required review remain pending on the draft replacement.

Closing this duplicate standalone review entry as part of the requested consolidation after verifying coverage. This is not a merge or release claim; remaining integration checks and reviews are tracked on the replacement. Original branches are retained.

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

Labels

bug Something isn't working review-ready

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants