potiuk commented on PR #71853: URL: https://github.com/apache/airflow/pull/71853#issuecomment-5751677421
My inclination is to close this rather than iterate on it, but I want the common.ai owners to confirm before I do. @gopidesupavan @kaxil @Lee-W @ashb — this proposes that `HookToolset.call_tool` convert every tool failure into `ModelRetry`, which reverses the behaviour `choosing_a_toolset.rst` currently documents as a deliberate difference from `SQLToolset`. My concern is that `HookToolset` exposes arbitrary hook methods and cannot know which are idempotent, so a method that performed its side effect and then raised would be re-invoked by the model. That seems like a design decision for you rather than something to settle in review. To be clear about what I think is legitimate here: when the model passes a bad argument, a retry genuinely could fix it, and that is what the three-bucket contract in `toolsets.rst` means by "the agent rejected the request in a way rephrasing could fix". A narrow version — convert argument-shaped errors, let transient and terminal ones propagate — would fit the contract. That is a different change from this one and deserves its own design discussion. @ColtenOuO — nothing wrong with the implementation, and thanks for the iteration here and on #71734. If the owners agree with me, the productive next step is an issue proposing the narrow version, which I would be glad to review. If they would rather take this as-is with the docs updated, I will happily defer. -- 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]
