ColtenOuO commented on code in PR #70433:
URL: https://github.com/apache/airflow/pull/70433#discussion_r3713936431
##########
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:
Confirmed, and agreed both directions belong here. Proposal below.
Same wire format both ways, so a round trip is a pass-through and the model
never transforms anything:
```python
def _json_default(value: Any) -> str:
if isinstance(value, (bytes, bytearray, memoryview)):
return _b64(value)
return str(value)
def _serialize_for_llm(value: Any) -> str:
if value is None:
return "null"
if isinstance(value, str):
return value
if isinstance(value, (bytes, bytearray, memoryview)):
return _b64(value)
try:
return json.dumps(value, default=_json_default)
except (TypeError, ValueError):
return str(value)
```
Top-level bytes bypass `json.dumps`, mirroring the existing `str` early
return — no surrounding quotes, so the value goes straight back into a `bytes`
parameter. Nested bytes are handled by swapping `default=`, leaving `str()` as
the fallback for datetimes and friends.
Correctness comes from the runtime `isinstance`, not from annotations, and
deliberately so: `download` is overloaded on `filename`, `CloudKMSHook.decrypt`
yields no hints at all, and nested `dict[str, bytes]` gives no signal either.
Annotations only decide whether the model gets *told* — `" Binary output is
returned base64-encoded."` appended to the description when the return
annotation mentions `bytes`. That check is looser than the argument side's
`_resolves_to_bytes`, which is safe here because it governs a sentence of
documentation rather than how data is handled; the worst case is a missing
hint, never a wrong payload.
**Not** adding `contentEncoding` to `return_schema`: `download` returns a
path string when `filename` is passed, so the declaration would be wrong half
the time. The argument side can declare it because parameter types are static.
`toolsets.rst` gets both directions plus the limitation that bytes nested
inside a JSON structure are indistinguishable from ordinary strings once
encoded.
--
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]
