Skip to content

feat(agent): show backend-selected thought level - #3734

Open
itsklimov wants to merge 2 commits into
iOfficeAI:mainfrom
itsklimov:feat/thought-level-indication
Open

feat(agent): show backend-selected thought level#3734
itsklimov wants to merge 2 commits into
iOfficeAI:mainfrom
itsklimov:feat/thought-level-indication

Conversation

@itsklimov

@itsklimov itsklimov commented Jul 24, 2026

Copy link
Copy Markdown

Pull Request

Description

Make thought-level indicators and selectors reflect the backend's reported current_value instead of inferring that the first available option is selected.

This keeps conversation and guide selectors honest when the backend is still resolving a default, preserves known current values across empty snapshot refreshes, and avoids sending an implicit thought-level override from an untouched guide picker. The selector falls back to the existing localized “Default” label until the backend reports an active value.

Related Issues

Type of Change

  • fix — Bug fix (non-breaking change which fixes an issue)
  • feat — New feature (non-breaking change which adds functionality)
  • perf — Performance improvement
  • refactor — Code restructuring (no behavior change)
  • Breaking change (fix or feature that would break existing functionality)
  • docs — Documentation update

Atomic PR Checklist (Rule 1)

  • This PR contains exactly one feature or bug fix that cannot be further decomposed
  • The PR title follows Conventional Commit format: <type>(<scope>): <subject> (English)

Local Checks (Rule 3)

  • bun run format:check — formatting passes
  • bun run lint -- --quiet — no lint errors
  • bunx tsc --noEmit — no type errors
  • bunx vitest run — 2,593 tests passed; 5 skipped
  • i18n validated (bun run i18n:types + node scripts/check-i18n.js)
  • New/changed user-facing text uses i18n keys (reuses common.default)

Runtime Verification

  • Verified on macOS
  • Verified on Windows
  • Verified on Linux
  • I have performed a self-review of my own code

Screenshots

Not applicable; this changes selection-state accuracy without introducing new UI structure.

Additional Context

The full pre-push gate completed successfully from a clean worktree with dependencies installed from the lockfile. Existing translation-completeness warnings remain for es-ES, fr-FR, and fa-IR; validation passes and this PR adds no translation keys.

…alue

The thinking-level pill and submenus now render ONLY what the backend
reports as active. The single source of truth is the config option's
current_value (which the Core seeds from the assistant default and keeps
reconciled); the UI no longer invents a selection:

- getCurrentThoughtLevelLabel/composeRuntimeSelectorLabel accept a
  localized default label and render 'model · Default' when the thought
  axis exists but no current is known — never options[0], which may not
  be what the backend actually runs.
- GuidModelSelector drops its options[0] normalization, and
  useGuidAssistantSelection no longer pre-seeds the selection: an
  untouched picker sends NO thought_level override at create, so the
  backend resolves the real default (the old implicit override silently
  defeated assistant fixed defaults).
- useAcpConfigOptions gains an anti-flicker merge: a whole-snapshot
  replace whose options carry no current information at all preserves
  known non-null currents (matched by category then id, only when still
  selectable); a frame with any non-null current stays authoritative, so
  the Core's reject re-push still clears a refused level.
- Thought-level matching falls back to ids effort/thinking/
  thinking_budget (category thought_level stays priority; 'thinking'
  keeps this compatible with upstream PR iOfficeAI#3597).

Reuses the existing common.default i18n key (present in all locales).
@itsklimov
itsklimov marked this pull request as ready for review July 24, 2026 21:54
@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 86.20690% with 4 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...op/src/renderer/hooks/agent/useAcpConfigOptions.ts 85.71% 0 Missing and 2 partials ⚠️
...kages/desktop/src/renderer/pages/guid/GuidPage.tsx 0.00% 1 Missing ⚠️
...erer/pages/guid/hooks/useGuidAssistantSelection.ts 0.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@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: 6be5085e10

ℹ️ 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 on lines +117 to +118
const informed = next.some((option) => option.current_value != null);
if (informed) return next;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve authoritative all-null snapshot updates

When a backend intentionally clears the only exposed config option—or clears every option at once—its snapshot contains only null current_value fields. This branch classifies that authoritative update as informationless and restores every still-selectable previous value, leaving the selector showing stale model or thought-level state. A non-null sibling is not a reliable indicator of whether nulls are authoritative; preservation needs to distinguish omitted/partial data from explicit nulls rather than treating every all-null snapshot as empty information.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This is an intentional anti-flicker tradeoff, and the distinction you're describing isn't representable at the current DTO level.

mergeSnapshotPreservingKnownCurrents only preserves previous currents for a frame where every option's current_value is null. Telling an authoritative all-null snapshot (a backend deliberately clearing every exposed option) apart from an informationless one (an early catalog push before currents land, or an older Core that never stamps currents) requires signaling omitted vs explicit null — but AcpConfigOptionDto carries current_value: null as the single representation for both cases. There is no field today that marks a null as authoritative.

Given that, preferring preservation for all-null frames is the safer default: it avoids the far more common flash-to-Model-only regression, while any frame carrying at least one non-null current stays authoritative — so the Core's reject re-push still wipes a refused level (the corrected frame keeps the model current, so its effort null passes through). The deliberate clear-everything-at-once case needs a DTO change to distinguish explicit nulls, which is out of scope for this PR. Leaving this thread open to track that as a follow-up rather than expanding scope here.

…override

The defaults effect fell back to `options[0]` when an assistant had no
resolved thought-level default and the backend reported no current value.
That pre-selected a concrete level which the send path then serialized as
an explicit `thought_level`, silently defeating the assistant's backend
default — the same implicit override useGuidAssistantSelection already
stopped emitting.

Drop the `options[0]` fallback so an untouched picker stays empty (`''`)
unless the backend actually reports a current value; the send path then
omits `thought_level` and the backend resolves the real default.

Add an end-to-end send-path regression test asserting that an untouched
(empty) selection serializes to `thought_level: undefined` in the
conversation create request.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant