Skip to content

fix(ui): show grep search parameters in pending tool state - #2070

Merged
andrinoff merged 4 commits into
charmbracelet:mainfrom
LarsArtmann:fix/grep-show-search-params
Oct 15, 2026
Merged

andrinoff merged 4 commits into
charmbracelet:mainfrom
LarsArtmann:fix/grep-show-search-params

Conversation

@LarsArtmann

@LarsArtmann LarsArtmann commented Jan 31, 2026 •

Copy link
Copy Markdown
Contributor

Video

OLD — Crush v0.88.1 (https://vhs.charm.sh/vhs-2qd1GGwcD2e6KaecbheOXm.gif)

OLD

NEW (https://vhs.charm.sh/vhs-3oraVxwIQ9M6BJH1KPy79i.gif)

NEW

Problem

When the grep tool is running, the pending state only showed ● Grep with an animation — hiding the search pattern, path, include filter, and literal mode from the user. There was no way to know what was being searched while the tool was in progress.

Solution

Move parameter parsing (JSON unmarshal) before the pending check in GrepToolRenderContext.RenderTool(), then render the pending state with the full tool header (pattern + params) followed by the animation.

Before

● Grep ▋                                          ← pending (no params visible)
● Grep pattern path=. include=*.go                ← completed

After

● Grep pattern path=. include=*.go literal=true ▋ ← pending (full params + animation)
● Grep pattern path=. include=*.go                ← completed (unchanged)

Still applicable

Checked against current main (v0.97.x, 2026-09-29): GrepToolRenderContext.RenderTool in internal/ui/chat/search.go still early-returns the bare pendingTool while the tool runs, so the params stay hidden without this change.

Merge conflict resolution

Rebased against latest main to adopt the refactored toolHeader signature (now takes *ToolRenderOpts instead of bool).

Test plan

  • go build ./... passes
  • go test ./internal/ui/chat/... passes
  • go test ./internal/agent/tools/... passes
  • Manually verify pending state shows full params with animation
  • Manually verify completed state is unchanged

💘 Generated with Crush

@LarsArtmann
LarsArtmann requested a review from a team as a code owner January 31, 2026 03:41
@LarsArtmann
LarsArtmann requested review from kujtimiihoxha and meowgorithm and removed request for a team January 31, 2026 03:41
LarsArtmann added a commit to LarsArtmann/SystemNix that referenced this pull request Mar 11, 2026
Re-applies the unmerged upstream PR #2070 which was temporarily removed
during the v0.47.2 update due to hash computation issues.

## What the patch does

Displays grep search parameters (pattern, path, include filters, literal
mode) in the pending state UI instead of just showing "● Grep" with
animation. This provides better UX transparency when grep tool is running.

## Technical details

- Patch URL: https://github.com/charmbracelet/crush/commit/e4aa1742699db27c2ccd5e9c2b9f4d0948870581.patch
- Patch hash: sha256-3G73sqv4UdwNZHs6HKr9mCYO8WWplJAnLrurDpEiK20=
- PR status: Still open (awaiting review)
- Applies cleanly to v0.47.2

## Changes

- Added `fetchpatch` to imports
- Added patches array with PR #2070 commit patch

## Verification

- Build succeeds: `nix build .#crush-patched`
- Version output: `crush version v0.47.2`
- Flake check passes: `nix flake check --no-build`

## Related

- PR #2070: charmbracelet/crush#2070
- Previous commits: 6fc7606, 65f79fb (hash fixes for v0.47.2)

💘 Generated with Crush

Assisted-by: Claude via Crush <crush@charm.land>
@LarsArtmann

Copy link
Copy Markdown
Contributor Author

This PR is now MERGEABLE again. 😃

@LarsArtmann LarsArtmann changed the title fix(ui): show grep search parameters in pending state instead of just 'Working..' fix(ui): show grep search parameters in pending tool state Aug 12, 2026
@meowgorithm
meowgorithm removed the request for review from kujtimiihoxha August 12, 2026 21:07
@meowgorithm

Copy link
Copy Markdown
Member

Oh this looks nice!

@LarsArtmann

Copy link
Copy Markdown
Contributor Author

Oh this looks nice!

Looks nice in the Terminal, then the screenshot somehow

Screenshot 2026-08-12 at 23 20 58

@meowgorithm thing so too :)

Comment thread internal/ui/chat/search.go Resolved
@LarsArtmann

Copy link
Copy Markdown
Contributor Author

It's hard to show the difference but I tried.
See description Gifs

@LarsArtmann
LarsArtmann force-pushed the fix/grep-show-search-params branch from 72fadb4 to 0d16f0d Compare August 25, 2026 01:56
@LarsArtmann
LarsArtmann force-pushed the fix/grep-show-search-params branch from 0d16f0d to 53ad6a0 Compare September 15, 2026 08:06
@LarsArtmann

Copy link
Copy Markdown
Contributor Author

Try merging main again. Merged what I hope is a fix for that this afternoon.

Rebased and green again (verified against current main). The gap itself is still there though — GrepToolRenderContext.RenderTool in internal/ui/chat/search.go still returns the bare pendingTool while grep runs, so the search params stay hidden without this PR.

@meowgorithm
meowgorithm requested review from andrinoff and removed request for meowgorithm September 30, 2026 13:30
@LarsArtmann
LarsArtmann force-pushed the fix/grep-show-search-params branch from 53ad6a0 to 4e6c9f2 Compare September 30, 2026 13:51

@andrinoff andrinoff left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just small changes! Thank you!

}

