bito-code-review[bot] commented on code in PR #44508:
URL: https://github.com/apache/superset/pull/44508#discussion_r4067808776
##########
superset/mcp_service/middleware.py:
##########
@@ -1809,41 +1809,39 @@ async def on_call_tool(
# Execute the tool
response = await call_next(context)
- # When the response is a ToolResult, estimate tokens on the actual
- # payload inside content[0].text rather than on the ToolResult
- # wrapper (which would double-serialize the JSON string).
+ # When the response is a ToolResult, measure the size of the actual
+ # payload inside content[0].text rather than the ToolResult wrapper
+ # (which would double-serialize the JSON string).
extracted = self._extract_payload_from_tool_result(response)
estimation_target = extracted if extracted is not None else response
try:
- estimated_tokens = estimate_response_tokens(estimation_target)
+ actual_bytes = get_response_size_bytes(estimation_target)
except MemoryError as me:
logger.warning(
- "MemoryError while estimating tokens for %s: %s", tool_name, me
+ "MemoryError while measuring response size for %s: %s",
tool_name, me
)
# Treat as over limit to avoid further serialization
- estimated_tokens = self.token_limit + 1
+ actual_bytes = self.max_bytes + 1
except Exception as e: # noqa: BLE001
- logger.warning(
- "Failed to estimate response tokens for %s: %s", tool_name, e
- )
+ logger.warning("Failed to measure response size for %s: %s",
tool_name, e)
# Conservative fallback: block rather than risk OOM
- estimated_tokens = self.token_limit + 1
+ actual_bytes = self.max_bytes + 1
Review Comment:
<!-- Bito Reply -->
The suggestion is correct because `get_response_size_bytes` is designed to
handle exceptions internally and return a fallback value, making the
surrounding `try-except` blocks in the code unreachable. Removing these dead
branches simplifies the logic and improves readability without changing the
functional behavior of the error handling.
**superset/mcp_service/middleware.py**
```
actual_bytes = get_response_size_bytes(estimation_target)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
