Skip to content

fix(acp): pick the reasoning-effort ladder from the selected model - #286

Open
Astro-Han wants to merge 3 commits into
LodyAI:mainfrom
Astro-Han:fix/149-per-model-effort-ladder
Open

fix(acp): pick the reasoning-effort ladder from the selected model#286
Astro-Han wants to merge 3 commits into
LodyAI:mainfrom
Astro-Han:fix/149-per-model-effort-ladder

Conversation

@Astro-Han

@Astro-Han Astro-Han commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Related issue

Closes #149

Problem / pressure

The reasoning-effort picker showed one flat list — the ladder measured for whatever model was current at probe time — for every model. Selecting Grok 4.6 while the probe had run under a model with a high/low/max ladder hid medium/xhigh and offered max, which xAI rejects with 400 Invalid reasoning effort. Grok 4.5 and 4.6 differ the same way (xhigh only on 4.6). The Grok adapter kept one flat ladder and never rebuilt it on model change, and nothing carried a model-independent view to the UI.

Summary

  • Grok adapter (fix: follow the selected model's reasoning-effort ladder acp-extension-grok#11): rebuilds the ladder from the selected model on every change, translates the runtime model_changed notification into config_option_update, and publishes the per-model view as _meta.lody.modelReasoningEfforts on the session response. The root gitlink is bumped after that PR merges, per the runtime-upgrade convention from chore: upgrade built-in AI agent runtimes #201.
  • CLI: capability normalization reads that map into the capability cache, merged with the existing legacy model[effort] derivation — now scoped to builtin Codex, because Claude uses the same brackets for context windows (opus[1m]). ACP_CAPABILITY_CACHE_VERSION goes to 7, because an entry probed before that guard stored a bogus ladder for every agent that spells other variants with the same brackets (a Claude probe left { opus: ['1m'] }), and the picker would now rebuild that model's ladder from it. summarizeAgentRunConfigCapabilities and resolveAgentRunConfigSelection already consume the map, so MCP options and dispatch validation pick it up unchanged.
  • UI: normalizeReasoningEffortSelectors rebuilds the thought-level selector from the selected model's ladder, in the selector builders and again in the selection resolution so it follows an authoritatively resolved model. Applying a whole saved configuration (a recent run config or a Role) that also moves the model passes its effort through unvalidated, because the selectors still describe the outgoing model; the selection resolution re-validates it against the resolved one. It is a fallback, never an override — Codex's hand-maintained extended tiers and Claude's adapter-side rebuild keep their own paths, and a model the map does not cover keeps the probe-time list, since the adapter owns the wire and rejects an unsupported effort with a visible warning.

Before / after

Before After
Picker shows the probed model's ladder for every model Picker shows the selected model's ladder
Grok 4.6 picker under a foreign probe: high/low/max xhigh/high/medium/low
Effort validated against the probed model's ladder Validated against the selected model's ladder
MCP lody_session_create_options reports one flat reasoningEffortValues Adds per-model models[].reasoningEffortValues

Test plan

  • packages/acp-extension-grok: npm test — 39 passing (model-switch rebuild, effort validation against the selected model, published meta).
  • apps/cli: npx vitest run tests/acp-capability-normalization.test.ts src/mcp/lody-mcp-server.test.ts src/commands/session.test.ts — passing.
  • packages/components: npx vitest run tests/acp-selector-options.test.ts tests/acp-session-config-selection.test.ts tests/session-config-selection-oscillation.test.tsx tests/agent-role-form.test.ts tests/recent-run-configs.test.ts — passing (the two localStorage failures in recent-run-configs reproduce on a clean checkout). packages/shared: full run, 995 passing.
  • Repo-wide pnpm typecheck, pnpm lint, pnpm lint:i18n, check:code-collab-imports, check:platform-boundaries, check:public-boundary — pass. Full test:ci was not run; the known environment-related component failures reproduce on a clean checkout without this change.

Context handoff

Instructions for reviewing agents

  • Review focus: normalizeReasoningEffortSelectors in packages/components/src/components/shared/acp-selector-options.ts, and the zod-scoped _meta.lody read plus Codex-only legacy merge in apps/cli/src/agent/acp-capability-normalization.ts.
  • Decisions to challenge: the map is fallback-only, because Codex and Claude already adapt the ladder per model themselves and must not be overridden; the cache-version bump, which drops every entry (builtins fall back to their static tables, registry agents to no selectors) until the next probe; a model absent from the map keeps the probe-time list instead of losing the control.
  • Plausible failures / evidence gaps: the runtime half ships only once grok#11 merges and the gitlink follows; the _meta.lody.modelReasoningEfforts contract is documented in AGENTS.md rather than declared in acp-extension-core — say the word if you prefer it in core.

Authoring context

  • User goal / directives: fix [Bug] Grok 4.6 reasoning effort picker is missing Medium (uses probed model's ladder) #149 as agreed with the maintainer on the issue — take the ladder from the selected model, not the probed one — keeping the change small and preferring deletions.
  • Constraints / non-goals: Codex and Claude keep their existing per-model handling; vendor model _meta must not leak into Lody business code; the adapter's startup sessionConfig translation is untouched.
  • Risk-bearing decisions: an effort invalid for the selected model falls back to the neutral medium rather than the ladder's first (most expensive) entry; an uncovered model keeps the probe-time list and relies on the adapter's rejection warning.
  • Destructive or irreversible behavior: the cache-version bump discards cached probe results; they are re-derived by the next probe and no user data is touched.
  • Deliberately not done or tested: UI-side withdrawal of the control for uncovered models was tried during review and rolled back (it needed a four-branch rule plus a measured-model pipeline to duplicate a guard the adapter already owns); registry/custom agents select models through configOptionValues, which the resolver does not read, and none publishes the map today; no end-to-end Grok run on a real machine.
  • Unknowns / confidence: high for the cache and picker layers, unit-tested at each seam with fixtures from the issue; the adapter half depends on grok#11 merging first.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 2, 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-05T14:17:42.826231Z 510fe86 New commits
ℹ️ 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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 36e6a05e1f

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/cli/src/agent/acp-capability-normalization.ts Outdated
@Astro-Han
Astro-Han force-pushed the fix/149-per-model-effort-ladder branch from 36e6a05 to 4242863 Compare September 2, 2026 03:56

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4242863562

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/components/src/lib/acp-session-config-selection.ts
Comment thread packages/components/src/lib/acp-session-config-selection.ts
@Astro-Han
Astro-Han force-pushed the fix/149-per-model-effort-ladder branch from 4242863 to 97dd43d Compare September 2, 2026 04:02

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 97dd43d578

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/components/src/components/shared/acp-selector-options.ts Outdated
@Astro-Han
Astro-Han force-pushed the fix/149-per-model-effort-ladder branch from 97dd43d to ca3b71f Compare September 2, 2026 04:08

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ca3b71f362

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/components/src/components/shared/acp-selector-options.ts Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 65bb1807a2

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/components/src/lib/acp-session-config-selection.ts
@Astro-Han
Astro-Han force-pushed the fix/149-per-model-effort-ladder branch from 65bb180 to 8604664 Compare September 2, 2026 05:03

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 860466473f

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/components/src/components/shared/acp-selector-options.ts
@Astro-Han
Astro-Han force-pushed the fix/149-per-model-effort-ladder branch from 8604664 to eae92ca Compare September 2, 2026 05:18

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: eae92cad85

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/components/src/components/shared/acp-selector-options.ts Outdated
@Astro-Han
Astro-Han force-pushed the fix/149-per-model-effort-ladder branch from eae92ca to 3d6d68b Compare September 2, 2026 05:23

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3d6d68b317

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/components/src/components/chat/chat-landing.tsx
@Astro-Han
Astro-Han force-pushed the fix/149-per-model-effort-ladder branch from 3d6d68b to 4e66a56 Compare September 2, 2026 08:11

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4e66a561f3

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/components/src/lib/acp-session-config-selection.ts
Comment thread packages/components/src/components/shared/acp-selector-options.ts
@Astro-Han
Astro-Han force-pushed the fix/149-per-model-effort-ladder branch 3 times, most recently from 2ffb88f to 8a400be Compare September 2, 2026 08:29

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8a400be590

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/components/src/components/shared/acp-selector-options.ts Outdated
@Astro-Han
Astro-Han force-pushed the fix/149-per-model-effort-ladder branch from 8a400be to 47e3335 Compare September 2, 2026 09:22

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 47e3335117

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/components/src/components/shared/acp-selector-options.ts
@Astro-Han

Copy link
Copy Markdown
Contributor Author

@Leeeon233 The PR is ready for review, thanks!

@Astro-Han
Astro-Han force-pushed the fix/149-per-model-effort-ladder branch from 9cba5f6 to 0902fc2 Compare September 5, 2026 10:11
@github-actions github-actions Bot added status:needs-pr-attention External PR needs contributor attention before review and removed status:needs-pr-attention External PR needs contributor attention before review labels Sep 5, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0902fc2ee8

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/cli/src/agent/acp-capability-normalization.ts
Comment thread packages/components/src/components/shared/acp-selector-options.ts
@Astro-Han
Astro-Han force-pushed the fix/149-per-model-effort-ladder branch from 0902fc2 to 3088326 Compare September 5, 2026 14:03
The capability cache treated the probed model's thought-level list as
global. An adapter can now publish each model's reasoning-effort ladder
under the Lody-owned "_meta.lody.modelReasoningEfforts" namespace on the
session response (the Grok adapter does), and the capability
normalization merges that map with the existing legacy
"model[effort]" derivation into modelReasoningEfforts.

No cache-version bump: the startup capability refresh re-probes every
configured agent unconditionally and every session rewrites its entry,
so existing caches pick up the map without a global invalidation that
would hide registry/custom selectors until the refresh completes.

"summarizeAgentRunConfigCapabilities" and
"resolveAgentRunConfigSelection" already consume the map, so MCP callers
and dispatch validation get per-model efforts without further change.

Ref: LodyAI#149

Model: glm-5.3
@Astro-Han
Astro-Han force-pushed the fix/149-per-model-effort-ladder branch from 3088326 to 5aaddd3 Compare September 5, 2026 14:06

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5aaddd3053

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread apps/cli/src/agent/acp-capability-normalization.ts
The composer's reasoning-effort picker rendered whatever thought-level
list the capability probe captured, no matter which model was selected:
selecting Grok 4.6 while the probe ran under a model with a
high/low/max ladder hid "medium" and offered the invalid "max"
(LodyAI#149).

A new normalizeReasoningEffortSelectors rebuilds the thought-level
selector from the per-model ladder the capability source publishes
(cached runtime map or static builtin table), reusing probe-time labels
and descriptions for values the ladder still offers and synthesizing
labels for the rest. It runs in both the selector builders and the
selection resolution, so a model moved by authoritative validation
re-normalizes against the RESOLVED model, and it skips Codex, which
keeps its own hardcoded extended-effort table for now.

The map is a required field of the selection input, so the session,
draft, and Role composer call sites cannot drop it silently; the Role
editor passes its pinned model so its picker and compatibility check
agree on the same ladder.

The static builtin Grok table gains per-model ladders so the picker is
correct before any probe, and resolveConfigOptions now surfaces the
cached per-model map alongside configOptions.

Model: glm-5.3
@Astro-Han
Astro-Han force-pushed the fix/149-per-model-effort-ladder branch from 5aaddd3 to 510fe86 Compare September 5, 2026 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Grok 4.6 reasoning effort picker is missing Medium (uses probed model's ladder)

1 participant