Loosen typing of actions to Message#1318
Conversation
There was a problem hiding this comment.
Pull request overview
This PR loosens type annotations around agent “actions” so callbacks and environment steps can accept a generic Message (while preserving runtime safeguards for non-tool actions).
Changes:
- Broadened
on_agent_action_callbacktypes fromToolRequestMessagetoMessagein agent runners and LDP wiring. - Updated
PaperQAEnvironment.step()to acceptMessageand perform a runtime type-check, returning a guidance message for non-tool actions.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/paperqa/agents/main.py | Broadens callback type annotations to accept Message instead of ToolRequestMessage. |
| src/paperqa/agents/env.py | Broadens step() input type to Message and keeps a runtime guard for non-tool actions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # if given a plain Message, or into LDP as a tool-calling SimpleAgent | ||
| # should not be responding with a plain Message | ||
| return ( # type: ignore[unreachable] | ||
| return ( |
There was a problem hiding this comment.
Can you use the default_no_tool_calls_response you added in aviary?
There was a problem hiding this comment.
Hm are you sure? I retained the logicthat was already here to avoid changing behavior. There are two differences wrt the default:
- slightly different message
- configurable reward
There was a problem hiding this comment.
Ah I see per configurable reward.
I am not attached to the "You must call tools to proceed." message. Wdyt then of default_no_tool_calls_response[0]?
| # if given a plain Message, or into LDP as a tool-calling SimpleAgent | ||
| # should not be responding with a plain Message | ||
| return ( # type: ignore[unreachable] | ||
| return ( |
There was a problem hiding this comment.
Ah I see per configurable reward.
I am not attached to the "You must call tools to proceed." message. Wdyt then of default_no_tool_calls_response[0]?
|
@jamesbraza I feel that's less readable, I'm ok with it as-is |
dosubot
bot
added
the
size:M
Companion PR to Future-House/ldp#424, turns out we already had some safeguard logic here.