var params tools.GrepParams
if err := json.Unmarshal([]byte(opts.ToolCall.Input), &params); err != nil {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The pending check now runs after the input is parsed, but while a tool call is still streaming its input is usually incomplete. That means users will briefly see an "Invalid parameters" error instead of the loading spinner. The pending branch should fall back to the old spinner when parsing fails, rather than showing an error. Can you test this out?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch — a partial input like {"pattern":"fo indeed flashed "Invalid parameters" while streaming.

Now falls back to the old pendingTool spinner when the input doesn't parse yet and the call is still pending. Covered by a test feeding the streaming prefixes "", "{", "{"pattern":"fo" in both compact modes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Went one step further here: while the input streams, a token walk (json.Decoder.Token) now recovers every field that already closed — the pattern shows up as soon as its quote arrives, path/include pop in as they complete, spinner until then. Covered in TestGrepToolMessageItem_PendingPartialInputShowsCompleteFields + TestPartialJSONFields.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I am thinking of stream parsing it instead though (WIP)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Stream parsing landed: after the token walk, the raw tail is matched for the value still streaming, so the pattern grows character by character (cut escapes like a trailing \ or short \u are trimmed). Covered in TestPartialJSONFields.

toolParams = append(toolParams, "literal", "true")
}

if opts.IsPending() {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The old pending view respected opts.Compact (nested tools get a different style), but the new one ignores it. Grep calls inside sub-agents will now look different from the rest while loading

@LarsArtmann LarsArtmann Oct 3, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The old pending view respected opts.Compact (nested tools get a different style), but the new one ignores it.

I think this one is actually covered — toolHeader applies the nested style itself:

nested := opts != nil && opts.Compact // tools.go, toolHeader

Same NameNested that pendingTool used. Rendered both byte-for-byte and the icon+name prefix is identical, compact or not — only the params differ, which is the feature. (NameNormal and NameNested even render identically in every current theme.) The spinner fallback from the other thread uses pendingTool directly, so it stays compact-aware too.

@LarsArtmann
LarsArtmann force-pushed the fix/grep-show-search-params branch 3 times, most recently from ec61117 to d80bcfd Compare October 9, 2026 15:37
@LarsArtmann

This comment was marked as low quality.

@LarsArtmann

Copy link
Copy Markdown
Contributor Author

@andrinoff I think the PR is ready for a new review.

@LarsArtmann
LarsArtmann requested a review from andrinoff October 10, 2026 00:00
@andrinoff

Copy link
Copy Markdown
Member

Are animations all well?

@LarsArtmann
LarsArtmann force-pushed the fix/grep-show-search-params branch from d80bcfd to 2c1562b Compare October 12, 2026 08:49
@LarsArtmann

LarsArtmann commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor Author

@andrinoff I tested a local build of the last commit for a couple of hours and it looks all fine too me. I would merge it.

@andrinoff
andrinoff enabled auto-merge (squash) October 14, 2026 13:14
… "Working.."

Previously, when the grep tool was running (pending state), the UI only displayed
"● Grep" with an animation, hiding what the tool was actually searching for.
This was frustrating for users who wanted to see the search pattern, path,
include filters, and literal mode setting while the grep was in progress.

The root cause was that the tool parameters were parsed AFTER the pending check,
so they weren't available when rendering the pending state. This commit moves the
parameter parsing before the pending check, ensuring the full tool header with
all parameters is available during execution.

Changes:
- Moved parameter parsing before the pending check in GrepToolRenderContext
- Updated pending state to call toolHeader with the parsed parameters
- Added animation rendering to the tool header in pending state
- Maintains consistency with non-pending state rendering

The user now sees: "● Grep A9041!eAD$80@a9 path=. include=*.go literal=true ..."
with animation instead of just "● Grep ... animation", providing complete context
about what the grep tool is searching for while it's running.

File: internal/ui/chat/search.go:95-140
Function: GrepToolRenderContext.RenderTool

💘 Generated with Crush

Assisted-by: GLM-4.7 via Crush <crush@charm.land>
💘 Generated with Crush

Assisted-by: Crush:glm-5.3
💘 Generated with Crush

Assisted-by: Crush:glm-5.3
💘 Generated with Crush

Assisted-by: Crush:glm-5.3
auto-merge was automatically disabled October 14, 2026 16:13

Head branch was pushed to by a user without write access

@LarsArtmann
LarsArtmann force-pushed the fix/grep-show-search-params branch from 2c1562b to 4d9b67d Compare October 14, 2026 16:13
@LarsArtmann

Copy link
Copy Markdown
Contributor Author

I only tested on Linux. Now the CI/CD for mac should work too.

@andrinoff
andrinoff enabled auto-merge (squash) October 15, 2026 11:49
@andrinoff
andrinoff merged commit c89ef9d into charmbracelet:main Oct 15, 2026
15 checks passed
@LarsArtmann
LarsArtmann deleted the fix/grep-show-search-params branch October 15, 2026 19:45
BrunoKrugel pushed a commit to BrunoKrugel/crush that referenced this pull request Oct 16, 2026
…elet#2070)

* fix(ui): show grep search parameters in pending state instead of just "Working.."

Previously, when the grep tool was running (pending state), the UI only displayed
"● Grep" with an animation, hiding what the tool was actually searching for.
This was frustrating for users who wanted to see the search pattern, path,
include filters, and literal mode setting while the grep was in progress.

The root cause was that the tool parameters were parsed AFTER the pending check,
so they weren't available when rendering the pending state. This commit moves the
parameter parsing before the pending check, ensuring the full tool header with
all parameters is available during execution.

Changes:
- Moved parameter parsing before the pending check in GrepToolRenderContext
- Updated pending state to call toolHeader with the parsed parameters
- Added animation rendering to the tool header in pending state
- Maintains consistency with non-pending state rendering

The user now sees: "● Grep A9041!eAD$80@a9 path=. include=*.go literal=true ..."
with animation instead of just "● Grep ... animation", providing complete context
about what the grep tool is searching for while it's running.

File: internal/ui/chat/search.go:95-140
Function: GrepToolRenderContext.RenderTool

💘 Generated with Crush

Assisted-by: GLM-4.7 via Crush <crush@charm.land>

* fix(ui): fall back to spinner while grep input is still streaming

💘 Generated with Crush

Assisted-by: Crush:glm-5.3

* feat(ui): render partially streamed grep params while pending

💘 Generated with Crush

Assisted-by: Crush:glm-5.3

* feat(ui): grow grep pattern live while its value streams

💘 Generated with Crush

Assisted-by: Crush:glm-5.3
(cherry picked from commit c89ef9d)
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.

3 participants