Skip to content

feat(combo): adapt reasoning effort to target capabilities - #2734

Closed
tone-leh wants to merge 1 commit into
lidge-jun:devfrom
tone-leh:feat/adaptive-combo-reasoning
Closed

tone-leh wants to merge 1 commit into
lidge-jun:devfrom
tone-leh:feat/adaptive-combo-reasoning

Conversation

@tone-leh

@tone-leh tone-leh commented Aug 27, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Add omitReasoningEffortWithToolsModels for OpenAI Chat targets that support selectable reasoning on ordinary requests but reject the tools plus reasoning-parameter combination. The adapter omits the incompatible wire field only when the request actually carries tools.
  • Add opt-in Combo reasoningEffortMode: adaptive. Default strict behavior remains unchanged; adaptive derives the picker ladder from non-empty target ladders while concrete targets still omit unsupported effort fields.
  • Validate, normalize, expose, PATCH, and document both configuration surfaces.

Closes #2731

Verification

  • Focused adapter, provider-validation, combo, catalog, and management tests: 403 pass.
  • bun run typecheck
  • bun run test using the repository standard preload and parallel runner.
  • bun run privacy:scan
  • docs-site: bun install --frozen-lockfile and bun run build.

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.
  • Local CI green against the current PR commit.
  • Branch is on the latest dev commit.
  • All correct Codex and CodeRabbit findings are fixed.
  • Ready for review confirmation.

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 Aug 27, 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 added the intake: hygiene-blocked Deterministic PR hygiene checks failed label Aug 27, 2026
@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Deterministic hygiene checks failed.

  • unsponsored_surface — This changes an authentication, workflow, release-automation, or dependency surface. MAINTAINERS.md requires security review for these; ask a maintainer to apply maintainer-sponsored once they have reviewed it. Paths: src/server/auth-cors.ts.

@github-actions github-actions Bot added the enhancement New feature or request label Aug 27, 2026
@github-actions

github-actions Bot commented Aug 27, 2026 •

Copy link
Copy Markdown
Contributor

⏳ DRAFT

  • hygiene: unsponsored_surface.

What to do

  • Fix unsponsored_surface — This changes an authentication, workflow, release-automation, or dependency surface. MAINTAINERS.md requires security review for these; ask a maintainer to apply maintainer-sponsored once they have reviewed it. Paths: src/server/auth-cors.ts.
  • 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 pull request was already a draft. Its draft status will be preserved after every issue above is resolved.
@TooSpace Tick the boxes once your local CI is green, your branch is on the latest dev commit, and every correct Codex and CodeRabbit finding is resolved.

Hygiene

⚠️ Deterministic hygiene checks failed.

  • unsponsored_surface — This changes an authentication, workflow, release-automation, or dependency surface. MAINTAINERS.md requires security review for these; ask a maintainer to apply maintainer-sponsored once they have reviewed it. Paths: src/server/auth-cors.ts.

@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 62 / 80

이 PR은 이슈 #2731을 구현합니다. OpenAI Chat 쪽에 omitReasoningEffortWithToolsModels를 추가해서, 도구가 있을 때만 reasoning effort 와이어 필드를 빼고, 콤보에는 기본 strict를 유지한 채 선택형 reasoningEffortMode: "adaptive"를 넣습니다. adaptive는 공개 피커 사다리를 비어 있지 않은 타깃 사다리의 교집합으로 만들고, 빈 사다리 타깃은 피커를 망가뜨리지 않으면서 요청 시 effort를 생략합니다. 지금 dev에는 이 두 설정이 없습니다. deriveComboCatalogModel은 빈 사다리도 교집합에 넣고, openai-chat 어댑터는 noReasoningModels만으로 통째로 끕니다. 그래서 도구+effort를 거절하는 모델과 노력 없는 모델을 한 콤보에 넣기 어렵습니다. 문서·zod·관리 API PATCH·DTO·테스트까지 한 세트로 맞춰 두었습니다.

