Skip to content

refactor(router): isolate API-key selection capture - #3897

Draft
parkjs101 wants to merge 1 commit into
lidge-jun:devfrom
parkjs101:codex/router-selection-capture
Draft

refactor(router): isolate API-key selection capture#3897
parkjs101 wants to merge 1 commit into
lidge-jun:devfrom
parkjs101:codex/router-selection-capture

Conversation

@parkjs101

@parkjs101 parkjs101 commented Sep 7, 2026

Copy link
Copy Markdown

Summary

Closes #3894.

Move captureProviderApiKeySelection unchanged into a small provider module with type-only dependencies. The router imports this leaf directly, removing its direct dependency on the stateful selection module. The original module imports and re-exports the same function, preserving existing callers.

Focused tests cover selected, missing, unmatched, and repeated pool references, immutable snapshots, compatibility export identity, and the runtime import boundary. Both test-layout registries and the runtime structure map include the new files. Other transitive router cycles are outside this change.

Verification

Windows, Bun 1.4.0; independent branch from dev at 522ce5f8c8527d1e6c479a0090af14e390214bcb.

  • bun test tests/providers/api-key-selection-capture.test.ts tests/adapters/key-failover.test.ts tests/providers/provider-key-store.test.ts tests/lab/core-lab-boundary.test.ts tests/test-layout.test.ts tests/test-layout-tooling.test.ts: 73 passed, 0 failed.
  • Reintroducing the original router import caused the new boundary test to fail; restoring the extraction passed.
  • Baseline key-failover, provider-key-store, and Lab-boundary suites: 49 passed.
  • bun run typecheck: passed.
  • bun run privacy:scan: passed.
  • bun install --frozen-lockfile: passed; lockfile unchanged.

This draft does not attest the repository-wide test suite or maintainer approval. Plan and evidence: devlog/_plan/260907_router_selection_capture/010_implementation.md.

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.

Maintainer review remains pending; selection, resolution, and failover behavior are unchanged.

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

  • New Features

    • Added provider API key selection capture, preserving the selected entry ID, reference, and revision.
    • Preserved compatibility with the existing selection export.
  • Bug Fixes

    • Improved module separation to prevent unnecessary runtime dependencies during routing.
  • Documentation

    • Documented the new provider selection capture component and its role in the runtime structure.
  • Tests

    • Added coverage for matching, unmatched, repeated, and optional API key selections.

@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Review 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: Team

Run ID: c4dcd752-662c-4e22-9c04-056ec26df84b

📥 Commits

Reviewing files that changed from the base of the PR and between 522ce5f and 356f2c1.

📒 Files selected for processing (8)
  • devlog/_plan/260907_router_selection_capture/010_implementation.md
  • scripts/test-layout/layout.json
  • src/providers/api-key-selection-capture.ts
  • src/providers/api-key-selection.ts
  • src/router.ts
  • structure/01_runtime.md
  • tests/fixtures/test-layout-expected.json
  • tests/providers/api-key-selection-capture.test.ts

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


📝 Walkthrough

Walkthrough

The change extracts captureProviderApiKeySelection into a new leaf module, updates router.ts and api-key-selection.ts imports, preserves the legacy export, registers the test layout, updates runtime documentation, and adds behavior and dependency-boundary tests.

Changes

Router selection capture

Layer / File(s) Summary
Extract the selection snapshot leaf
src/providers/api-key-selection-capture.ts, src/providers/api-key-selection.ts, src/router.ts
The helper now creates the same ProviderApiKeySelection snapshot from the provider API-key pool, reference, and revision. api-key-selection.ts re-exports it, and router.ts imports it from the new leaf module.
Validate snapshot behavior and imports
tests/providers/api-key-selection-capture.test.ts, scripts/test-layout/layout.json, tests/fixtures/test-layout-expected.json
Tests cover matched, unmatched, missing, and duplicate pool entries, immutability, export identity, and runtime import boundaries. The new test is registered under the providers layout.
Record ownership and verification
structure/01_runtime.md, devlog/_plan/260907_router_selection_capture/010_implementation.md
Runtime documentation identifies the new pure entrypoint. The implementation plan records the module map, verification results, and scoped non-goals.

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

Merge Risk: ⚪ Minimal · up to 356f2

This change moves API-key selection snapshot capture into a shared leaf module while retaining the existing export and routing behavior. The covered compatibility and import-boundary changes present no remaining merge-blocking risk.

🚥 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 4 files. (4 skipped: 4 … 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, "refactor(router): isolate API-key selection capture," clearly identifies the main change: isolating API-key selection capture from the router dependency cycle.
Linked Issues check ✅ Passed The changes satisfy issue #3894. The helper moved to the dependency-neutral module src/providers/api-key-selection-capture.ts, src/router.ts imports the new leaf module, src/providers/api-key-selectio…
Out of Scope Changes check ✅ Passed The changes remain within issue #3894. The implementation, focused tests, test-layout updates, and structure documentation directly support the module extraction, compatibility export, and dependency-…
Full details: Docstring Coverage

Explanation

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 4 files. (4 skipped: 4 unsupported.)

  • 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.

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the chore Maintenance, CI, tests, refactors, or build changes (not a user-facing bug or feature). label Sep 7, 2026
@github-actions

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

@parkjs101
parkjs101 marked this pull request as ready for review September 7, 2026 11:55
@github-actions
github-actions Bot marked this pull request as draft September 7, 2026 11:58
@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 52 / 80

이 PR은 #3894를 닫습니다. 지금은 src/router.tscaptureProviderApiKeySelection 때문에 src/providers/api-key-selection.ts를 가져오고, 그 모듈은 다시 routedProviderConfig 때문에 router를 가져와서 직접 런타임 순환이 생깁니다. 이 변경은 캡처 함수 본문을 그대로 src/providers/api-key-selection-capture.ts로 옮기고, 라우터는 그 leaf만 import합니다. 옛 api-key-selection.ts는 같은 심볼을 re-export해서 기존 호출자는 깨지지 않습니다. 키 해석·failover·선택 저장 동작은 바꾸지 않는 의존성 위생 작업입니다.

새 leaf는 OcxProviderConfig / ProviderApiKeySelection type-only import만 쓰고, 런타임 import가 없습니다. 테스트는 선택·미매칭·빈 풀·중복 reference first-match·불변 스냅샷·legacy export identity, 그리고 Bun Transpiler로 leaf/router의 런타임 import 경계를 검사합니다. scripts/test-layout/layout.json과 fixture, structure/01_runtime.md 소유권 행도 맞춰 두었습니다. 포커스 스위트 73개 통과·typecheck·privacy scan을 보고했고, 옛 router import를 되돌리면 boundary 테스트가 깨진다고 재현했습니다. 다른 전이적 router 순환은 이번 범위 밖이라고 본문에 명시되어 있습니다.

src/providers/api-key-selection-capture.ts - 함수 본문이 이동만 되었고 로직 변경이 없습니다. entryId는 apiKeyPool에서 apiKey와 같은 key의 첫 id입니다.

src/providers/api-key-selection.ts - 구현 삭제 후 export { captureProviderApiKeySelection } from "./api-key-selection-capture"로 호환을 지킵니다. matchesSelection 등은 계속 이 모듈에 남습니다.

src/router.ts - import 경로만 leaf로 바뀝니다. 순환의 한쪽 화살표가 끊깁니다.

tests/providers/api-key-selection-capture.test.ts · dependency boundary - source를 읽어 런타임 import를 검사하므로 문서 문자열에 속지 않습니다. “전체 router 비순환”을 주장하지 않는 점이 솔직합니다.

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

  • 이 한 순환만 끊는 것으로 충분한지, 남은 전이적 순환을 이어서 칠지
  • draft checklist가 비어 있는데 위생 PR을 지금 랜딩할지
  • re-export를 당분간 유지할지, 호출자를 leaf로 옮긴 뒤 곧 제거할지

너의 추천
동작 변경 없는 작은 추출이고 경계 테스트까지 있으니 mid로 머지해도 됩니다. checklist만 채운 뒤 넣고, 남은 router 순환은 별도 이슈로 남겨 두세요. #3894는 이 PR과 함께 닫으면 됩니다.

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

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

Labels

chore Maintenance, CI, tests, refactors, or build changes (not a user-facing bug or feature).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants