Conversation
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Extract expanded tool call cards into Svelte components and dispatch edit, command, read/search, network, and generic renderers from the tool call view model. Signed-off-by: Matt Toohey <contact@matttoohey.com>
Stop rendering raw JSON alongside structured tool call details. Raw output now appears only when no structured blocks (error, stdout, stderr, output, network response) could be extracted, so typed tool calls render just their designed UI. Signed-off-by: Matt Toohey <contact@matttoohey.com>
Give each tool call renderer a tighter, type-specific UI: - Diffs get a header with the file path, a created/deleted badge, and add/remove counts, and long unchanged runs collapse behind an expandable "N unchanged lines" row (logic extracted to inlineDiffRows.ts with tests). - Command cards drop the redundant status row (already in the header dot and footer) and show the exit code only when non-zero. - Edit and read/search cards stop repeating the same path across the field row, location chips, and diff label. - Search matches are labelled with their count, and read output is labelled Content. Signed-off-by: Matt Toohey <contact@matttoohey.com>
Address the code review on the tool call detail work:
- Fix buildDiffRows dropping both halves of a modified pair that
straddles an LCS anchor (e.g. a line edited while moving across an
unchanged line); each half now renders as its own removed/added row
with its char highlights, with a regression test.
- Render diff char highlights from segments with {#each} instead of
{@html}, removing the XSS-sensitive path and the manual escaping.
- Make OutputSections project viewModel.sections through its include
options (via a new source discriminator on output sections) instead
of re-deriving the error/stdout/stderr/raw suppression rules, so the
logic lives only in buildToolCallSections.
- Compute hasDetails from sections other than status and empty rows so
the expand caret hides for tools with nothing to show.
- Delete the dead acpTranscript exports the view model superseded
(AcpDiff, diffsFromAcpContent, simpleUnifiedDiff, displayLocations,
toolResultText).
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Clean up the duplication and raw markdown in the expanded read/search tool card: - Stop showing the path twice: a location matching the Path field now folds into it as a ":line" suffix (path.rs:1952) instead of a chip repeating the full path. Remaining chips hide the path prefix when the card already names it, collapsing to "Line N"; edit cards get the same treatment for locations the diff header covers. - Strip wrapping markdown code fences from ACP content text before rendering, so read output no longer shows literal ``` lines around the file content (matches the existing legacy tool_result handling). - Run search match paths through makePathsRelative so they drop the workspace-root prefix like paths elsewhere in the chat UI. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
Trim titles from expanded tool call cards that only restate what the
card already conveys:
- Successful calls no longer render a "Succeeded" footer row; the green
check already in the card header shows it. The status footer now
appears only when it adds something (failed, cancelled, or still
running).
- The primary output block renders without a header label ("Content"
for reads, "Output" elsewhere), since it is obviously the tool's
result. Stdout, stderr, error, and raw-output blocks keep their
labels to tell them apart.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Stop the Run/Ran tool expansion from spending a whole row on the "$" prompt and another on the copy button: - The command now flows beside the prompt like a terminal line. The "$" sits in a fixed-width gutter (via padding + a negative-margin prefix) and the command wraps under itself with a hanging indent, instead of flex-wrapping the long command onto a line below a stranded "$". - A label-less output block (the primary command result) no longer renders a header row just to hold the copy button; the button tucks into the top-right corner of the output, with the block's own background masking any text it overlaps. Labelled blocks (stdout, stderr, error) keep their header row. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6270339842
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| export function extractToolCallOutput(item: RichToolItem): ToolCallOutput { | ||
| const rawRecord = recordOrNull(item.rawOutput); | ||
| const rawText = formatJson(item.rawOutput); |
There was a problem hiding this comment.
When a collapsed tool card has a large structured rawOutput (or large raw input), ToolCallCard still builds the view model unconditionally for the header, so this formatJson runs during every live transcript rebuild/poll instead of only when the user expands the card. That reintroduces the expensive pretty-printing path the previous implementation avoided for collapsed cards and can make sessions with verbose tool payloads sluggish; keep raw formatting lazy or cache it behind expansion.
Useful? React with 👍 / 👎.
|
|
||
| if (metadata.diffs.length > 0) return 'edit'; | ||
| if (metadata.terminalRefs.length > 0) return 'command'; | ||
| if (hasNetworkMetadata(metadata.input) || hasNetworkMetadata(item.rawOutput)) return 'network'; |
There was a problem hiding this comment.
For an unknown/non-network tool whose raw JSON output happens to contain a common key like status or title (for example {status: "ok", id: ...}), this branch classifies it as network; NetworkToolDetails then treats that status as a structured response and suppresses the raw output, so the rest of the payload disappears from the expanded card. Network detection should not rely on output-only generic keys unless there is request/url/method evidence, or it should still include the raw fallback for these cases.
Useful? React with 👍 / 👎.
Resolve the two Codex review comments on the tool call detail work: - Keep raw output/input JSON formatting lazy. buildToolCallViewModel runs on every live transcript rebuild to feed the collapsed card header, but it was eagerly formatJson-ing rawOutput (and raw input) even for collapsed cards and even when structured output made the raw fallback unused. inputText and rawText are now memoized getters gated by cheap hasInputText/hasRawText presence flags, so the pretty-printing only runs once an expanded card reads the value. - Stop generic status payloads from hijacking the network renderer. An unknown tool whose output merely contained a common key like status or title was classified as network, and NetworkToolDetails then treated that status as a structured response and suppressed the raw output, dropping the rest of the payload. Network detection now requires strong request/response evidence (url/method/request/response) rather than generic keys. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replaces the generic expanded tool call rendering in the session chat pane with a typed view model and per-type detail components.
toolCallViewModel.ts): classifies ACP tool calls (edit, command, read/search, network, generic) and projects each into structured sections — fields, diffs, output blocks, locations, status — so rendering logic lives in one tested place instead of the chat pane template.tool-calls/): dedicated Svelte renderers dispatched from the view model —EditToolDetails,CommandToolDetails,ReadSearchToolDetails,NetworkToolDetails, and aGenericToolDetailsfallback — plus sharedToolCallCard/ToolCallHeader/ToolStatusDot/OutputSectionsbuilding blocks.SessionChatPane.svelteshrinks by ~370 lines.InlineToolDiff.svelte,inlineDiffRows.ts): diff header with file path, created/deleted badge, and add/remove counts; long unchanged runs collapse behind an expandable "N unchanged lines" row; char-level highlights render from segments (no{@html}).$gutter with a hanging indent; redundant labels dropped (no "Succeeded" footer, no "Output"/"Content" label on the primary block); paths no longer repeated across fields, location chips, and diff headers; markdown fences stripped from read content; search match paths made workspace-relative.acpTranscript.tsexports superseded by the view model are deleted.Testing
toolCallViewModel.test.ts) and diff row building (inlineDiffRows.test.ts), including a regression test for modified pairs straddling an LCS anchor.🤖 Generated with Claude Code