perf(core): cache static registry model lookups and eliminate snapshot traversal allocations

[chilung-cgu]

Summary

• Caches static PROVIDER_REGISTRY model definitions in src/router.ts into static lookup maps (REGISTRY_BY_ID, REGISTRY_STATIC_MODEL_IDS, REGISTRY_ALIAS_BY_ID), replacing O(N) array scans (.find()) and repeated Object.keys() array allocations with O(1) lookups in knownModelIdsForProvider, routedProviderConfig, and routeModelInternal.
• Optimizes writeBoundedSnapshot in src/responses/state.ts by replacing [...states].reverse() with reverse-indexed traversal over Array.from(states), and concatenating pre-serialized entries directly ({"version":2,"states":[" + serializedEntries.join(",") + "]}) instead of a second full-tree JSON.stringify, eliminating 24-48 MiB of transient heap allocations during snapshot schedules.
• Preserves native byte-exactness in request logging metering (Buffer.byteLength(JSON.stringify(entry), "utf8")) based on benchmark verification.
• Adds regression unit tests in tests/routing/router.test.ts and tests/responses/responses-state-write-amplification.test.ts ensuring static registry cache lookup integrity and snapshot serialization output formatting.

Verification

• bun run typecheck (passed with 0 errors)
• bun run privacy:scan (passed)
• bun test tests/codex-integration/app-owned-memory.test.ts tests/routing/router.test.ts tests/lab/core-lab-boundary.test.ts tests/responses/responses-state-write-amplification.test.ts tests/codex-integration/slug-codec.test.ts (76 pass, 0 fail, 206 expect calls)

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

• Performance

  • Improved snapshot writing efficiency while preserving existing size limits and JSON compatibility.
  • Optimized provider and model lookups to support faster routing.

• Reliability

  • Ensured generated snapshots remain valid JSON and retain all remembered entries.
  • Confirmed provider model listings include expected NVIDIA registry models.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[github-actions]

⚠️ Deterministic hygiene checks failed.

• missing_regression_test — Behavior changed under src/ or gui/src/ without a test change. Add focused coverage or obtain test-exception-approved.

[coderabbitai]

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

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: 5555cb42-a3c2-4ae6-894b-ec6607bd1b89

📥 Commits

Reviewing files that changed from the base of the PR and between 8f5a334 and e70c08c.

📒 Files selected for processing (2)

• src/responses/state.ts
• src/router.ts

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

---

📝 Walkthrough

Walkthrough

The change optimizes bounded snapshot serialization and provider registry access. Snapshot entries are serialized once during backward selection. Router paths use precomputed maps for provider, model-ID, and alias lookups. Tests validate snapshot parsing and static model-ID discovery.

Changes

Runtime efficiency improvements

Layer / File(s)	Summary
Bounded snapshot serialization src/responses/state.ts, tests/responses/responses-state-write-amplification.test.ts	Snapshot generation walks entries backward, serializes each selected entry once, measures UTF-8 size, applies the existing limits, and reverses the serialized strings before building the JSON payload. The test validates the version, JSON structure, and remembered entry IDs.
Provider registry lookup maps src/router.ts, tests/routing/router.test.ts	The router precomputes provider, static model-ID, and alias maps. Model-ID discovery, provider configuration, and alias fallback use these maps instead of repeated registry scans. The test verifies NVIDIA static model IDs include moonshotai/kimi-k2.6.

Priority: ⬇️ Low

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

Change: Refactor

🚥 Pre-merge checks | ✅ 4 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 4 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Linked Issues check	❓ Inconclusive	The context identifies PR #4732 but does not provide a linked issue or issue requirements. The linked-issue status cannot be verified from the supplied information.	Provide the linked issue reference and confirm that the PR addresses its acceptance criteria, or confirm that no linked issue is required.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Description check	✅ Passed	The PR summary clearly describes the registry lookup caches, snapshot serialization optimization, preserved behavior, and validation results. It matches the changes in src/router.ts, `src/responses/…
Out of Scope Changes check	✅ Passed	The changes remain within the stated performance objective. They optimize provider registry lookups and response-state snapshot serialization, with focused tests and no unrelated public API changes.
Title check	✅ Passed	The title accurately and concisely identifies both primary changes: caching static registry lookups and removing snapshot traversal allocations.

✨ 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[github-actions]

⏳ DRAFT

• author re-attestation is required for the current head.

What to do

• Change the first managed item to Required local validation passed; commands, results, and any full-suite exception are documented., clear all four boxes and save. Wait for the bot to acknowledge the cleared checklist before validating and ticking the boxes again.
• Only a new body edit by the PR author after this notice can advance the checkpoint. If edits share a checkpoint timestamp, make another body edit and save later.

Review readiness checklist

• ⬜ Required local validation passed; commands, results, and any full-suite exception are documented.
• ⬜ I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).
• ⬜ I resolved all correct Codex and CodeRabbit findings.
• ⬜ My PR is ready for review.

0/4 boxes ticked.

Current head: 1f32c93b9d373dc7e40ce9938eca119827ce02ad. Existing PR text and checkbox marks were preserved.

[lidge-jun]

리뷰 · 우선순위 70 / 80

이 PR은 chilung-cgu가 연 draft이고, 베이스는 지금 dev tip(45cfb04e9, package 2.57.0)에 맞춰져 있다. 바꾸는 파일은 딱 두 개다. src/router.ts와 src/responses/state.ts. 목적은 “기능 추가”가 아니라, 이미 자주 도는 두 길을 더 싸게 만드는 것이다.

첫째, 라우터 쪽. 지금 dev의 knownModelIdsForProvider / routedProviderConfig / routeModelInternal은 호출마다 PROVIDER_REGISTRY.find(...)로 배열을 훑고, 힌트 맵마다 Object.keys로 임시 배열을 만든다. 레지스트리는 모듈 로드 때 이미 고정된 목록이라, 매번 같은 선형 탐색을 반복하는 셈이다. 이 PR은 모듈 상단에 REGISTRY_BY_ID, REGISTRY_STATIC_MODEL_IDS, REGISTRY_ALIAS_BY_ID 세 맵을 미리 만들고, 핫 패스에서는 O(1) lookup만 한다. 슬러그 코덱이 기대하는 “알려진 native id 합집합” 의미는 그대로 두고, 계산만 앞당긴다.

둘째, 응답 스냅샷 쪽. writeBoundedSnapshot은 주기적으로 디스크에 상태를 쓴다. 지금 코드는 [...states].reverse()로 맵을 한 번 뒤집은 뒤, 각 엔트리를 JSON.stringify로 크기를 재고, 마지막에 다시 JSON.stringify({ version: 2, states: entries })로 전체 트리를 한 번 더 직렬화한다. 스냅샷 상한이 24 MiB라서, 큰 홈에서는 “크기 재기용 문자열 + 최종 payload 문자열”이 잠깐 동시에 살아 힙을 크게 쓴다. 이 PR은 (1) Array.from(states)를 인덱스 역순으로 걸어 뒤집기 할당을 줄이고, (2) 이미 만든 엔트리 JSON 문자열을 join,으로 이어 {"version":2,"states":[...]}를 손으로 조립한다. 바이트 길이·digest 비교 로직은 그대로라서, “이전에 쓴 것과 같으면 다시 안 쓴다”는 절약도 유지하려는 모양이다. 로컬에서 같은 엔트리 배열로 비교하면 손조립 payload와 JSON.stringify 결과가 바이트 단위로 같다.

지금 dev가 바쁘게 가는 축은 godfile facade 분할, send-budget/#4546, 2.56.0→2.57.0 릴리즈 레일이다. 이 PR은 그 축을 건드리지 않는 작은 perf 정리라서, 방향과 싸우지 않는다. 다만 CI hygiene이 missing_regression_test로 막혀 있고, enforce-target도 같은 이유로 실패했으며, 봇이 draft로 붙잡아 둔 상태다. 작성자가 로컬에서 router/state 관련 테스트를 돌렸다고 적었지만, 이 PR 자체에는 테스트 파일 변경이 없다. 게이트는 “src를 바꿨으면 테스트도 같이”를 본다.

