Conversation
|
Understand this PR’s impact Explore downstream dependencies and potential security impact with Blast Radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: lidge-jun/opencodex/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (8)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe change moves reasoning metadata snapshot refresh from request-time effort resolution into catalog synchronization. Sync checks configured provider destinations, refreshes metadata when supported, and runs the refresh before catalog gathering. Documentation and integration tests describe and verify this ordering. ChangesReasoning metadata synchronization
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~15 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant syncModelsToCodex
participant reasoningMetadata
participant catalog
syncModelsToCodex->>reasoningMetadata: Check configured provider destinations
reasoningMetadata-->>syncModelsToCodex: Return metadata support status
syncModelsToCodex->>reasoningMetadata: Refresh supported metadata snapshot
syncModelsToCodex->>catalog: Gather catalog after refresh
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 4 files. (6 skipped: 6 unsupported.)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
✅ Deterministic PR hygiene checks passed. |
✅ READY
Review readiness checklist
✅ 4/4 boxes ticked. Automatic ready-for-review conversion failed; please mark the pull request ready manually if it is still a draft. |
리뷰 · 우선순위 64 / 80이 PR은 models.dev reasoning-effort 스냅샷을 “요청이 올 때”가 아니라 “카탈로그 sync 할 때” 받아 오게 바꿉니다. 예전에는 스냅샷이 이미 있을 때만 백그라운드로 갱신해서, 없거나 깨진 상태면 영원히 안 받아 왔습니다. 지금은 라인 - GitHub mergeable= 메인테이너의 판단이 필요한 지점 #5268이 이미 reasoning-metadata·effort를 credential 범위로 바꾼 뒤라, 이 PR을 너의 추천 방향은 맞고, “없는 스냅샷을 요청 경로에서 안 받고 sync가 받는다”는 설명이 코드·테스트·문서와 맞습니다. 지금 머지하지 마세요. 먼저 이 댓글은 grok-bot이 작성했습니다 |
cb8a85a to
fcca85a
Compare
The models.dev effort snapshot was only ever refreshed after a snapshot had already answered a request-time ladder read, so a missing or corrupt snapshot was never bootstrapped. Catalog sync now refreshes the snapshot for configured gated destinations before gathering the catalog, keeping the models.dev fetch off the request path.
fcca85a to
a58bee8
Compare
|
Checklist note: |
|
Consolidated into #5503 in native Stack #5505. Source head: Closing this standalone PR as a duplicate review entry at the author's request. The change is not merged or released: the replacement remains draft, with remaining integration/CI/security-review work tracked there. Original branches are retained. The maintainer-cancelled CI is not treated as a pass. |
Summary
syncModelsToCodexand the management-triggered catalog refresh now bootstrap the optional models.dev reasoning-effort snapshot before gathering the catalog, but only when a configured provider is a destination the snapshot covers (providerUsesReasoningMetadata).src/reasoning-effort.tsno longer kick off a background refresh: a missing or corrupt snapshot is now fetched by catalog sync instead of never being fetched at all, and no models.dev request can interleave with a routed turn.Verification
bun test tests/codex-integration/codex-sync-api.test.ts— 17 pass, including a new ordering test asserting the metadata refresh runs before catalog gathering for a gated provider.bun x tsc --noEmit— clean.bun run structure:check— green.Checklist
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Documentation
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.