fix(tools): harden Grep format_output for local-model searches JSON

[JessicaMulein]

Summary

• Coerce searches in Grep format_output/execute using parse_tool_arguments and _coerce_searches
• Prevents AttributeError: 'str' object has no attribute 'get' when local models emit truncated/double-encoded searches JSON
• Add regression test for malformed string searches

Test plan

• pytest tests/tools/test_tool_arguments.py -k grep_format_output

Note: also cherry-picked to dev-integration (383b6fd) for BrightVision dogfood.

Made with Cursor

PR Summary by Typo

Overview

This PR hardens the Grep tool's format_output function to gracefully handle malformed or double-encoded JSON inputs, particularly from local models, preventing crashes and improving the robustness of search result processing.

Key Changes

• Introduced a new _coerce_searches method to normalize and parse various forms of the searches parameter, including strings, lists, and dictionaries, and to handle double-encoded JSON.
• Modified the execute method to utilize _coerce_searches for input validation and normalization.
• Updated the format_output method to use responses.parse_tool_arguments for safer parsing of tool arguments and _coerce_searches for processing search operations.
• Added a new test case to ensure the format_output function does not crash when encountering malformed string searches.

Work Breakdown

Category	Lines Changed
New Work	50 (89.3%)
Churn	1 (1.8%)
Rework	5 (8.9%)
Total Changes	56

To turn off PR summary, please visit Notification settings.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e6ff45fe-988f-4b8e-93fd-eea06864d7dc

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/grep-searches-coerce

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[typo-app]

Static Code Review 📊

🛑 1 quality checks failed!

• 1 Vulnerability Issues CRITICAL

[qodo-code-review]

Review Summary by Qodo

Harden Grep tool for malformed local model searches JSON

🐞 Bug fix 🧪 Tests

Walkthroughs

Description

• Add _coerce_searches() method to handle double-encoded or malformed JSON strings from local
  models
• Replace manual JSON parsing with parse_tool_arguments() helper in format_output()
• Coerce searches parameter in execute() method for robustness
• Add regression test for malformed string searches handling

Diagram

flowchart LR
  A["Local Model<br/>Double-Encoded JSON"] -->|"malformed searches"| B["_coerce_searches()"]
  B -->|"normalize to list"| C["format_output()"]
  C -->|"safe display"| D["Tool Output"]
  E["execute()"] -->|"coerce input"| F["Validated searches"]

Loading

File Changes

1. cecli/tools/grep.py 🐞 Bug fix +32/-5

Add robust search parameter coercion for malformed JSON

• Add _coerce_searches() classmethod to normalize double-encoded or malformed JSON strings into
 valid list of dicts
• Import responses helper module for JSON parsing utilities
• Replace manual json.loads() with responses.parse_tool_arguments() in format_output()
• Apply _coerce_searches() in both execute() and format_output() methods
• Add validation to return error if searches is empty after coercion

cecli/tools/grep.py

---

2. tests/tools/test_tool_arguments.py 🧪 Tests +23/-1

Add malformed searches regression test

• Fix existing test assertion: change assert coder.io.tool_error.called to `assert not
 coder.io.tool_error.called` (empty searches should not error)
• Add new regression test test_grep_format_output_malformed_string_searches_does_not_crash() for
 double-encoded JSON strings
• Test verifies that malformed searches like "[{\\"file_pattern>\\nCOPYING" are handled gracefully
 without crashing

tests/tools/test_tool_arguments.py

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0)

1. Silent malformed searches display 🐞 Bug ◔ Observability

Description

GrepTool.format_output coerces params['searches'] and then prints nothing when coercion yields an
empty list, without emitting any warning/error, so malformed/dropped searches appear as a normal
tool call with no operations shown. This can hide broken tool arguments and makes UI output
inconsistent with execute(), which returns an explicit error when searches becomes empty after
coercion.

Code

cecli/tools/grep.py[R267-276]

Evidence

format_output() calls _coerce_searches() and only outputs operations when the coerced list is
truthy; there is no warning path when coercion returns []. _coerce_searches() returns [] on
unparseable strings, and the new regression test demonstrates a malformed string searches input
where the code now intentionally avoids tool_error—meaning the UI will stay quiet unless we add an
explicit warning.

cecli/tools/grep.py[87-108]
cecli/tools/grep.py[262-308]
cecli/tools/grep.py[121-128]
tests/tools/test_tool_arguments.py[83-124]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`GrepTool.format_output()` uses `_coerce_searches()` to avoid crashes, but when coercion returns `[]` it silently omits the search operation list (no `tool_warning`/`tool_error`). This hides malformed tool arguments (especially common with local-model double-encoding/truncation) and makes it harder to understand why the tool call has no displayed searches.

### Issue Context
- `_coerce_searches()` explicitly returns `[]` on parse failures and drops non-dict/unparseable items.
- `execute()` now returns an explicit error when `searches` is empty after coercion; `format_output()` does not, so the UI can look “fine” while execution fails or behaves unexpectedly.

### Fix Focus Areas
- cecli/tools/grep.py[267-308]
- cecli/tools/grep.py[87-108]

### Suggested fix
- In `format_output()`, capture the raw value (e.g., `raw_searches = params.get('searches')`).
- After coercion, if `raw_searches` was provided/non-empty but `searches` is empty, call `coder.io.tool_warning(...)` (preferred) or `coder.io.tool_error(...)` with a short message like: `"Malformed searches; unable to display search operations"`.
- Optionally, enhance `_coerce_searches()` to return `(coerced, dropped_count, had_parse_error)` so `format_output()` can warn when some entries were dropped even if others remain.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[typo-app]

AI Code Review 🤖

Files Reviewed: 2
Comments Added: 0
Lines of Code Analyzed: 61
Critical Issues: 0

PR Health: Excellent 🔥

_{Give 👍 or 👎 on each review comment to help us improve.}

[JessicaMulein]

Superseded by upstream PR to cecli-dev/cecli (branch rebased on upstream/main).

[JessicaMulein]

Upstream PR opened: cecli-dev#549