gopidesupavan commented on code in PR #70433:
URL: https://github.com/apache/airflow/pull/70433#discussion_r3708007662
##########
providers/common/ai/src/airflow/providers/common/ai/toolsets/hook.py:
##########
@@ -136,6 +136,12 @@ async def call_tool(
) -> Any:
method_name = name.removeprefix(self._tool_name_prefix) if
self._tool_name_prefix else name
method: Callable[..., Any] = getattr(self._hook, method_name)
+ bytes_params = _bytes_param_names(method)
Review Comment:
Agree with all your points. I checked `_digest`, it is `json.dumps` without
`default=`, so bytes in `tool_args` will fingerprint as `None` and those calls
lose replay verification with `durable=True`. So keeping the decode in
`call_tool` makes sense.
one more thing from my side.
**Output side has the same problem.** `_serialize_for_llm` passes bytes to
`json.dumps(..., default=str)`, so the model gets the Python repr:
```python
json.dumps(b'\x89PNG', default=str) # -> "b'\\x89PNG'"
```
`GCSHook.download` returns bytes and our docs recommend exposing such
read-only methods. So download -> load_bytes through an agent already writes
wrong data, and base64 only on the input side does not fix it. I think we
should define both directions in this PR. @ColtenOuO FYI
--
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]
