Skip to content

fix(providers): inherit reasoning metadata for renamed key destinations - #4370

Merged
lidge-jun merged 29 commits into
devfrom
codex/260912-60plus-models-destination
Sep 13, 2026
Merged

fix(providers): inherit reasoning metadata for renamed key destinations#4370
lidge-jun merged 29 commits into
devfrom
codex/260912-60plus-models-destination

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Sep 12, 2026

Copy link
Copy Markdown
Owner

Summary

A renamed API-key preset such as CommandCode now inherits missing reasoning-effort tables from its existing fixed destination match. Explicit per-model lists, including empty arrays, and explicit provider-wide lists are preserved. Unknown models retain the preset's default policy.

Refs #4308. This fixes missing destination metadata independently of the already-landed #4282 table row. It does not change provider identity, routing, OAuth matching or unrelated capability axes. Regression coverage follows gathered rows into DSH and ZCode exports.

Verification

  • Independent source-plan review PASS; Arendt final source review PASS at a2fb378.
  • Static manifest owner coverage and git diff --check passed.
  • Local suites of every size, build, typecheck and install: NOT RUN by explicit maintainer instruction. GitHub-hosted final-head CI is required after publication.
  • Independent dev PR; no merge or source issue closure performed.

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.

@coderabbitai

coderabbitai Bot commented Sep 12, 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

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Sep 12, 2026
@lidge-jun

Copy link
Copy Markdown
Owner Author

리뷰 · 우선순위 56 / 80

