Skip to content

fix(tui): stop double-formatting friendly tool labels (#62796) - #62816

Closed
PRATHAMESH75 wants to merge 2 commits into
NousResearch:mainfrom
PRATHAMESH75:fix/tui-double-format-friendly-labels
Closed

fix(tui): stop double-formatting friendly tool labels (#62796)#62816
PRATHAMESH75 wants to merge 2 commits into
NousResearch:mainfrom
PRATHAMESH75:fix/tui-double-format-friendly-labels

Conversation

@PRATHAMESH75

Copy link
Copy Markdown
Contributor

What does this PR do?

The TUI double-formats built-in tool rows: instead of Reading package.json L1-300 it shows Read File("Reading package.json L1-300") (and likewise Skill View("Reading skill …"), Web Extract("Reading …"), Search Files("Searching files for …")).

Since #55166, the gateway's _tool_ctx returned a complete friendly label under the wire's context field, but context is a contract for a raw argument preview that clients wrap themselves as Read File("…"). Ink dutifully wrapped the already-complete label a second time.

This restores context to a raw preview and ships the complete phrase in an additive label field. Clients render label verbatim when present and otherwise fall back to wrapping context — so pre-label gateways/clients, custom/plugin/MCP tools (no curated verb), disabled display.friendly_tool_labels, and the desktop's own localized rendering are all unaffected. This is the approach suggested by the reporter (@CNSeniorious000) in the issue.

Related Issue

Fixes #62796

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • tui_gateway/server.py: _tool_ctx now returns the raw preview (build_tool_preview) again; new _tool_label returns the complete friendly label only when it differs from that preview. Both the live tool.start payload and the resumed-history tool message now carry context (raw) plus an optional label.
  • ui-tui/src/lib/text.ts: formatToolCall(name, context, friendlyLabel?) renders friendlyLabel verbatim when present; buildToolTrailLine / buildVerboseToolTrailLine thread it through so the completed/resumed trail matches the live row.
  • ui-tui/src/app/turnController.ts, createGatewayEventHandler.ts, domain/messages.ts, types.ts, gatewayTypes.ts: carry the new label from the gateway event / transcript row to the render helpers.
  • ui-tui/src/components/thinking.tsx: render the live row from label; make the inline-subagent delegate-group detection accept both the wrapped Delegate Task(…) and the friendly Delegating … phrasings.
  • Tests: composition coverage the issue called out as missing — a Python test that runs _on_tool_start and asserts context stays a bare preview while label carries the complete phrase (and is omitted for unlabeled tools), plus TS tests for formatToolCall, the trail line, and resumed-transcript rendering. Updated test_history_to_messages_preserves_tool_calls_for_resume_display to the corrected contract.

How to Test

scripts/run_tests.sh tests/test_tui_gateway_server.py -q

Result: 319 passed (includes the two new composition tests).

TS (from ui-tui/, after npm install + npm run build --prefix packages/hermes-ink):

npx vitest run src/__tests__/text.test.ts src/__tests__/messages.test.ts src/__tests__/createGatewayEventHandler.test.ts

Result: all pass. (Pre-existing, unrelated failures in statusRule.test.ts / virtualHeights.test.ts reproduce on clean upstream/main.)

Manual: with display.friendly_tool_labels: true (default), run hermes --tui, trigger read_file, and observe the live row, completion trail, and a resumed transcript all show Reading package.json L1-300 — not Read File("Reading package.json L1-300").

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix
  • I've run the affected tests and they pass
  • I've added tests for my changes
  • I've tested on my platform: macOS 15 (Darwin 25.5)

Documentation & Housekeeping

  • I've updated relevant documentation (docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — N/A (no config keys)
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
  • I've considered cross-platform impact (Windows, macOS) — N/A (display formatting only)
  • I've updated tool descriptions/schemas if I changed tool behavior — N/A

Credits

Root cause and the context + additive label fix strategy were diagnosed by @CNSeniorious000 in #62796; this PR implements that approach.

Since NousResearch#55166 the gateway's `_tool_ctx` returned a complete friendly
label ("Reading package.json L1-300") under the `context` field, which
Ink then wrapped again via `formatToolCall`, producing
`Read File("Reading package.json L1-300")` on live, completed-trail, and
resumed rows.

Restore `context` to a raw argument preview and ship the complete phrase
in an additive `label` field. Ink renders `label` verbatim when present
and otherwise falls back to wrapping `context`, so pre-`label` clients,
custom/MCP tools, disabled friendly labels, and desktop rendering are all
unaffected. Fixes NousResearch#62796.
@alt-glitch alt-glitch added type/bug Something isn't working comp/tui Terminal UI (ui-tui/ + tui_gateway/) P2 Medium — degraded but workaround exists duplicate This issue or pull request already exists labels Jul 11, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #62809 — twin fresh PR fixing the same issue (#62796) via the same mechanism (split the raw context preview from the complete friendly label across tui_gateway/server.py and ui-tui/*). #62809 was opened ~8 minutes earlier (2026-07-11T18:58Z vs 19:06Z) and is the canonical entry; the two diffs cover an identical file set and approach. Regression source is the merged #55166. Maintainer picks which to merge.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for tracing the regression through both the gateway and Ink rendering paths. The premise is confirmed on current main: tui_gateway/server.py:3438-3442 sends build_tool_label() as context, while ui-tui/src/lib/text.ts:198-202 wraps every nonempty context.

Problems

  • ui-tui/README.md:283 documents the tool.start payload but does not include the new optional label field, leaving the protocol contract incomplete.

Suggested changes

  • Document label? in the tool.start event table as the complete friendly label rendered verbatim, while context? remains the raw preview.

Automated hermes-sweeper review.

The tool.start payload now carries an optional label (the complete
friendly label rendered verbatim) distinct from the raw context preview.
Document it in the protocol event table so the contract is complete.
@PRATHAMESH75

Copy link
Copy Markdown
Contributor Author

Addressed the review: documented the optional label field in the tool.start event table in ui-tui/README.mdlabel? is the complete friendly label rendered verbatim, context? remains the raw preview.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 11, 2026
@alt-glitch alt-glitch removed the sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades label Jul 11, 2026
@teknium1 teknium1 added the sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades label Jul 11, 2026
@alt-glitch alt-glitch removed the sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades label Jul 12, 2026
@teknium1 teknium1 added the sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades label Jul 12, 2026
@PRATHAMESH75

Copy link
Copy Markdown
Contributor Author

Heads-up on the current CONFLICTING state: this can't be mechanically rebased onto upstream/main, and it needs a design decision rather than a merge fixup.

Since this PR was opened, upstream reworked the TranscriptRow transcript model: it added a display_kind field plus a display_metadata object (with task_count etc.) and threads them through the renderer. This PR's fix instead introduced a label field on the same type and destructuring site (ui-tui/src/domain/messages.ts, tui_gateway/server.py), so the two approaches now collide directly — the row schema, the destructuring, and the server-side row builder all conflict on both sides.

I'm intentionally not force-resolving this, because it isn't additive: naively keeping both fields would leave two parallel label paths, and it's a real question whether the new display_kind rendering already resolves the double-formatting this PR targeted (it may — the renderer now owns friendly-label derivation). That's an author/maintainer call.

Suggested next step for whoever picks this up: check whether double-formatting still reproduces on current upstream/main given the display_kind renderer. If it does, re-port the fix in terms of display_kind/display_metadata rather than a separate label field; if it doesn't, this can be closed as superseded. Happy to do either once the direction is clear.

@alt-glitch alt-glitch added needs-decision Awaiting maintainer decision before any implementation and removed duplicate This issue or pull request already exists labels Jul 29, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Correction: the prior duplicate conclusion is cleared. The current head added the requested protocol documentation, but it now conflicts with main's TranscriptRow/display schema rework. It is related to #62809, yet a maintainer must decide whether the current renderer already resolves the bug or whether to re-port the fix via display_kind/display_metadata.

@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Correction: current heads share the #62796 event-contract repair but have distinct regression coverage, and maintainer review says #62816 should remain open. Treating it as related to #62809, not a duplicate.

@PRATHAMESH75

Copy link
Copy Markdown
Contributor Author

Following up on the open question from the previous note — I checked, and the double-formatting is now fixed on upstream/main, so I'm closing this as superseded.

The fix landed in f0031ab ("fix(gateway): send a raw arg preview on tool.start, not a phrased label", by @OutThisLife). _tool_ctx had switched to build_tool_label in #55166, so every tool.start carried an already-phrased string that both clients then re-verbed. Upstream fixed it at the source: the gateway now emits the raw arg preview, and tui_gateway/server.py explicitly documents that using build_tool_label there "stutters the verb." Because the fix is gateway-side it covers both the TUI and the desktop row ("Ran Running …"), whereas this PR only touched the TUI path — strictly narrower than what already landed.

Verified f0031ab is on main and no phrased label is emitted on tool.start; nothing to re-port. Closing in favour of it. Thanks @OutThisLife.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tui Terminal UI (ui-tui/ + tui_gateway/) needs-decision Awaiting maintainer decision before any implementation P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: TUI double-formats friendly tool labels

3 participants