test: Extract method to extract text from mcp result#3154
Open
Mofef wants to merge 4 commits intodeepset-ai:mainfrom
Open
test: Extract method to extract text from mcp result#3154Mofef wants to merge 4 commits intodeepset-ai:mainfrom
Mofef wants to merge 4 commits intodeepset-ai:mainfrom
Conversation
|
|
github-actions
bot
added
integration:mcp
type:documentation
julian-risch
requested changes
Apr 13, 2026
Member
julian-risch
left a comment
julian-risch
left a comment
There was a problem hiding this comment.
Looks quite good to me already, main change request is about renaming method name.
| def test_extract_first_text_element(): | ||
| """Test that extract_first_text skips non-text blocks and parses the first text block.""" | ||
| result = json.dumps( | ||
| { |
Member
There was a problem hiding this comment.
Let's use an actual existing tool call result, for example search the tests in the haystack repo for "content": [
| return resolved_headers | ||
|
|
||
|
|
||
| def extract_first_text_element(result: str) -> str | dict[str, Any]: |
Member
There was a problem hiding this comment.
Let's rename to _extract_first_text_element
Comment on lines
+68
to
+70
| Return the first text content block from an MCP tool response. | ||
|
|
||
| MCP tool responses may include mixed content types such as text, image, or |
Member
There was a problem hiding this comment.
Suggested change
| Return the first text content block from an MCP tool response. | |
| MCP tool responses may include mixed content types such as text, image, or | |
| Return the first text content block from an MCP tool call result. | |
| MCP tool call results may include mixed content types such as text, image, or |
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
3 participants
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.
Proposed Changes:
While looking through the coverage report vor mcp_tool.py I found a section of untested duplicate code between the tool invokation and its async counter part. I extracted that part and added a test for it.
How did you test it?
Ran tests and linting accoding to contribution guidelines. Added a new test (AI generated) for the extracted method
Checklist
fix:,feat:,build:,chore:,ci:,docs:,style:,refactor:,perf:,test:.