feat(webui): redesign tool-call rendering with inline diffs, group summaries, and cheap-model purpose titles#103748
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c9fa76403
ℹ️ 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".
| // allowed only when the agent already sends its turns to OpenAI. Tool args | ||
| // must not reach a new provider just for decorative titles. | ||
| const useUtility = hasConfiguredUtilityModel(params.cfg, params.agentId); | ||
| if (!useUtility && !agentPrimaryUsesOpenAI(params.cfg, params.agentId)) { |
There was a problem hiding this comment.
Gate Luna titles on the session model
When a chat session has a per-session model override away from OpenAI, this egress check still consults only the agent's configured primary model. The existing chat paths resolve the effective session provider from the loaded session entry via resolveSessionModelRef before sending a turn, but chat.toolTitles never loads that entry; with an OpenAI-default agent and an Anthropic-overridden session, cache misses still send tool names/inputs to openai/gpt-5.6-luna for decorative titles even though that session is not using OpenAI and no utilityModel was explicitly configured.
Useful? React with 👍 / 👎.
| // Args already represented in the collapsed row / header detail for kinds that | ||
| // summarize their primary target; everything else stays auditable on expand. | ||
| const ROW_SUMMARIZED_ARG_KEYS: Partial<Record<ToolCallView["kind"], ReadonlySet<string>>> = { | ||
| read: new Set(["path", "file_path", "filePath", "notebook_path", "offset", "limit"]), |
There was a problem hiding this comment.
Keep read slice arguments visible
For read tools that include offset or limit, these keys are treated as already summarized, but the collapsed/read row only renders the file target and directory. Because extraArgsBeyondRowTarget drops all keys in this set, expanding a sliced read hides whether the agent read the full file or only a window, which removes important audit context for partial read_file/notebook reads.
Useful? React with 👍 / 👎.
|
Codex review: found issues before merge. Reviewed July 10, 2026, 11:53 AM ET / 15:53 UTC. Summary PR surface: Source +2259, Tests +966, Docs +2. Total +3227 across 35 files. Reproducibility: yes. An OpenAI-default agent with a non-OpenAI session override follows the provider-mismatch path, and a large Review metrics: 2 noteworthy metrics.
Stored data model Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review findings
Review detailsBest possible solution: Keep the demonstrated deterministic rendering and pairing improvements, derive implicit Luna eligibility from the loaded session's effective model, impose one aggregate computation and rendered-row budget across multi-edit fallback diffs, and document the approved explicit-utility-model egress policy. Do we have a high-confidence way to reproduce the issue? Yes. An OpenAI-default agent with a non-OpenAI session override follows the provider-mismatch path, and a large Is this the best way to solve the issue? No, not yet. The UX and deterministic pairing approach are strong, but the maintainable solution must respect effective session routing and enforce one aggregate browser-work limit. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 4d004e2054ba. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +2259, Tests +966, Docs +2. Total +3227 across 35 files. View PR surface stats
Security concerns:
Acceptance criteria:
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (1 earlier review cycle)
|
9c9fa76 to
bb4a9ce
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bb4a9ce8e9
ℹ️ 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".
| const sections = pairs.map((pair) => computeLineDiff(pair.oldText, pair.newText)); | ||
| const joined = joinDiffSections(sections); |
There was a problem hiding this comment.
Bound multi-edit diff work across all edits
When a multi_edit/edits payload contains many replacements, this maps every pair through the quadratic LCS diff and then joinDiffSections keeps up to 400 rows per edit, so a large but valid transcript can allocate millions of cells and mount thousands of diff rows during chat rendering. The existing caps only apply per replacement; cap the number of pairs or the total diff rows before computing/rendering the fallback diff.
Useful? React with 👍 / 👎.
…mmaries, and cheap-model purpose titles Kind-aware tool rows (terminal-style commands with wrapper stripping and display highlighting, file edits with inline numbered diffs and diffstat, write previews, key-value args), aggregate group summaries with live run status, and a new batched chat.toolTitles gateway RPC that titles complex calls via the configured utilityModel or the OpenAI Luna default (gated to OpenAI-primary agents, cached in the per-agent SQLite cache_entries). Also fixes two transcript pairing bugs: result blocks now inherit call id/name/details at merge time, and results pair with any open call in the current tool run so parallel calls render as single rows. Fixes #103554
bb4a9ce to
19a4322
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 19a43225fd
ℹ️ 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".
| } | ||
| if (changed) { | ||
| titlesVersion += 1; | ||
| head.notify?.(); |
There was a problem hiding this comment.
Notify every pane in a title batch
In split-pane views of the same session/agent, queued items from different panes are batched together because the grouping only compares client, sessionKey, and agentId, but this only calls the first item's notify callback. Titles generated for the other pane(s) update the global store and titlesVersion, yet those panes do not request a render, so their rows stay on deterministic labels until some unrelated UI update happens.
Useful? React with 👍 / 👎.
| titlesByKey.set(item.key, title); | ||
| changed = true; | ||
| } else { | ||
| failedKeys.add(item.key); |
There was a problem hiding this comment.
Scope title misses to request context
When a title request returns no title for an item, this records the digest in a process-global failedKeys set even though misses are context-dependent: for example, the first pane/session may have no eligible cheap model, while another agent/session later has a utility model or OpenAI primary for the exact same command. Because scheduleTitleRequest suppresses any key in failedKeys, that later eligible context never asks the gateway and the title is permanently disabled for the rest of the tab.
Useful? React with 👍 / 👎.
|
Merged via squash.
|
…mmaries, and cheap-model purpose titles (openclaw#103748) Kind-aware tool rows (terminal-style commands with wrapper stripping and display highlighting, file edits with inline numbered diffs and diffstat, write previews, key-value args), aggregate group summaries with live run status, and a new batched chat.toolTitles gateway RPC that titles complex calls via the configured utilityModel or the OpenAI Luna default (gated to OpenAI-primary agents, cached in the per-agent SQLite cache_entries). Also fixes two transcript pairing bugs: result blocks now inherit call id/name/details at merge time, and results pair with any open call in the current tool run so parallel calls render as single rows. Fixes openclaw#103554
What Problem This Solves
Fixes #103554.
Tool calls in the Control UI chat were opaque: groups collapsed to "Activity: N tools", expanded calls dumped raw "Tool input"/"Tool output" JSON, file edits had no diff view, and shell commands had no terminal presentation. Two pairing bugs made it worse: transcript results whose call id lives on the message (not the block) rendered as a second bare "Tool" row, and parallel calls (call, call, result, result) never paired because coalescing was adjacent-only.
Why This Change Was Made
Requested redesign toward coding-agent-style activity rendering: scannable verb-first rows, inline diffs, and cheap-model purpose titles so long sessions read like a work log instead of a JSON dump.
User Impact
$ commandwith display-only syntax highlighting (harnesssh -lc '…'wrappers stripped),Edited protocol.md +2 -1with colored diffstat,Read file.ts,Wrote new-file.ts +14,Searched pattern, plus afailedbadge on errors.details.diff(real file line numbers) as add/remove tinted rows; foreign-harness edits (old_string/new_string, Codexapply_patch) fall back to a locally computed bounded line diff; writes show an all-added preview. Raw output stays behind the existing "Raw details" toggle.chat.toolTitlesRPC. Model selection is cheap-only: per-agent/defaultutilityModelwhen configured, elseopenai/gpt-5.6-luna— and the Luna default applies only when the agent's primary model already routes to OpenAI, so tool arguments never reach a provider the session was not already using. Titles never fall through to the expensive primary model. Titles cache in the per-agent SQLitecache_entries(scopetool-call-titles), so reopening sessions never re-bills. If no cheap model is usable the RPC returns nothing and rows keep deterministic labels.New protocol surface is additive:
chat.toolTitles(params/result schemas + validator), registered withoperator.writescope because cache misses spend utility-model tokens.Before / After
Collapsed before:
Live gateway session (real run, gpt-5.5, real
chat.toolTitlesround-trip producing "Ran linked list tests"):Evidence
tool-call-diff.test.ts,tool-call-view.test.ts,tool-call-grouping.test.ts,tool-titles.test.ts,src/gateway/chat-tool-titles.test.ts; updated:chat-tool-cards.test.ts,chat-message.test.ts,chat-thread.test.tsincl. new interleaved-pair and user-boundary regressions). Testbox run: https://github.com/openclaw/openclaw/actions/runs/29077207952ui/src/e2e/chat-tool-turn-outcome.e2e.test.tsgreen with a strengthened semantic assertion (error row keeps error class after a later turn recovers).pnpm check:changedgreen on Testbox (exit=0: conflict/changelog/dependency guards, typecheck core, typecheck core tests, lint core). The last few review-fix commits after that run are covered by local focused tests + scoped oxlint; PR CI reruns the full gates.gpt-5.5, branch mode vsorigin/main) ran through 11 rounds; every accepted finding was fixed (NUL-byte source file, unbounded patch parsing, Luna egress gating to OpenAI-primary agents, split-pane session/agent/client capture for title batches, stale view cache on late diff details, lit-guard repaint for arriving titles, live-vs-historical running state, streaming-output completion, arg auditability on expand). Final run: "autoreview clean: no accepted/actionable findings". One finding was consciously rejected: per-run provider provenance for titles on sessions whose primary later switched to OpenAI — the full transcript flows to the new provider on the next turn anyway, so a decorative-title-only gate would not change real egress.OPENCLAW_STATE_DIRcopy, port 19789) ran a real multi-tool coding task twice; screenshots above show unwrapped commands, merged parallel reads, group summaries, and a real Luna-generated title ("Ran linked list tests") arriving viachat.toolTitlesand rendering on the command row.docs/gateway/protocol.md(chat.toolTitles),docs/web/control-ui.md(tool rendering + titles).