src/adapters/openai-chat.ts - 도구가 있고 목록에 있으면 mapReasoningEffort 결과를 버리고, gateway-object의 reasoning: none 경로도 같이 막습니다. 도구 없을 때는 기존처럼 effort를 보냅니다.
src/codex/catalog/aggregation.ts - adaptive일 때만 길이 0 사다리를 교집합에서 뺍니다. 기본 strict는 지금 dev와 같습니다.
src/combos/types.ts / config 스키마 - reasoningEffortMode 검증·정규화 기본값이 strict라서 기존 콤보 동작이 바뀌지 않습니다.
src/server/management/provider-routes.ts - PATCH에서 omitReasoningEffortWithToolsModels: null로 삭제할 수 있게 했습니다. (참고: 예전에 비어 있던 oauthAccountFailover:null PATCH 구멍과는 별개입니다.)
tests/* - 어댑터·카탈로그·관리 검증·정규화 테스트가 핵심 분기를 덮습니다. 드래프트 체크리스트는 아직 비어 있습니다.

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

  • 아직 draft이고 로컬 CI·최신 dev 맞춤·CodeRabbit 정리 체크가 안 끝났습니다. 기능 방향은 #2731과 잘 맞지만, 지금 바로 머지할 상태는 아닙니다.
  • adaptive에서 모든 알려진 타깃 사다리가 비면 피커는 여전히 빈 배열입니다. 문서에 적어 두었지만, 운영자가 "adaptive면 무조건 피커가 생긴다"고 오해하지 않게 릴리즈 노트에 한 줄 더 있으면 좋습니다.
  • config.ts / types/*에 필드가 추가됩니다. types/config 분할 캠페인 이후에도 이 파일들은 자주 겹치니, 머지 직전에 dev를 다시 맞추고 충돌만 확인하면 됩니다. 닫고 다시 쓰라는 대상은 아닙니다.

너의 추천
드래프트로 두고 체크리스트 네 칸을 채운 뒤 ready로 올린 다음 머지하세요. 이슈 #2731은 이 PR이 머지될 때 Closes #2731로 같이 닫히게 두면 됩니다. 지금은 이슈를 먼저 닫지 마세요.

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

@lidge-jun
lidge-jun force-pushed the feat/adaptive-combo-reasoning branch 2 times, most recently from 1c16381 to 573d0f6 Compare August 29, 2026 02:16
lidge-jun pushed a commit that referenced this pull request Sep 1, 2026
Adopts the design from #2734 and completes the surfaces it left open.

Two defects, both opt-in fixes that default to today's behavior:

- A combo intersects its targets' effort ladders, and an explicitly empty
  ladder (a target with no effort control) collapsed the intersection for the
  whole group. reasoningEffortMode: "adaptive" excludes empty ladders from the
  published intersection; "strict" remains the default.
- openai-chat could declare per-model effort support but not a per-request
  rule, so a model that rejects effort only alongside tools had to use
  noReasoningModels and lose its picker entirely.
  omitReasoningEffortWithToolsModels drops the wire field for tool-bearing
  requests only.

Beyond #2734: the dashboard round-trip. PUT /api/combos replaces the stored
combo wholesale and the GUI serializer is a field allowlist, so a hand-edited
reasoningEffortMode was silently destroyed by any dashboard combo edit. The
field is now parsed, serialized, dirty-checked and sync-keyed, and the shared
sparse helper keeps GET, the PUT response and disk consistent so an unset
default is never materialized into config.json.

The opt-in is a switch in the combo Capabilities section beside image input,
where combo-wide policy already lives - not under Default reasoning, which is
a value chosen from the intersection rather than a rule that changes it. Both
the create modal and the detail panel share that component, so the create flow
shows the corrected picker too.

Refs #2731. Supersedes #2734.
lidge-jun added a commit that referenced this pull request Sep 1, 2026
…3190)

* feat(combo): adapt reasoning effort to target capabilities

Adopts the design from #2734 and completes the surfaces it left open.

Two defects, both opt-in fixes that default to today's behavior:

- A combo intersects its targets' effort ladders, and an explicitly empty
  ladder (a target with no effort control) collapsed the intersection for the
  whole group. reasoningEffortMode: "adaptive" excludes empty ladders from the
  published intersection; "strict" remains the default.
- openai-chat could declare per-model effort support but not a per-request
  rule, so a model that rejects effort only alongside tools had to use
  noReasoningModels and lose its picker entirely.
  omitReasoningEffortWithToolsModels drops the wire field for tool-bearing
  requests only.

Beyond #2734: the dashboard round-trip. PUT /api/combos replaces the stored
combo wholesale and the GUI serializer is a field allowlist, so a hand-edited
reasoningEffortMode was silently destroyed by any dashboard combo edit. The
field is now parsed, serialized, dirty-checked and sync-keyed, and the shared
sparse helper keeps GET, the PUT response and disk consistent so an unset
default is never materialized into config.json.

The opt-in is a switch in the combo Capabilities section beside image input,
where combo-wide policy already lives - not under Default reasoning, which is
a value chosen from the intersection rather than a rule that changes it. Both
the create modal and the detail panel share that component, so the create flow
shows the corrected picker too.

Refs #2731. Supersedes #2734.

* test(combo): cover adaptive effort mode and the tool-bearing opt-out

Activation-grounded regressions for both halves of #2731, plus the dashboard
round-trip the original PR left uncovered.

- codex-catalog: adaptive keeps the surviving sibling ladder where strict
  empties it, still intersects non-empty ladders, and yields nothing when every
  target is empty.
- openai-chat-hardening: effort dropped only with tools present, kept without
  them, untouched for an unlisted sibling and for an unset provider list, and
  suppressed on the gateway-object branch that writes its own reasoning field.
- combo-management-api: adaptive survives PUT to disk to GET; the strict default
  is never materialized; turning it back off clears the stored field; an unknown
  value is rejected.
- combo-workspace-data: the GUI intersection honors the mode, parse and
  toPutBody round-trip it, and draftEquals treats a change as dirty so Save
  enables.
- management-provider-validation: validates, exposes and normalizes the new
  provider list.

* docs(devlog): record the 3190 cherry-pick onto privacy-clean dev

---------

Co-authored-by: jun <jun@lidge.dev>
@lidge-jun

Copy link
Copy Markdown
Owner

Superseded by #3190, landed on origin/dev as 88c4275. That carry keeps the adaptive effort-mode design and closes the dashboard round-trip this draft left open.

@lidge-jun lidge-jun closed this Sep 1, 2026
@lidge-jun lidge-jun added the landed-via-maintainer Original PR closed after landing via a maintainer merge train label Sep 1, 2026
tarunravi pushed a commit to tarunravi/opencodex that referenced this pull request Sep 14, 2026
…#2731) (lidge-jun#3190)

* feat(combo): adapt reasoning effort to target capabilities

Adopts the design from lidge-jun#2734 and completes the surfaces it left open.

Two defects, both opt-in fixes that default to today's behavior:

- A combo intersects its targets' effort ladders, and an explicitly empty
  ladder (a target with no effort control) collapsed the intersection for the
  whole group. reasoningEffortMode: "adaptive" excludes empty ladders from the
  published intersection; "strict" remains the default.
- openai-chat could declare per-model effort support but not a per-request
  rule, so a model that rejects effort only alongside tools had to use
  noReasoningModels and lose its picker entirely.
  omitReasoningEffortWithToolsModels drops the wire field for tool-bearing
  requests only.

Beyond lidge-jun#2734: the dashboard round-trip. PUT /api/combos replaces the stored
combo wholesale and the GUI serializer is a field allowlist, so a hand-edited
reasoningEffortMode was silently destroyed by any dashboard combo edit. The
field is now parsed, serialized, dirty-checked and sync-keyed, and the shared
sparse helper keeps GET, the PUT response and disk consistent so an unset
default is never materialized into config.json.

The opt-in is a switch in the combo Capabilities section beside image input,
where combo-wide policy already lives - not under Default reasoning, which is
a value chosen from the intersection rather than a rule that changes it. Both
the create modal and the detail panel share that component, so the create flow
shows the corrected picker too.

Refs lidge-jun#2731. Supersedes lidge-jun#2734.

* test(combo): cover adaptive effort mode and the tool-bearing opt-out

Activation-grounded regressions for both halves of lidge-jun#2731, plus the dashboard
round-trip the original PR left uncovered.

- codex-catalog: adaptive keeps the surviving sibling ladder where strict
  empties it, still intersects non-empty ladders, and yields nothing when every
  target is empty.
- openai-chat-hardening: effort dropped only with tools present, kept without
  them, untouched for an unlisted sibling and for an unset provider list, and
  suppressed on the gateway-object branch that writes its own reasoning field.
- combo-management-api: adaptive survives PUT to disk to GET; the strict default
  is never materialized; turning it back off clears the stored field; an unknown
  value is rejected.
- combo-workspace-data: the GUI intersection honors the mode, parse and
  toPutBody round-trip it, and draftEquals treats a change as dirty so Save
  enables.
- management-provider-validation: validates, exposes and normalizes the new
  provider list.

* docs(devlog): record the 3190 cherry-pick onto privacy-clean dev

---------

Co-authored-by: jun <jun@lidge.dev>
agentHits pushed a commit to agentHits/opencodex that referenced this pull request Sep 17, 2026
…#2731) (lidge-jun#3190)

* feat(combo): adapt reasoning effort to target capabilities

Adopts the design from lidge-jun#2734 and completes the surfaces it left open.

Two defects, both opt-in fixes that default to today's behavior:

- A combo intersects its targets' effort ladders, and an explicitly empty
  ladder (a target with no effort control) collapsed the intersection for the
  whole group. reasoningEffortMode: "adaptive" excludes empty ladders from the
  published intersection; "strict" remains the default.
- openai-chat could declare per-model effort support but not a per-request
  rule, so a model that rejects effort only alongside tools had to use
  noReasoningModels and lose its picker entirely.
  omitReasoningEffortWithToolsModels drops the wire field for tool-bearing
  requests only.

Beyond lidge-jun#2734: the dashboard round-trip. PUT /api/combos replaces the stored
combo wholesale and the GUI serializer is a field allowlist, so a hand-edited
reasoningEffortMode was silently destroyed by any dashboard combo edit. The
field is now parsed, serialized, dirty-checked and sync-keyed, and the shared
sparse helper keeps GET, the PUT response and disk consistent so an unset
default is never materialized into config.json.

The opt-in is a switch in the combo Capabilities section beside image input,
where combo-wide policy already lives - not under Default reasoning, which is
a value chosen from the intersection rather than a rule that changes it. Both
the create modal and the detail panel share that component, so the create flow
shows the corrected picker too.

Refs lidge-jun#2731. Supersedes lidge-jun#2734.

* test(combo): cover adaptive effort mode and the tool-bearing opt-out

Activation-grounded regressions for both halves of lidge-jun#2731, plus the dashboard
round-trip the original PR left uncovered.

- codex-catalog: adaptive keeps the surviving sibling ladder where strict
  empties it, still intersects non-empty ladders, and yields nothing when every
  target is empty.
- openai-chat-hardening: effort dropped only with tools present, kept without
  them, untouched for an unlisted sibling and for an unset provider list, and
  suppressed on the gateway-object branch that writes its own reasoning field.
- combo-management-api: adaptive survives PUT to disk to GET; the strict default
  is never materialized; turning it back off clears the stored field; an unknown
  value is rejected.
- combo-workspace-data: the GUI intersection honors the mode, parse and
  toPutBody round-trip it, and draftEquals treats a change as dirty so Save
  enables.
- management-provider-validation: validates, exposes and normalizes the new
  provider list.

* docs(devlog): record the 3190 cherry-pick onto privacy-clean dev

---------

Co-authored-by: jun <jun@lidge.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request intake: hygiene-blocked Deterministic PR hygiene checks failed landed-via-maintainer Original PR closed after landing via a maintainer merge train

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants