bito-code-review[bot] commented on code in PR #44508:
URL: https://github.com/apache/superset/pull/44508#discussion_r4067538836


##########
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:
   <div>
   
   
   <div id="suggestion">
   <div id="issue"><b>Dead exception branches</b></div>
   <div id="fix">
   
   `get_response_size_bytes` (response_size_utils.py:48-77) catches all 
exceptions internally and returns a 1MB fallback, so it never raises. Both 
`except MemoryError` and `except Exception` branches here are unreachable dead 
code — the MemoryError handling is misleading since the callee swallows 
MemoryError itself. Simplify to a direct call.
   </div>
   
   
   <details>
   <summary>
   <b>Code suggestion</b>
   </summary>
   <blockquote>Check the AI-generated fix before applying</blockquote>
   <div id="code">
   
   
   ````suggestion
           actual_bytes = get_response_size_bytes(estimation_target)
   ````
   
   </div>
   </details>
   
   
   
   </div>
   
   
   
   
   <small><i>Code Review Run #970b02</i></small>
   </div>
   
   ---
   Should Bito avoid suggestions like this for future reviews? (<a 
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
   - [ ] Yes, avoid them



-- 
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]

Reply via email to