Skip to content

feat(desktop): collapse thinking by default - #69217

Closed
isak-ialogics wants to merge 1 commit into
NousResearch:mainfrom
isak-ialogics:feat/69161-collapse-reasoning-default
Closed

feat(desktop): collapse thinking by default#69217
isak-ialogics wants to merge 1 commit into
NousResearch:mainfrom
isak-ialogics:feat/69161-collapse-reasoning-default

Conversation

@isak-ialogics

Copy link
Copy Markdown
Contributor

Summary

  • adds a persisted, Desktop-local Appearance preference for default-collapsed thinking blocks
  • keeps streamed reasoning accessible on demand and preserves per-block manual expansion
  • localizes the setting across supported Desktop locales

Validation

  • npx vitest run src/components/assistant-ui/thread/streaming.test.tsx
  • npm run typecheck
  • npm run lint (passes with 9 pre-existing warnings)

Note

npm test was also attempted. It currently has unrelated Windows/host-environment failures (14 files / 29 tests), including POSIX path and SSH control-socket expectations; the focused affected suite passes.

Closes #69161

@eduardoejp

Copy link
Copy Markdown

Haha! 😆

I set my Hermes agent to fix it and make a pull request.
Then I went to make dinner, while it worked it out.
As soon as I come back, you've got a PR ready. 😛

You guys don't sleep.

Thank you for the effort.
I've been using Hermes for almost a week now and I'm falling in love with it.
Maybe some time in the future I'll be able to contribute a patch as a way of saying "thank you".

@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/desktop Electron desktop app (apps/desktop/*) area/config Config system, migrations, profiles labels Jul 22, 2026
@eduardoejp

Copy link
Copy Markdown

Amicus here; eduardoejp's Hermes agent 👋

Reviewed this properly and it's the right design. Keeping the preference Desktop-local (your store/reasoning-disclosure.ts, persisted to localStorage) instead of folding it into the gateway display config was the correct call — we'd initially taken the backend-config route and it was the wrong one; it would've leaked a per-platform display key into Telegram/Discord/Slack that don't even render that component.

The message-parts.tsx two-liner (userOpen ?? (pending && !collapsed)) is exactly the minimal surface, and click-to-expand still holds. We'd independently landed on the same lines, so this is well-targeted. LGTM from our corner — thanks for picking this up.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused Desktop-local approach. Current main still has the reported behavior: ThinkingDisclosure uses const open = userOpen ?? pending at apps/desktop/src/components/assistant-ui/thread/message-parts.tsx:91, so an untouched streaming reasoning block opens by default. The PR's local presentation store fits the Desktop authority boundary in apps/desktop/AGENTS.md:34-45 and preserves manual expansion.

Problems

  • Current main now supports Arabic (apps/desktop/src/i18n/types.ts:8; apps/desktop/src/i18n/catalog.ts:8-13), but this diff adds the new Appearance strings only to en/ja/zh/zh-hant. defineLocale() falls back to English for omitted keys (apps/desktop/src/i18n/define-locale.ts:39-40), so Arabic users would receive English copy for this setting.

Suggested changes

  • Add Arabic overrides for the two new Appearance strings when salvaging onto current main.
  • Reapply the small disclosure-default change against current ThinkingDisclosure while retaining its newer duration and preview-performance logic (apps/desktop/src/components/assistant-ui/thread/message-parts.tsx:78-149).

Automated hermes-sweeper review.

@teknium1 teknium1 added the sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users label Jul 30, 2026
@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Summary

One PR addresses this issue complex: #69217 directly implements #69161’s visible-but-collapsed default, while its complete diff does not implement #71870’s response-only mode because reasoning disclosures remain rendered and the new preference is separate from display.show_reasoning.

Related pull requests

Suggested consolidation

Keep #69217 open with a salvage path for #69161: rebase the focused disclosure-default change onto current main, preserve the newer duration and preview-performance logic, and add the missing Arabic translations. Track #71870 separately because #69217 intentionally keeps reasoning chrome available rather than wiring display.show_reasoning to hide it.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I69161(["issue #69161 (open)"])
    P69217["PR #69217 (open)"]
    P69217 -->|best fix| I69161
    class I69161 open
    class P69217 open
    class P69217 best
    class P69217 target
    click I69161 "https://github.com/NousResearch/hermes-agent/issues/69161"
    click P69217 "https://github.com/NousResearch/hermes-agent/pull/69217"
Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 1 pull request and 2 issues in this complex. Each diff was read against this issue; Assessment working set: 11 kB of PR diffs, 8 kB of issue/PR text, 4 kB of discussion (7 comments), 2 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

@isak-ialogics
isak-ialogics force-pushed the feat/69161-collapse-reasoning-default branch from 6991ba9 to 14fd1aa Compare August 5, 2026 09:36
@isak-ialogics

Copy link
Copy Markdown
Contributor Author

Refreshed against current main (14fd1aa):\n\n- rebased the disclosure-default change onto the newer ThinkingDisclosure duration/preview implementation;\n- preserved per-block manual expansion as the lifecycle authority;\n- added Arabic overrides for the two new Appearance strings;\n- added a runtime i18n regression proving Arabic does not fall back to English.\n\nVerified locally: focused streaming + i18n tests (23/23), Desktop typecheck, and ESLint (0 errors; existing warnings only). npm run check still has 12 unrelated existing UI-suite failures on this Windows host, none in changed files. The branch is current with upstream/main and GitHub reports it mergeable.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for this fix! It was salvaged into #86771 (cherry-picked onto current main with your authorship preserved in the commit history) and is now merged. Closing since the work has landed.

@teknium1 teknium1 closed this Aug 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]: Add "collapse thinking/reasoning blocks by default" display setting (Desktop)

5 participants