Conversation
PatchSummarizationTool.run() called .rfind() on the last message's .content, which is a plain str for ordinary replies but a list of content blocks for extended-thinking responses. Use BaseMessage.text, which normalizes both shapes to a string. Fixes mozilla#6517.
suhaibmujahid
requested changes
Oct 3, 2026
| # .text normalizes list-of-blocks content (e.g. extended thinking) to a | ||
| # plain string; .content does not, and the .rfind below needs a str. | ||
| summary = str(result["messages"][-1].text) | ||
|
|
Member
There was a problem hiding this comment.
We need here the final result, without the extended thinking.
This branch has not been deployed
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
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.
Fixes #6517.
PatchSummarizationTool.run() read
result["messages"][-1].contentand called.rfind()on it directly. LangChain message content is a plainstrforordinary replies, but for extended-thinking responses it comes back as a
list of content blocks instead, and
.rfindhas no meaning on a list. Thatis exactly the traceback in the issue.
BaseMessage.text(langchain-core) is built for this: it joins only thetype: "text"blocks into a string when content is a list, and returns thestring unchanged when it already is one. Swapping
.contentfor.textfixes both cases with no other change to the method.
Verified live (not just read):
AIMessage(content=[...]).textreturns the joined text for list content,the original string for str content, and
""for an empty list, allconfirmed against the pinned langchain 1.3.15.
tests/test_patch_summarization.py: one test reproduces the crashwith list content, without the fix it raises the same
AttributeErrorand with the fix it passes; a second test pins the existing plain-string
path still works.
ruff check,ruff format --check,codespell, and a scopedmypyrunon the changed file are all clean.
bugbug testsCI task (it clones Mercurialhistory and installs the full ML dependency set); I verified the changed
module and its new tests against the actual production code path with
only the packages that module imports.