fix: Refactor MCP API to fix value truncation#8298
Conversation
|
""" WalkthroughThe Changes
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
dosubot
Bot
added
size:M
github-actions
Bot
added
bug
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/backend/base/langflow/api/v1/mcp.py (1)
260-262: Avoid spawning an unnecessary background task when progress notifications are disabled
progress_taskis always created, even whenenable_progress_notificationsisFalse.
That means a task is scheduled, immediately returns, then gets cancelled – an avoidable event-loop hop.-progress_task = asyncio.create_task(send_progress_updates()) +progress_task = None +if mcp_config.enable_progress_notifications and server.request_context.meta.progressToken: + progress_task = asyncio.create_task(send_progress_updates())Remember to guard later
cancel()/awaitblocks withif progress_task:.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/backend/base/langflow/api/v1/mcp.py(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/backend/base/langflow/api/v1/mcp.py (3)
src/backend/base/langflow/api/v1/endpoints.py (1)
simple_run_flow(109-162)src/backend/base/langflow/api/v1/schemas.py (1)
SimplifiedAPIRequest(354-363)src/backend/base/langflow/schema/message.py (1)
Message(35-277)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Update Starter Projects
- GitHub Check: Run Ruff Check and Format
- GitHub Check: Run benchmarks (3.12)
- GitHub Check: Optimize new Python code in this PR
- GitHub Check: Ruff Style Check (3.13)
github-actions
Bot
added
bug
CodSpeed Performance ReportMerging #8298 will degrade performances by 14.16%Comparing Summary
Benchmarks breakdown
|
dosubot
Bot
added
size:L
github-actions
Bot
added
bug
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/backend/base/langflow/api/v1/mcp_projects.py (1)
322-334: Result extraction may produce unreadable output for complex types
str(value)is used as a fallback for non-Messageresults. For dicts, lists, or custom objects this leads to raw Python representations leaking through the API.
Consider serialising withjson.dumps(for JSON-serialisable types) or adding a dedicated formatter to ensure clients always receive user-friendly text.-else: - collected_results.append(types.TextContent(type="text", text=str(value))) +else: + collected_results.append( + types.TextContent( + type="text", + text=json.dumps(value, default=str) if isinstance(value, (dict, list)) else str(value), + ) + )🧰 Tools
🪛 Pylint (3.3.7)
[convention] 326-326: Line too long (103/100)
(C0301)
[convention] 331-331: Line too long (112/100)
(C0301)
[convention] 334-334: Line too long (117/100)
(C0301)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/backend/base/langflow/api/v1/mcp_projects.py(3 hunks)
🧰 Additional context used
🪛 Pylint (3.3.7)
src/backend/base/langflow/api/v1/mcp_projects.py
[error] 11-11: Unable to import 'fastapi'
(E0401)
[error] 12-12: Unable to import 'fastapi.responses'
(E0401)
[error] 13-13: Unable to import 'mcp'
(E0401)
[error] 14-14: Unable to import 'mcp.server'
(E0401)
[error] 15-15: Unable to import 'mcp.server.sse'
(E0401)
[error] 16-16: Unable to import 'sqlalchemy.orm'
(E0401)
[error] 17-17: Unable to import 'sqlmodel'
(E0401)
[convention] 310-310: Line too long (115/100)
(C0301)
[convention] 326-326: Line too long (103/100)
(C0301)
[convention] 331-331: Line too long (112/100)
(C0301)
[convention] 334-334: Line too long (117/100)
(C0301)
[convention] 344-344: Line too long (112/100)
(C0301)
[warning] 335-335: Catching too general exception Exception
(W0718)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Run benchmarks (3.12)
- GitHub Check: Optimize new Python code in this PR
- GitHub Check: Update Starter Projects
| input_request = SimplifiedAPIRequest( | ||
| input_value=processed_inputs.get("input_value", ""), session_id=conversation_id | ||
| ) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Potential loss of user-supplied arguments – only input_value is forwarded
SimplifiedAPIRequest is instantiated with the conversation id and a single field, input_value.
All other entries contained in the incoming arguments dict (e.g. file selectors, optional parameters, model settings, etc.) are silently discarded.
Unless SimplifiedAPIRequest is intentionally restricted to one field, this breaks backward compatibility and is the most likely root-cause for “missing/empty input” bug reports.
-input_request = SimplifiedAPIRequest(
- input_value=processed_inputs.get("input_value", ""), session_id=conversation_id
-)
+input_request = SimplifiedAPIRequest(
+ session_id=conversation_id,
+ **processed_inputs, # keep behaviour symmetrical with the public API
+)Please verify the target schema and forward all supported keys, or explicitly validate/raise for unsupported ones to avoid silent data loss.
🏁 Script executed:
#!/bin/bash
# Locate the definition of SimplifiedAPIRequest
rg -n "class SimplifiedAPIRequest" -A5 -B5Length of output: 974
#!/bin/bash
# Show the full definition of SimplifiedAPIRequest including Config
sed -n '340,380p' src/backend/base/langflow/api/v1/schemas.py#!/bin/bash
# Print the full SimplifiedAPIRequest class definition (including all fields and Config)
sed -n '340,420p' src/backend/base/langflow/api/v1/schemas.py
# Search for where processed_inputs is defined or populated in mcp_projects.py
rg -n "processed_inputs" -C5 src/backend/base/langflow/api/v1/mcp_projects.pyForward all supported request fields to avoid silent data loss
The SimplifiedAPIRequest schema (in src/backend/base/langflow/api/v1/schemas.py) defines multiple fields beyond input_value—including input_type, output_type, output_component—and is configured with extra="forbid". In src/backend/base/langflow/api/v1/mcp_projects.py (lines 282–284), only input_value and session_id are passed, dropping everything else from processed_inputs.
• File: src/backend/base/langflow/api/v1/mcp_projects.py
Lines: ~282–284
Current code:
- input_request = SimplifiedAPIRequest(
- input_value=processed_inputs.get("input_value", ""),
- session_id=conversation_id
- )Suggested change:
- input_request = SimplifiedAPIRequest(
- input_value=processed_inputs.get("input_value", ""),
- session_id=conversation_id
- )
+ input_request = SimplifiedAPIRequest(
+ session_id=conversation_id,
+ **processed_inputs, # forwards all schema-supported keys
+ )This ensures backward compatibility and surface validation errors for any unsupported keys.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| input_request = SimplifiedAPIRequest( | |
| input_value=processed_inputs.get("input_value", ""), session_id=conversation_id | |
| ) | |
| input_request = SimplifiedAPIRequest( | |
| session_id=conversation_id, | |
| **processed_inputs, # forwards all schema-supported keys | |
| ) |
🤖 Prompt for AI Agents
In src/backend/base/langflow/api/v1/mcp_projects.py around lines 282 to 284, the
instantiation of SimplifiedAPIRequest only forwards input_value and session_id,
ignoring other supported fields from processed_inputs. To fix this, extract all
keys from processed_inputs that match the fields defined in SimplifiedAPIRequest
(such as input_type, output_type, output_component, etc.) and pass them along
with session_id when creating the SimplifiedAPIRequest instance. This will
preserve all supported user-supplied arguments and avoid silent data loss or
validation errors due to extra fields.
| result = await simple_run_flow( | ||
| flow=flow, | ||
| input_request=input_request, | ||
| stream=False, |
There was a problem hiding this comment.
@phact Is this a change in user-facing behavior?
There was a problem hiding this comment.
no, looks like it's a backend optimization. @ogabrielluiz is there a reason not to point build_flow_and_stream to simple_run_flow? or replace it entirely? That way other code that uses it (voice mode IIRC) gets the impovement too.
There was a problem hiding this comment.
I think we should remove the use of build_flow_and_stream but maybe in another PR.
The simple_run_flow takes in the flow object while build_flow_... takes the id. We could update it but one returns a response which is then iterated, while the other returns the result of the run.
| text_content = types.TextContent(type="text", text=value.get_text()) | ||
| collected_results.append(text_content) | ||
| else: | ||
| collected_results.append(types.TextContent(type="text", text=str(value))) |
There was a problem hiding this comment.
Possible change in behavior re: collecting events by messages before results? Unsure if ordering changes or matters.
There was a problem hiding this comment.
Before it was collecting events and was getting duplicated data since the last end_vertex event and the end event would have the same data.
Now it gets the result of the flow.
| try: | ||
| progress_task = asyncio.create_task(send_progress_updates()) | ||
| progress_task = None | ||
| if mcp_config.enable_progress_notifications and self.server.request_context.meta.progressToken: |
There was a problem hiding this comment.
leaving for seb to check - I am unfamiliar with the options here.
github-actions
Bot
added
bug
|
Quick check will Simple Run flow create or initiate a separate task/job just like build_flow_and_stream @ogabrielluiz Any other views no moving from build_flow_and_stream to simple run flow? |
we should move to simple_run_flow asap because it is the one used by the |
- Replaced `InputValueRequest` with `SimplifiedAPIRequest` for cleaner input management. - Updated flow execution logic to utilize `simple_run_flow`, enhancing clarity and performance. - Removed unnecessary background task handling and streamlined progress updates. - Improved message collection from flow outputs, ensuring robust handling of results.
- Implemented a try-except block around the flow execution to catch and handle exceptions gracefully. - Enhanced message collection logic to ensure that errors during tool execution are communicated back as text content. - Improved robustness of the `handle_call_tool` function by ensuring all potential errors are captured and reported.
- Updated error handling in the `handle_call_tool` function to provide more descriptive error messages. - Changed the error message format to include the flow name, enhancing clarity for debugging purposes. - Ensured that all exceptions during tool execution are captured and reported as text content.
- Improved the logic for processing outputs in the `handle_call_tool` function to handle messages and results more comprehensively. - Streamlined the collection of text content from both messages and results, ensuring all relevant outputs are captured. - Enhanced robustness by ensuring that all outputs are processed, regardless of their structure.
- Updated the logic for progress task creation in the `handle_call_tool` function to ensure it only initializes when progress notifications are enabled and a progress token is present. - Enhanced the cancellation and exception handling of the progress task to prevent potential errors when it is not created. - Improved overall robustness of the function by ensuring that progress updates are managed correctly based on the current context.
…PServer - Replaced `InputValueRequest` with `SimplifiedAPIRequest` for improved input management. - Updated flow execution to utilize `simple_run_flow`, enhancing clarity and performance. - Refined progress notification handling to ensure tasks are only created when necessary. - Improved message collection from flow outputs, ensuring robust handling of both messages and results. - Enhanced error handling during tool execution to provide clearer feedback on failures.
Updated the send_progress_updates function to accept a progress token as an argument, improving its flexibility. Adjusted the task cancellation logic to use asyncio.gather for better exception handling. This change aims to streamline progress notifications when enabled.
ogabrielluiz
force-pushed
the
fix-mcp-truncation
branch
from
fd2d02f to
1bae2c5
Compare
github-actions
Bot
added
bug
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/backend/base/langflow/api/v1/mcp.py (1)
235-237: Address existing input handling concerns from past reviews.This code segment has the same issues identified in past reviews:
- Defaulting
input_valueto empty string when absent changes semantics- Other supported fields like
input_type,output_type,output_component,tweaksare not forwardedThe past review comment provides a comprehensive fix to preserve all supported fields and proper validation.
src/backend/base/langflow/api/v1/mcp_projects.py (1)
657-659: Same input handling issue as identified in past reviews.This code segment has the identical issue to the one in
mcp.py- onlyinput_valueandsession_idare forwarded toSimplifiedAPIRequest, dropping other supported fields fromprocessed_inputs. The past review comment provides the correct solution using**processed_inputs.
🧹 Nitpick comments (1)
src/backend/base/langflow/api/v1/mcp_projects.py (1)
679-716: Progress task management improvements but output processing concerns remain.The refactoring successfully addresses the progress task lifecycle issues identified in past reviews:
- Fixed progress token scoping - Line 682 correctly passes
progress_tokenexplicitly tosend_progress_updates- Improved task cancellation - Line 715 uses
asyncio.gather(return_exceptions=True)instead of manual exception checkingHowever, similar to the other file, output processing could be more robust.
Consider adding the same defensive checks as suggested for the other file:
result = await simple_run_flow( flow=flow, input_request=input_request, stream=False, api_key_user=current_user, ) + # Ensure outputs exist before processing + if not result.outputs: + collected_results.append(types.TextContent(type="text", text="No outputs generated from flow execution")) + return collected_results + # Process all outputs and messages for run_output in result.outputs:This maintains consistency across both MCP implementation files.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 680-680: Line too long (115/100)
(C0301)
[convention] 698-698: Line too long (103/100)
(C0301)
[convention] 703-703: Line too long (112/100)
(C0301)
[convention] 706-706: Line too long (117/100)
(C0301)
[warning] 707-707: Catching too general exception Exception
(W0718)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/backend/base/langflow/api/v1/mcp.py(3 hunks)src/backend/base/langflow/api/v1/mcp_projects.py(3 hunks)src/backend/base/langflow/initial_setup/starter_projects/Research Translation Loop.json(0 hunks)src/backend/base/langflow/initial_setup/starter_projects/Youtube Analysis.json(0 hunks)
💤 Files with no reviewable changes (2)
- src/backend/base/langflow/initial_setup/starter_projects/Youtube Analysis.json
- src/backend/base/langflow/initial_setup/starter_projects/Research Translation Loop.json
🧰 Additional context used
🪛 Pylint (3.3.7)
src/backend/base/langflow/api/v1/mcp_projects.py
[error] 17-17: Unable to import 'fastapi'
(E0401)
[error] 18-18: Unable to import 'fastapi.responses'
(E0401)
[error] 19-19: Unable to import 'mcp'
(E0401)
[error] 20-20: Unable to import 'mcp.server'
(E0401)
[error] 21-21: Unable to import 'mcp.server.sse'
(E0401)
[error] 22-22: Unable to import 'sqlalchemy.orm'
(E0401)
[error] 23-23: Unable to import 'sqlmodel'
(E0401)
[convention] 680-680: Line too long (115/100)
(C0301)
[convention] 698-698: Line too long (103/100)
(C0301)
[convention] 703-703: Line too long (112/100)
(C0301)
[convention] 706-706: Line too long (117/100)
(C0301)
[warning] 707-707: Catching too general exception Exception
(W0718)
src/backend/base/langflow/api/v1/mcp.py
[convention] 261-261: Line too long (102/100)
(C0301)
[convention] 282-282: Line too long (104/100)
(C0301)
[convention] 285-285: Line too long (109/100)
(C0301)
[convention] 295-295: Line too long (104/100)
(C0301)
[warning] 286-286: Catching too general exception Exception
(W0718)
⏰ Context from checks skipped due to timeout of 90000ms (19)
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 6/10
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 8/10
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 7/10
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 1/10
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 10/10
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 5/10
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 4/10
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 9/10
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 3/10
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 2/10
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 2
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 4
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 3
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 1
- GitHub Check: Run Backend Tests / Unit Tests - Python 3.10 - Group 5
- GitHub Check: Lint Backend / Run Mypy (3.13)
- GitHub Check: Optimize new Python code in this PR
- GitHub Check: Update Starter Projects
- GitHub Check: Run Ruff Check and Format
🔇 Additional comments (3)
src/backend/base/langflow/api/v1/mcp.py (2)
20-21: LGTM: Import changes align with the refactoring objectives.The new imports (
simple_run_flow,SimplifiedAPIRequest,Message) are correctly added to support the transition from streaming to synchronous flow execution.Also applies to: 24-24
260-297:Details
✅ Verification successful
Refactoring improves synchronicity but has implementation issues.
The transition from streaming to synchronous execution using
simple_run_flowaligns with the PR objectives. However, several issues need attention:
Progress task management is improved - The explicit progress token parameter and proper cancellation handling address previous concerns.
Output processing still has potential for data loss - Lines 273-285 process nested outputs correctly, but there's no verification that
result.outputsexists or is non-empty.Add defensive checks for the output processing:
result = await simple_run_flow( flow=flow, input_request=input_request, stream=False, api_key_user=current_user, ) + # Ensure outputs exist before processing + if not result.outputs: + collected_results.append(types.TextContent(type="text", text="No outputs generated from flow execution")) + return collected_results + # Process all outputs and messages for run_output in result.outputs: for component_output in run_output.outputs:Could you verify that
simple_run_flowalways returns a result with anoutputsattribute, even on failure?
🏁 Script executed:
#!/bin/bash # Description: Verify the structure of result returned by simple_run_flow # Search for the return type or structure of simple_run_flow ast-grep --pattern 'def simple_run_flow($$$) -> $_:' ast-grep --pattern 'async def simple_run_flow($$$) -> $_:' # Search for result.outputs usage patterns rg -A 3 "result\.outputs" --type pyLength of output: 1024
🏁 Script executed:
#!/bin/bash # Locate the definition of simple_run_flow and confirm its return structure rg -n "async def simple_run_flow" --type py rg -n "def simple_run_flow" --type py # List all files referencing simple_run_flow to identify its import origin rg -l "simple_run_flow" --type pyLength of output: 572
🏁 Script executed:
#!/bin/bash # Extract the simple_run_flow implementation to inspect its return structure sed -n '80,300p' src/backend/base/langflow/api/v1/endpoints.pyLength of output: 8620
simple_run_flow always returns a valid outputs list, no guard needed
We verified that
simple_run_flowreturns aRunResponsewith anoutputs: list[RunOutputs]attribute on success, and on failure it raises (caught by your outerexceptblock, which appends an error message). Thereforeresult.outputsis guaranteed to exist and be iterable. No additional defensive check is required to prevent missing‐attribute errors.If you’d like to provide a user‐facing notification when the list is empty, that would be a separate UX enhancement rather than a bugfix.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 261-261: Line too long (102/100)
(C0301)
[convention] 282-282: Line too long (104/100)
(C0301)
[convention] 285-285: Line too long (109/100)
(C0301)
[convention] 295-295: Line too long (104/100)
(C0301)
[warning] 286-286: Catching too general exception Exception
(W0718)
src/backend/base/langflow/api/v1/mcp_projects.py (1)
17-17: Import changes correctly support the refactoring.The removal of
BackgroundTasksand addition ofsimple_run_flow,SimplifiedAPIRequest, andMessageimports are consistent with the synchronous execution approach.Also applies to: 25-25, 32-32, 35-35
🧰 Tools
🪛 Pylint (3.3.7)
[error] 17-17: Unable to import 'fastapi'
(E0401)
github-actions
Bot
added
bug
* refactor: Simplify flow execution and update input handling in MCP API - Replaced `InputValueRequest` with `SimplifiedAPIRequest` for cleaner input management. - Updated flow execution logic to utilize `simple_run_flow`, enhancing clarity and performance. - Removed unnecessary background task handling and streamlined progress updates. - Improved message collection from flow outputs, ensuring robust handling of results. * fix: Add error handling for tool execution in MCP API - Implemented a try-except block around the flow execution to catch and handle exceptions gracefully. - Enhanced message collection logic to ensure that errors during tool execution are communicated back as text content. - Improved robustness of the `handle_call_tool` function by ensuring all potential errors are captured and reported. * fix: Improve error messaging in tool execution for MCP API - Updated error handling in the `handle_call_tool` function to provide more descriptive error messages. - Changed the error message format to include the flow name, enhancing clarity for debugging purposes. - Ensured that all exceptions during tool execution are captured and reported as text content. * refactor: Enhance message and result handling in handle_call_tool - Improved the logic for processing outputs in the `handle_call_tool` function to handle messages and results more comprehensively. - Streamlined the collection of text content from both messages and results, ensuring all relevant outputs are captured. - Enhanced robustness by ensuring that all outputs are processed, regardless of their structure. * refactor: Improve progress notification handling in handle_call_tool - Updated the logic for progress task creation in the `handle_call_tool` function to ensure it only initializes when progress notifications are enabled and a progress token is present. - Enhanced the cancellation and exception handling of the progress task to prevent potential errors when it is not created. - Improved overall robustness of the function by ensuring that progress updates are managed correctly based on the current context. * refactor: Streamline flow execution and message handling in ProjectMCPServer - Replaced `InputValueRequest` with `SimplifiedAPIRequest` for improved input management. - Updated flow execution to utilize `simple_run_flow`, enhancing clarity and performance. - Refined progress notification handling to ensure tasks are only created when necessary. - Improved message collection from flow outputs, ensuring robust handling of both messages and results. - Enhanced error handling during tool execution to provide clearer feedback on failures. * refactor: enhance progress update handling in ProjectMCPServer Updated the send_progress_updates function to accept a progress token as an argument, improving its flexibility. Adjusted the task cancellation logic to use asyncio.gather for better exception handling. This change aims to streamline progress notifications when enabled. * refactor: add group_outputs property to message configurations in starter projects
4 participants
Simplify input management by replacing
InputValueRequestwithSimplifiedAPIRequest. Enhance flow execution clarity and performance through the use ofsimple_run_flow, while streamlining progress updates and message collection from flow outputs.Summary by CodeRabbit