Skip to content

Fix ClearToolResults corrupting typed ToolReturnPart subclasses (ToolSearchReturnPart)#383

Open
dsfaccini wants to merge 1 commit into
mainfrom
fix-380-clear-tool-results-typed-parts
Open

Fix ClearToolResults corrupting typed ToolReturnPart subclasses (ToolSearchReturnPart)#383
dsfaccini wants to merge 1 commit into
mainfrom
fix-380-clear-tool-results-typed-parts

Conversation

@dsfaccini

Copy link
Copy Markdown
Contributor

This pull request was posted by Claude Code using claude-opus-4-8 on behalf of David.

Summary

ClearToolResults blanks old tool results in place. rebuild_with_cleared selected
them with isinstance(part, ToolReturnPart), which also matches typed subclasses such
as ToolSearchReturnPart (core's search_tools return). dataclasses.replace preserves
the concrete class, so blanking the content produced a ToolSearchReturnPart whose
structured TypedDict content was now a plain placeholder string.

Core's parse_discovered_tools trusts that content shape (docstring: "No defensive
isinstance walks needed") and re-reads it on every request via content['discovered_tools'].
Once the corrupted history is written back into run state, the run's next request raises
TypeError: string indices must be integers.

The fix narrows the request-side clearing predicate to an exact type check
(type(part) is ToolReturnPart), so typed subclasses are skipped rather than blanked.
They carry discovery state core re-reads each request and reclaim little, so skipping
them is both correct and near-free. The response-side ToolCallPart args-clearing branch
is unchanged (its placeholder is type-valid there). A code comment guards against a future
revert to isinstance.

Linked Issue

Fixes #380

Checklist

  • Linked issue exists and is referenced above
  • Tests added/updated for new behavior
  • make lint && make typecheck && make test passes locally (don't stress about CI -- we'll help)
  • No changes to pyproject.toml or uv.lock (dependency changes require a separate issue)
  • Docstrings use single backticks (not RST double backticks)

`rebuild_with_cleared` matched tool results with `isinstance(part, ToolReturnPart)`,
which also caught typed subclasses like `ToolSearchReturnPart`. `dataclasses.replace`
preserves the concrete class, so blanking the content produced a `ToolSearchReturnPart`
whose structured `TypedDict` content was a plain string. Core's `parse_discovered_tools`
trusts that content shape and re-reads it on the next request, raising TypeError.

Narrow the request-side clearing predicate to an exact type check so typed subclasses
are skipped -- they carry discovery state core re-reads and reclaim little.
@coderabbitai

This comment was marked as resolved.

@macroscopeapp

macroscopeapp Bot commented Jul 17, 2026

Copy link
Copy Markdown

Approvability

Verdict: Approved

Straightforward bug fix with clear intent and minimal scope. The single-line change narrows a type check from isinstance to exact type match, preventing corruption of typed subclasses. Well-documented with explanatory comment and regression test.

You can customize Macroscope's approvability policy. Learn more.

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.

ClearToolResults corrupts history: clearing a ToolSearchReturnPart crashes the run’s next request with TypeError

1 participant