Skip to content

add grep tool formatters#1019

Open
LasmarKhalifa wants to merge 1 commit into
07-02/pi-tool-formatter-editfrom
07-02/pi-tool-formatter-grep
Open

add grep tool formatters#1019
LasmarKhalifa wants to merge 1 commit into
07-02/pi-tool-formatter-editfrom
07-02/pi-tool-formatter-grep

Conversation

@LasmarKhalifa

@LasmarKhalifa LasmarKhalifa commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Closes #1027

LasmarKhalifa commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@LasmarKhalifa LasmarKhalifa force-pushed the 07-02/pi-tool-formatter-edit branch from 2d3ce63 to 7730ee2 Compare July 3, 2026 14:50
@LasmarKhalifa LasmarKhalifa force-pushed the 07-02/pi-tool-formatter-grep branch 2 times, most recently from 3b64e5c to 10a0676 Compare July 3, 2026 15:06
@LasmarKhalifa LasmarKhalifa force-pushed the 07-02/pi-tool-formatter-edit branch from 7730ee2 to 676a57b Compare July 3, 2026 15:30
@LasmarKhalifa LasmarKhalifa force-pushed the 07-02/pi-tool-formatter-grep branch 2 times, most recently from 3799ee2 to 919a175 Compare July 3, 2026 16:02
@LasmarKhalifa LasmarKhalifa marked this pull request as ready for review July 3, 2026 16:30

@mathiusj mathiusj left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

grep formatters are the most heuristic in the stack — a couple of notes.

Comment thread lib/roast/cogs/agent/providers/pi/messages/tool_call_message.rb
#: () -> String
def format_grep
lines = content.to_s.lines.map(&:strip).reject(&:empty?)
matches, notes = lines.partition { |line| line.match?(%r{\A\S+/}) || line.match?(/\A(?:\S+:)?\d+:/) }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Match/note heuristic is best-effort and will miscount. %r{\A\S+/} treats any line starting with a non-space run + / as a match, and \A(?:\S+:)?\d+: as a line-number hit. Real grep hits that don't carry a path/line prefix (e.g. grep -h, or matched content that doesn't fit either shape) get bucketed as notes and undercount; conversely a note containing an early / counts as a match. Since "3 matches" is shown to the user, a wrong count is a visible lie. It won't break anything, but please add a comment that this depends on Pi's grep output format and is intentionally approximate — so nobody later trusts the count as exact or "fixes" it by tightening the regex without knowing the contract.

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 matching was actually done against logs on the Claude side that spanned over a hundred calls to grep! The current matching got it all right, so I actually expect this to be accurate. My suggestion is to keep this as it is and if we ever notice inaccuracies we can always follow up. What do you think?

@LasmarKhalifa LasmarKhalifa force-pushed the 07-02/pi-tool-formatter-edit branch from 676a57b to 6401f99 Compare July 10, 2026 15:23
@LasmarKhalifa LasmarKhalifa force-pushed the 07-02/pi-tool-formatter-grep branch 2 times, most recently from 2ad8c53 to 74166f6 Compare July 10, 2026 15:50
@LasmarKhalifa LasmarKhalifa force-pushed the 07-02/pi-tool-formatter-edit branch from 6401f99 to 27d834e Compare July 10, 2026 15:50
@LasmarKhalifa LasmarKhalifa force-pushed the 07-02/pi-tool-formatter-grep branch from 74166f6 to f877583 Compare July 10, 2026 16:13
@LasmarKhalifa LasmarKhalifa force-pushed the 07-02/pi-tool-formatter-edit branch from 27d834e to 9f3ab5a Compare July 10, 2026 16:13
@LasmarKhalifa LasmarKhalifa force-pushed the 07-02/pi-tool-formatter-grep branch from f877583 to f55f3f1 Compare July 10, 2026 16:33
@LasmarKhalifa LasmarKhalifa force-pushed the 07-02/pi-tool-formatter-edit branch from 9f3ab5a to a161e59 Compare July 10, 2026 16:33
@LasmarKhalifa LasmarKhalifa force-pushed the 07-02/pi-tool-formatter-grep branch from f55f3f1 to d0bc73d Compare July 10, 2026 16:38
@LasmarKhalifa LasmarKhalifa force-pushed the 07-02/pi-tool-formatter-edit branch from a161e59 to a785239 Compare July 10, 2026 16:38
@LasmarKhalifa LasmarKhalifa force-pushed the 07-02/pi-tool-formatter-grep branch from d0bc73d to 5beb941 Compare July 10, 2026 18:11
@LasmarKhalifa LasmarKhalifa requested a review from mathiusj July 10, 2026 18:14
@LasmarKhalifa LasmarKhalifa linked an issue Jul 13, 2026 that may be closed by this pull request
@LasmarKhalifa LasmarKhalifa force-pushed the 07-02/pi-tool-formatter-edit branch from a785239 to d3310c0 Compare July 14, 2026 20:37
@LasmarKhalifa LasmarKhalifa force-pushed the 07-02/pi-tool-formatter-grep branch from 5beb941 to c15e82c Compare July 14, 2026 20:37
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.

Pi format_grep tool implemetation

2 participants