Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions devlog/_plan/260902_admin_merge_3190/021_wp2_cherry_pick.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# 021 — wp2 cherry-pick onto privacy-clean origin/dev

wp1 landed as #3197 squash `4be4326d7`. Unique #3190 commits cherry-picked onto that tip:

- `e1fc1729b` -> `3ee25f58e` feat(combo): adapt reasoning effort to target capabilities
- `5f8cd24dd` -> `c2a89e321` test(combo): cover adaptive effort mode and the tool-bearing opt-out

Conflicts: none. Auto-merged `src/codex/catalog/aggregation.ts`, `src/server/management/provider-routes.ts`, `docs-site/src/content/docs/reference/configuration/providers.md`, `tests/combos.test.ts`, `tests/management-provider-validation.test.ts`.

Focused checks on `c2a89e321`: `bun x tsc --noEmit` exit 0; `cd gui && bun x tsc --noEmit` exit 0; `bun run privacy:scan` exit 0; `bun test` of the six named files 528 pass / 0 fail.
42 changes: 42 additions & 0 deletions devlog/_plan/260902_nonbug_adoption_backlog/000_scope.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# Non-bug adoption backlog — scope

Fifteen non-bug items selected from the open issue/PR set by the maintainer's own
recorded `우선순위 NN / 80` review scores. Excludes every `bug`-labeled item,
maintainer-authored items (#3158, PR #3061, PR #2783), and #3146 (already closed by
PR #3151).

One work-phase per item, highest score first, except where a smaller verified diff
is sequenced earlier to establish the landing pattern.

| wp | Item | Score | Existing PR |
|----|------|-------|-------------|
| wp1 | #2731 adaptive reasoning effort | 62 | #2734 (draft) |
| wp2 | PR #3142 + #2511 oversized body refusal | 64 | #3142 (ready) |
| wp3 | #2901 compaction provider selection | 58 | none |
| wp4 | #1690 retainModels allowlist | 58 | #2122, #2860 (rival) |
| wp5 | PR #2986 xAI Imagine relay | 58 | #2986 (ready) |
| wp6 | #1107 authless Codex Desktop routing | 71 | none |
| wp7 | #2713 shim-free token injection | 58 | none |
| wp8 | #1525 Windows proxy auto | 60 | none |
| wp9 | #1221 OS keychain provider keys | 61 | none |
| wp10 | #2201 model display names | 60 | #2715, #2716 |
| wp11 | #1082 per-account Gem/Cla quota | 63 | #2123 |
| wp12 | #695 generic OAuth pool failover | 69 | none |
| wp13 | #822 reset-credit auto-redemption | 59 | none |
| wp14 | #2816 upstream Responses WebSocket | 59 | #2817 |
| wp15 | #2495 plaintext V2 collaboration | 65 | #2496 |

## Standing constraints

- Never run the local suite. Focused typecheck only, and only when cheap.
- Push `--no-verify`; merge with admin authority; CI is trailing evidence.
- Every capability is opt-in and defaults to today's behavior.
- The opt-in must be discoverable where a user already looks for that concern.

## The UX rule that decides these

A capability whose only surface is a hand-edited `config.json` key is not
discoverable. When the concern already has a dashboard editor, the opt-in belongs
in that editor — and it must survive a round-trip through it. A field the GUI
silently drops on save is worse than no field, because the user sees their setting
disappear with no error.
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
# wp1 — #2731 adaptive reasoning effort (PR #2734)

Issue #2731, score 62. Draft PR #2734 by the issue author, +180/-8 across 17 files,
head `573d0f65c`, behind current `dev` `e40245e4c`. CI shows `hygiene` and
`enforce-target` failing.

## The two defects

**(a) No per-request tool rule.** `createOpenAIChatAdapter.buildRequest`
(`src/adapters/openai-chat.ts:1441`) computes
`mapReasoningEffort(provider, parsed.modelId, parsed.options.reasoning)` with no
access to tool presence. The only model-scoped switch is `noReasoningModels`, which
is all-or-nothing: it strips reasoning from every request, so a model that accepts
effort on plain turns but rejects it alongside function tools has to give up its
effort ladder entirely. The wire `tools` array *is* already in scope at
`openai-chat.ts:1452`; nothing reads it for this decision.

**(b) Empty ladders poison the combo intersection.**
`deriveComboCatalogModel` (`src/codex/catalog/aggregation.ts:126`) filters only
`undefined` ladders as wildcards:

```
const advertisedLadders = members
.map(member => member.reasoningEfforts)
.filter((ladder): ladder is string[] => ladder !== undefined);
const reasoningEfforts = advertisedLadders.length === 0 ? [] : intersectStrings(advertisedLadders);
```

An explicit `[]` — meaning "this target has no effort control" — survives into
`intersectStrings` (`aggregation.ts:74`) and empties the result. The empty result
then flows into `effectiveComboDefault` (`aggregation.ts:170`), so the combo also
loses its default. One no-effort target silences the picker for every sibling that
does support tuning. `tests/codex-catalog.test.ts:250` pins this as current
intended behavior, which is why the fix must be opt-in rather than a correction.

## What PR #2734 gets right

Both halves default to today's behavior:

- `normalizeComboConfig` (`src/combos/types.ts:361`) maps anything that is not the
literal `"adaptive"` to `"strict"`, and the catalog branch only filters empty
ladders under `=== "adaptive"`.
- `omitReasoningEffortWithToolsModels` is optional, and `modelInList` returns
`false` for an absent or empty list.

A user who sets nothing observes an identical catalog and identical wire bodies.
That is the back-compat bar from the goal criteria, and the PR clears it on the
request path.

## Blocker: the GUI silently destroys the setting

`PUT /api/combos` replaces the stored combo wholesale
(`src/server/management/combo-routes.ts:170-172`: `nextCombos[id] = stored`).
The dashboard builds that body with an allowlist serializer, `toPutBody`
(`gui/src/combo-workspace-data.ts:496-527`), which enumerates `targets`,
`strategy`, `defaultEffort`, `imageInput`, `stickyLimit`, `alias`,
`nativeAlias`, `displayName` — and nothing else. `parseCombos`
(`combo-workspace-data.ts:222-232`) likewise never reads `reasoningEffortMode`.

So: a user hand-edits `reasoningEffortMode: "adaptive"`, later opens the dashboard
and renames the combo or reorders a target, and the save silently drops the field.
The picker they fixed goes empty again with no error and no diff they can see.
PR #2734 touches no `gui/` file, so it ships this hole.

This is the goal's UX criterion failing, not a nitpick: the opt-in is neither
discoverable nor durable.

## Second defect: the GUI has its own copy of the intersection

`intersectComboEfforts` (`gui/src/combo-workspace-data.ts:54`) reimplements the
same rule client-side and skips only `listed === undefined`. Even with the field
preserved, the dashboard's own effort dropdown stays empty under `adaptive`
because it never learns the mode. Fixing the server alone fixes the served
catalog and leaves the editor lying.

## Third: non-sparse persistence

`combo-routes.ts:139-152` destructures `alias`/`nativeAlias`/`displayName`/
`imageInput` out of the normalized object so defaults are not written, but
`reasoningEffortMode` stays in `normalizedBase`. Every combo save would therefore
stamp `"reasoningEffortMode": "strict"` into `config.json` for users who never
asked for the feature. Config churn on an untouched setting, against the file's
stated sparse convention.

## Plan

Adopt the PR's design — it is the right shape — and complete it in a
maintainer-authored branch that closes #2734 with credit.

### File change map

| File | Action | Change |
|------|--------|--------|
| `src/types/config.ts` | MODIFY | `OcxComboReasoningEffortMode`; `reasoningEffortMode?` on `OcxComboConfig` next to `defaultEffort` (:773) |
| `src/types.ts` | MODIFY | re-export the new type |
| `src/combos/types.ts` | MODIFY | field on `NormalizedComboConfig`; validate in `comboConfigIssues` (:175); normalize in `normalizeComboConfig` (:353) defaulting to `strict` |
| `src/codex/catalog/aggregation.ts` | MODIFY | under `adaptive`, drop zero-length ladders before `intersectStrings` |
| `src/adapters/openai-chat.ts` | MODIFY | `omitReasoningEffortWithTools` guard at :1428 and on the `gateway-object` branch |
| `src/types/provider.ts` | MODIFY | `omitReasoningEffortWithToolsModels?: string[]` beside `noStructuredOutputModels` (:506) |
| `src/config.ts` | MODIFY | zod schema (:516) + `nonBlankStringArrayConfigError` superRefine (:1216) |
| `src/server/auth-cors.ts` | MODIFY | `providerManagementConfigError` validation + `safeConfigDTO` exposure |
| `src/server/management/provider-routes.ts` | MODIFY | PATCH field handling + GET projection |
| `src/server/management/combo-routes.ts` | MODIFY | **NEW vs PR** — destructure `reasoningEffortMode` out of `normalizedBase`; persist only when `"adaptive"` |
| `gui/src/combo-workspace-data.ts` | MODIFY | **NEW vs PR** — `reasoningEffortMode` on `ComboItem`; read in `parseCombos`; emit in `toPutBody`; add to `baselineSyncKey` and the dirty comparison; teach `intersectComboEfforts` the mode |
| `gui/src/components/combo-workspace-detail-panel.tsx` | MODIFY | **NEW vs PR** — opt-in toggle directly under the Default-reasoning field |
| `gui/src/i18n/*.ts` | MODIFY | **NEW vs PR** — label + hint for all locales |
| `docs-site/.../combos.md`, `routing.md`, `providers.md` | MODIFY | as in the PR, minus the stale strategy list |
| `tests/codex-catalog.test.ts` | MODIFY | adaptive keeps sibling ladder; strict unchanged |
| `tests/combos.test.ts` | MODIFY | validation + normalization default |
| `tests/combo-management-api.test.ts` | MODIFY | round-trip; strict is NOT persisted |
| `tests/combo-workspace-data.test.ts` | MODIFY | `toPutBody` preserves the mode; GUI intersection honors it |
| `tests/openai-chat-hardening.test.ts` | MODIFY | tools present/absent, sibling model unaffected |

### UX decision

The control goes in the combo detail panel immediately below "Default reasoning",
because that is the field whose options the mode changes. Default state is the
current behavior. The hint says what turning it on does in one sentence, in the
user's terms: targets that have no reasoning control stop hiding the control for
the rest of the group.

No new top-level navigation, no new settings page. The concern already has a home.

### Scope boundary

IN: the two defects, GUI round-trip, sparse persistence, docs, focused tests.

OUT: `concreteComboRequestBody` per-target effort stripping (investigator concern
2) — that is a genuine gap but it is request-path routing behavior for combos
generally, not this issue's picker/wire problem, and folding it in here would
expand a +180 diff into cross-module routing work. Record it as follow-up.
OUT: an adapter-type guard on the provider key (concern 7) — it is a no-op outside
`openai-chat` today.

### Accept criteria

1. Unset config: catalog rows and wire bodies byte-identical to `e40245e4c`.
Activation: run the existing strict-mode assertions in
`tests/codex-catalog.test.ts` unchanged.
2. `adaptive` set: a combo with one `[]`-ladder member publishes the surviving
sibling intersection instead of `[]`. Activation: new case asserting a non-empty
ladder where the strict case asserts `[]`.
3. Tool-bearing request to a listed model omits `reasoning_effort`; the same model
without tools still sends it; an unlisted sibling always sends it. Activation:
three assertions in `tests/openai-chat-hardening.test.ts`.
4. Dashboard round-trip preserves `adaptive`. Activation: `toPutBody` on an item
parsed from a combo carrying the mode still contains it.
5. A strict combo saved through the API writes no `reasoningEffortMode` key.

### Verifier

`bun x tsc --noEmit` (whole-project, reads every file above) plus the five named
test files run individually by path. The full suite is forbidden by the operator.
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
# wp1 audit round 1 — synthesis

Reviewer: grok-4.6 adversarial lane (agent `01a05df6`).
Verdict: `GO-WITH-FIXES (blockers=3)`. All three accepted and folded. No rebuttals.

A first reviewer (`01a05de5`) produced nothing across four wait cycles and was
retired under DISPATCH-RETIRE-01; this is the replacement's round.

The reviewer independently confirmed the central blocker — `toPutBody` allowlist
plus wholesale `nextCombos[id] = stored` — so the plan's justification stands.

## Blocker 1 — GET echo, not just disk churn (ACCEPTED)

I planned to sparsify only the PUT persist destructure. The reviewer found
`sparseComboConfig` (`src/server/management/combo-routes.ts:71-78`), which the GET
list handler (`:84-91`) and the PUT response (`:248`) both run, and which today
strips exactly one default: `imageInput: "auto"`.

Since `getCombo` returns an already-normalized combo, every GET row would echo
`"reasoningEffortMode": "strict"` for users who never opted in. Worse, any client
that round-trips GET into PUT would then write that default straight back to disk,
defeating the persist-side fix entirely.

Correction: extend `sparseComboConfig` itself to drop `reasoningEffortMode: "strict"`,
which fixes GET, the PUT response, and the round-trip in one place. The persist-side
destructure becomes redundant — use the shared helper rather than two rules that can
drift.

## Blocker 2 — wrong file for the sync key, two GUI sites missing (ACCEPTED)

`baselineSyncKey` is at `gui/src/components/combo-workspace-detail-panel.tsx:90`,
not in `combo-workspace-data.ts` as my map said. Following the map as written would
ship a toggle whose draft never resyncs when only the mode changes.

Also missing: `emptyDraft` (`combo-workspace-data.ts:619-631`), `draftEquals`
(`:478-488`), and the create path — `combo-workspace-add-modal.tsx:50-52` calls
`intersectComboEfforts` with no mode, so a mixed group's picker still reads empty
while the user is creating the combo. Fixing only the detail panel leaves the
create flow lying at exactly the moment the user is assembling the mixed group
that motivates the feature.

## Blocker 3 — criteria that pass without the branch firing (ACCEPTED)

- Criterion 3 asserted only the plain `reasoning_effort` path, but the PR also
edits the `reasoningWireFormat === "gateway-object"` branch
(`src/adapters/openai-chat.ts:1498`). That branch could be left stale and the
criterion would still go green. Now requires a gateway-object provider assertion.
- Criterion 4 was a `toPutBody` unit assertion, which passes even if PUT still
drops the field and GET still echoes `strict`. Now requires the management
round-trip: PUT `adaptive` then GET and see it survive.
- Criterion 1 proved the catalog default, not wire byte-identity. Now split: a
catalog assertion and an adapter assertion that an unlisted model's body is
unchanged.

## UX correction (reviewer point 6 — accepted, changes the design)

I had placed the toggle under "Default reasoning". The reviewer's objection is
correct and it is not cosmetic: `defaultEffort` is a *value chosen from* the
intersection, whereas this is a *policy that changes what the intersection is* —
the same kind of thing as `imageInput`, which already lives in `ComboCapabilities`
as a switch (`gui/src/components/combo-workspace-controls.tsx:100-136`).

Placing a policy switch under a value dropdown invites the reading "this changes
my default effort", which is precisely what it does not do.

Revised: the toggle goes in `ComboCapabilities` beside the image-input switch,
where combo-wide capability policy already lives. That is also the component the
create modal and the detail panel share, so the create flow gets the control for
free — which is what blocker 2 requires anyway.

## Not folded

`buildOpenAIChatPassthroughRequest` (`openai-chat.ts:108-142`) ignores this key —
but it equally ignores `noReasoningModels` today, so it is a pre-existing passthrough
boundary, not a regression this unit introduces. Recorded, not fixed here.

Docs wording: the reviewer notes "unsupported effort fields are omitted" overclaims,
because Responses does not run `stripEmptyLadderEffort` (Chat-only,
`src/server/chat-completions.ts:191-196`). The docs sentence will be narrowed to
what is true rather than the feature being expanded.

Line drift: plan cited `openai-chat.ts:1428`; live reasoning block is `:1494-1498`.
32 changes: 32 additions & 0 deletions docs-site/src/content/docs/guides/combos.md
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,37 @@ default and leaves the target's own behavior unchanged. Supported values are `lo
`high`, `xhigh`, `max`, and `ultra`; omit the field or set it to `null` to leave effort entirely to
the caller and target.

### Mixed-capability groups (`reasoningEffortMode`)

The effort levels a combo advertises are the intersection of what its targets advertise. A target
that explicitly advertises **no** effort control takes part in that intersection, so a single
no-effort backup empties the effort picker for the whole combo — including for the targets that do
support tuning.

Set `reasoningEffortMode: "adaptive"` to exclude those empty ladders from the published
intersection instead. The picker then shows the levels the remaining targets share, and the
no-effort target stays eligible for routing. Targets whose ladder is simply *unknown* are treated
as wildcards in both modes.

```json
{
"combos": {
"mixed": {
"targets": [
{ "provider": "openai-apikey", "model": "gpt-5.6-luna" },
{ "provider": "local", "model": "no-effort-model" }
],
"reasoningEffortMode": "adaptive"
}
}
}
```

The default is `"strict"`, which keeps the original behavior. This setting changes published
catalog metadata only — it does not change target order, failover policy, or which effort a given
target receives at dispatch. In the dashboard it is the **Adaptive reasoning ladder** switch in a
combo's Capabilities section.

## Image / multimodal capability

By default a combo publishes the **intersection** of its targets' input modalities (image is
Expand Down Expand Up @@ -346,6 +377,7 @@ Combos are stored in the top-level `combos` object, keyed by combo id:
| `strategy` | No | `"failover"` | `"failover"`, `"round-robin"`, `"random"`, `"least-used"`, or `"reset-window"`. |
| `stickyLimit` | No | `1` | Integer from 1 to 100 successful requests per round-robin selection. Applies only to round-robin. |
| `defaultEffort` | No | `null` | `low`, `medium`, `high`, `xhigh`, `max`, or `ultra`; applied only when the caller omits effort and the target advertises support. |
| `reasoningEffortMode` | No | `"strict"` | `"strict"` intersects every known target ladder, so one target advertising no effort control empties the combo's picker. `"adaptive"` excludes those empty ladders from the published intersection. Metadata only; dispatch is unchanged. |
| `imageInput` | No | `"auto"` | `"auto"` or `"disabled"`. `"auto"` publishes image support only when every target supports images; `"disabled"` forces text-only (drops image from published modalities and rejects image-bearing requests before dispatch). |
| `alias` | No | none | Optional trimmed public model id; use the alias rules above. An empty value is stored as no alias. |
| `nativeAlias` | No | `false` | Explicitly permit a currently supported bare native `alias` to take routing and catalog precedence. Never inferred from the alias. |
Expand Down
Loading
Loading