Skip to content

fix(codex): attribute Spark quota headers - #4137

Closed
cb8010d6 wants to merge 1 commit into
lidge-jun:devfrom
cb8010d6:fix/spark-header-quota-attribution
Closed

fix(codex): attribute Spark quota headers#4137
cb8010d6 wants to merge 1 commit into
lidge-jun:devfrom
cb8010d6:fix/spark-header-quota-attribution

Conversation

@cb8010d6

@cb8010d6 cb8010d6 commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Summary

  • carry the resolved Codex quota scope through HTTP, WebSocket, compact-retry, and warmup header writers
  • attribute a Spark response's sub-day primary window to GPT-5.3-Codex-Spark 5h instead of the account-level short* fields
  • preserve the ordinary secondary weekly window, genuine shared 5-hour windows, other custom windows, and existing generation/main-writer fences
  • avoid plan checks or percent/reset equality heuristics; attribution comes from the model that produced the headers

Closes #4122.

Verification

  • bun test tests/codex-integration/codex-quota-parser-parity.test.ts (14 pass)
  • bun test tests/codex-integration/codex-quota-auto-refresh.test.ts --timeout 10000 (20 pass)
  • bun test tests/codex-integration/main-quota-provenance.test.ts tests/codex-integration/main-quota-window-observation.test.ts (all assertions passed)
  • bun test tests/responses/responses-account-label.test.ts --timeout 10000 (8 pass)
  • bun test tests/responses/responses-compaction-routing.test.ts --timeout 10000 (121 pass)
  • bun run typecheck
  • bun run lint:gui:if-changed (correctly skipped; no GUI changes)
  • bun run privacy:scan
  • git diff --check

Windows note: the existing successive idle windows auto-refresh test took about 5-6.5 seconds locally and crossed Bun's default 5-second per-test timeout; it passed with the explicit 10-second timeout above. No assertion failed.

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.

@coderabbitai

coderabbitai Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

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

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@cb8010d6

cb8010d6 commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

Closing as superseded by #4128, which was merged into dev while this PR was being prepared. The upstream implementation covers the same model-aware quota attribution across HTTP, WebSocket, and compact paths, preserves existing custom windows, and includes stronger Spark/non-Spark/legacy regression coverage. I also verified the focused quota parser suite on the merged dev head (15 passed). Keeping this open would only duplicate and conflict with the merged fix.

@cb8010d6 cb8010d6 closed this Sep 9, 2026
@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 24 / 80

이 PR은 Spark 모델 응답의 짧은(5시간) primary 헤더를 계정 공용 short* 슬롯이 아니라 모델 전용 창으로 적어 주려는 수정입니다. 바꾸는 곳은 src/codex/quota.tsapplyAccountQuotaFromUpstreamHeaders, 응답 경로(src/server/responses/core.ts, compact.ts)의 헤더 기록, 워밍업(src/codex/quota-auto-refresh.ts), 그리고 tests/codex-integration/codex-quota-parser-parity.test.ts입니다. 이슈 문구도 Closes #4122를 걸고 있습니다.

그런데 현재 dev HEAD(3b4d8c439)에는 이미 #4128(b2142586a)이 들어가 있습니다. 그 커밋이 같은 문제(#4122)를 닫았고, src/codex/quota.tsSPARK_SHORT_WINDOW_LABEL = "GPT-5.3-Codex-Spark 5h"customWindows 귀속이 이미 있습니다. #4132는 그 계획 단위를 _fin으로 옮긴 상태입니다. 이슈 #4122 자체도 이미 CLOSED입니다.

구현 모양도 겹치면서 다릅니다. 이 PR은 CodexQuotaScope("spark" / "shared")를 다섯 번째 positional 인자로 넘깁니다. 이미 머지된 #4128은 parseUpstreamQuotaHeaders / applyAccountQuotaFromUpstreamHeaders{ modelId } 옵션을 넣고, 파서 안에서 isCodexSparkModel(modelId)로 같은 라벨의 customWindows를 씁니다. HEAD API와 이 PR API는 그대로 합치면 충돌합니다. 리베이스해도 “같은 수정을 다른 모양으로 다시 넣는” 결과가 됩니다.

이 PR만의 작은 차이는 quota-auto-refresh.ts 워밍업에서 "shared"를 명시하는 부분입니다. 그런데 #4128 커밋 메시지와 HEAD 코드가 이미 “워밍업은 기본(비-Spark) 모델이라 modelId 없이 계정 short 경로를 유지”라고 정해 두었습니다. modelId가 없으면 Spark 분기로 가지 않으므로, 그 "shared" 명시는 동작상 거의 중복입니다. 굳이 가져올 조각도 없습니다.

작성자(cb8010d6)가 이미 “#4128에 의해 superseded”라고 닫아 둔 상태와도 맞습니다. 열린 PR 수를 부풀리지 말고, 닫힌 상태를 유지하면 됩니다.

src/codex/quota.ts - HEAD에 이미 SPARK_SHORT_WINDOW_LABEL과 Spark→customWindows 귀속이 있음 (#4128). 이 PR의 CODEX_SPARK_SHORT_LABEL + quotaScope === "spark" 분기는 같은 일을 다른 API로 다시 함
src/codex/quota.ts / applyAccountQuotaFromUpstreamHeaders - 이 PR은 5번째 positional quotaScope를 추가하지만, HEAD는 options?: { modelId?: string } 형태라 시그니처가 호환되지 않음
src/server/responses/core.ts / compact.ts - 모델 힌트를 넘기는 호출부도 #4128이 이미 route.modelId로 연결해 둠. 재배선 불필요
src/codex/quota-auto-refresh.ts - 워밍업에 "shared"를 넣는 것만 이 PR 고유 후보이나, HEAD는 modelId 생략 = 비-Spark 경로로 이미 동일 효과. cherry-pick 가치 낮음
tests/codex-integration/codex-quota-parser-parity.test.ts - Spark/공유 5h 회귀 테스트도 #4128 쪽에 더 강한 형태로 들어와 있음
Closes #4122 클레임 - #4122는 #4128로 이미 닫힘. 이 PR이 다시 닫을 이슈가 없음

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

  • (거의 없음) 워밍업에 명시적 "shared"/modelId를 추가로 박을지 — HEAD는 의도적으로 생략. 새 작업이면 별도 작은 PR로만
  • 작성자가 이미 close 했으므로 라벨/상태 추가 조작은 불필요

너의 추천
CLOSE 유지(이미 superseded). 리베이스·재오픈하지 말 것. #4122는 #4128이 처리했고, 이 diff는 HEAD와 API까지 겹치는 중복 구현이다. quota-auto-refresh"shared"도 cherry-pick 불필요.

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

@github-actions

github-actions Bot commented Sep 9, 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.

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.

2 participants