이 PR은 CommandCode처럼 이름이 바뀐 API-key preset이, fixed destination matcher에 걸릴 때 비어 있는 reasoning-effort 메타를 registry에서 상속하게 합니다 (#4308). 이미 올라간 #4282 table row와 별개로 “destination metadata 공백”만 고칩니다. provider identity·routing·OAuth matching·다른 capability 축은 건드리지 않는다고 본문에 명시되어 있습니다.

현재 checkout src/providers/derive.ts에는 enrichReasoningSummariesByDestination만 있고 summary map만 채웁니다. 이 PR은 함수를 enrichReasoningMetadataByDestination로 넓혀 modelReasoningEffortsfillRecordOfArrays로 채우고, provider-wide reasoningEfforts는 undefined일 때만 복사합니다. 명시적 per-model 리스트(빈 배열 포함)와 명시적 provider-wide 리스트는 유지됩니다. 테스트는 provider-registry-parity.test.ts에서 CommandCode known/newer/unknown/empty/non-match를 고정하고, client-config-export.test.ts에서 DSH/ZCode export까지 gathered row가 전달되는지 봅니다.

draft·base dev이지만 CONFLICTING/DIRTY입니다. +116/-3으로 작지만 derive.ts는 자주 충돌합니다. #4374 capability map이나 #4346 models-reasoning과 이웃 개념이라, 큰 capability PR에 흡수될지 독립 픽스로 살릴지 선택이 필요합니다. types/config split 본체는 거의 안 건드립니다(docs/structure와 derive·tests 중심).

CommandCode 같은 renamed key destination은 사용자 설정에 자주 남고, reasoning ladder가 비면 클라이언트 export(DSH/ZCode)와 Codex inbound reasoning drop으로 이어집니다. 변경이 derive 한 함수와 테스트·문서에 묶여 있어 리스크 대비 효과가 큽니다. 충돌만 아니면 READY 후보에 가깝습니다.

경로 enrichReasoningMetadataByDestination - summary와 effort를 한 함수에 묶은 것은 읽기 좋습니다. 다만 destination match가 adapter/authMode/baseUrl에 엄격하므로, 사용자가 baseUrl을 살짝 바꾼 CommandCode 클론은 계속 비어 있습니다. 문서의 “fixed key destination only”와 일치하니 괜찮지만, #4308 재현 환경이 그 제약을 만족하는지 확인하세요.

테스트 unknown-model - registry empty picker policy([]) vs provider-wide medium override 시나리오가 나뉘어 있어 좋습니다. export 테스트의 모델 id가 현재 catalog와 어긋나도 fixture로는 충분해 보입니다.

충돌 - 현 브랜치 그대로 머지 불가.

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

너의 추천
conflict만 풀면 land 가치가 있는 작은 픽스입니다. #4374가 close/지연되면 이 PR을 먼저 살리는 편이 사용자-visible export/reasoning ladder에 바로 도움이 됩니다. draft 유지한 채 rebase(또는 새 최소 PR) 후 CI 녹색으로 승격하세요.

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

lidge-jun and others added 9 commits September 12, 2026 14:45
Carry #4100 with case-preserving keys, normalized editor adoption, and recoverable native root-stamp provenance.

Co-authored-by: HarryZhou <2373256746@qq.com>
Refs #4308. Fill missing metadata through existing destination matching while preserving explicit provider and model overrides.
Refs #3377. Add strict management writes, per-axis load recovery, exact model IDs, and declaration-preserving DTO/mutation paths. Upstream tier/video activation remains evidence-gated.
…bilities

Refs #3377 and original text-only request #3268 by @turin-dev. Reuse existing vision description/omission handling, preserve exact model keys and legacy fallback.
@lidge-jun
lidge-jun force-pushed the codex/260912-60plus-models-destination branch from a2fb378 to 6c29ffa Compare September 12, 2026 05:47
Keep this layer's exact modelCapabilities storage and validation, and take
origin/dev's HMAC admission imports plus current structure ownership notes.
Keep this layer's exact-modality text-only consumer note and retain the
Cline CLI registry-parity sentence from origin/dev.
Keep reviewer selectors and modelCapabilities together in config schema,
editor adoption, and management tests; retain both structure ownership notes.
Keep renamed-destination reasoning inheritance with the stacked
modelCapabilities, text-only, and reviewer contracts.
The dev merge appended this layer's cross-reference notes after sections
origin/dev had added, so they rendered under Context relay ownership, the
OAuth Fast Tier section, and the capability section. Move them back beside
the prose they describe; no wording changes.
…t-only

Carry the lower layer's note placement and keep this layer's text-only
consumer and input-declaration notes beside the sections they describe.
Carry the lower layers' note placement, put this layer's reviewer note with
them, and give the reviewer heading the blank line Markdown needs to render it
as a heading.
Carry the lower layers' note placement and keep this layer's renamed-destination
note with them, above the OAuth Fast Tier section.
Keep both sides' config-load degradation tests and the stack's structure notes
beside the sections origin/dev added.
One conflict in structure/adapters/registry.md: both sides appended a new section at the same anchor. Kept the lane capability/reviewer paragraphs and dev SWE-2 effort section.
@lidge-jun

Copy link
Copy Markdown
Owner Author

Maintainer integration record — providers lane tip, merged by @lidge-jun.

Tip of the cumulative providers stack. Merging with a merge commit lands the lane and lets GitHub close each link as MERGED: #4374 (exact per-model capability declarations), #4376 (text-only models routed through the vision sidecar), #4358 (provider approval reviewer selection), and #4370 (reasoning metadata inherited for renamed key destinations).

  • Exact head at integration: 909607ab4d86d9bf5b736b0d6c4e6fc5bb0a6467.
  • CI evidence: the previous head 662086448 passed the full Cross-platform CI matrix — ci aggregate, gates, Linux shards 1-4, both macOS shards, keyring, docker smoke, npm-global, storage policy, api usage, react-doctor, and enforce-target.
  • The head above adds exactly one origin/dev merge on top of that verified content. It had a single conflict, in structure/adapters/registry.md, where both sides appended a new section at the same anchor; both were kept, the lane's capability and reviewer paragraphs and dev's SWE-2 effort section. Re-verified locally after the resolution: bun run structure:check, bun run typecheck and bun run privacy:scan all pass.
  • Ancestry verified with git merge-base --is-ancestor for all three links against this tip, which is what lets them close as merged rather than by hand.
  • Non-tip links carried [skip ci] by design, and this mechanical dev re-merge did too. Tip-only CI for this batch is an explicit repository-owner decision; MAINTAINERS.md otherwise requires a per-PR check, so this is a recorded deviation rather than something the maintainer-integration clause grants. The dev-branch run triggered by this merge is the regression gate.
  • Review state: no outstanding maintainer change requests.
  • Security review: not applicable — no authentication, credential, OAuth, workflow, release-automation, or dependency-installation surface.

@lidge-jun
lidge-jun marked this pull request as ready for review September 13, 2026 03:20
@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner September 13, 2026 03:20
@lidge-jun
lidge-jun merged commit 096d843 into dev Sep 13, 2026
5 checks passed
@lidge-jun
lidge-jun deleted the codex/260912-60plus-models-destination branch September 13, 2026 03:20
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 13, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-13T03:22:07.015173Z 909607a Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

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.

1 participant