src/router.ts - REGISTRY_* 맵은 모듈 로드 시점에만 채워진다. PROVIDER_REGISTRY가 readonly 상수라서 지금은 맞지만, 나중에 런타임에 레지스트리 항목을 붙이는 길이 생기면 이 캐시는 낡은 값이 된다. 지금은 문제 없고, “정적 전제”를 주석으로 남겨 두면 이후 기여자가 덜 헷갈린다.

src/router.ts knownModelIdsForProvider - 예전 코드는 transport guard를 통과한 뒤에야 registry models + hint map keys를 모았다. 새 코드도 guard 안에서 STATIC_MODEL_IDS만 읽으므로 의미는 같다. 다만 기존 slug-codec 테스트(tests/codex-integration/slug-codec.test.ts의 knownModelIdsForProvider 합집합 케이스)가 이미 있고, 이 PR은 그 파일을 건드리지 않아 hygiene에 걸린다. 캐시 경로를 고정하는 한 줄짜리 회귀(예: zenmux/nvidia hint-map id가 여전히 포함되는지)를 추가하는 편이 게이트와 리뷰어 모두에게 싸다.

src/responses/state.ts writeBoundedSnapshot - Array.from(states)도 여전히 O(N) 엔트리 배열을 만든다. 진짜 이득은 reverse 복사 제거보다, 전체 트리를 두 번 stringify하지 않는 쪽에 가깝다. PR 설명의 “24–48 MiB transient” 주장은 이 경로를 겨냥한 것이고, 방향은 타당하다.

src/responses/state.ts payload 조립 - 손조립 JSON이 digest/skip 로직의 입력이라, 바이트가 한 글자라도 어긋나면 “같다” 판정이 깨지거나 반대로 잘못된 skip이 난다. 로컬 등가성은 확인됐지만, tests/responses/responses-state.test.ts에 “flush 후 파일 내용이 version:2 + states 배열 형태이고, 같은 상태에서 두 번 flush하면 digest skip이 유지된다” 수준의 고정 테스트가 이 PR에 없다. hygiene가 가리키는 바로 그 구멍이다.

intake: hygiene-blocked / draft - 작성자 checklist는 4/4로 찍혀 있지만, 게이트는 여전히 missing_regression_test로 DRAFT다. “로컬에서 기존 테스트 통과”와 “이 PR이 테스트를 추가했다”는 다른 조건이다.

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

• missing_regression_test를 테스트 추가로 풀지, 아니면 좁은 perf-only라서 test-exception-approved를 줄지
• 스냅샷 payload 손조립을 “바이트 등가 고정 테스트 필수”로 볼지, 기존 flush 테스트만으로 충분한지
• 지금 릴리즈/godfile/[Bug]: Account pool routing destroys prompt cache and triggers 10x-50x token burn death-spiral above 80% usage threshold #4546 레일 옆에 이 perf를 언제 넣을지(작아서 언제든 가능하지만, 게이트 통과 전이면 대기)

너의 추천
머지하지 말고 draft 유지. 작성자에게 (1) tests/routing 또는 tests/codex-integration/slug-codec.test.ts에 REGISTRY 캐시 경로를 고정하는 회귀 한두 개, (2) tests/responses/responses-state.test.ts에 writeBoundedSnapshot payload/digest 등가(또는 이중 flush skip) 고정을 추가하라고 요청한 뒤, hygiene·enforce-target 초록이 되면 Ready로 올려 리뷰한다. 코드 방향 자체는 dev와 충돌하지 않으니, 게이트만 풀리면 작은 초록 후보로 보면 된다.

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

[lidge-jun]

Thank you, @chilung-cgu. This landed on dev through the maintainer carry #6360 (squash 0328373fb8), with you credited as co-author.

What was carried: the snapshot change. dev had since reshaped snapshot selection into selectSnapshotEntries, so the idea was reapplied there: the entry strings already serialized to measure their size are now joined into the payload instead of serializing the whole selection a second time. Output stays byte-identical. On a 24 MiB snapshot a standalone comparison against the previous dev code measured the median write going from about 29–30 ms to about 18 ms.

What was not carried: the static PROVIDER_REGISTRY lookup maps in src/router.ts. Measured against current dev, they made no measurable difference to routeModel, so they would add cached state without a benefit.

Closing as carried. Thanks again for finding the double serialization.