fix: tool parameters #168
Merged
fix: tool parameters #168
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
👍 Looks good to me! Reviewed everything up to a57ec30 in 2 minutes and 20 seconds
More details
- Looked at
219lines of code in3files - Skipped
0files when reviewing. - Skipped posting
11drafted comments based on config settings.
1. dynamiq/nodes/tools/exa_search.py:25
- Draft comment:
Ensure 'is_accessible_to_agent=False' is intentional for all added fields. Verify agents shouldn’t access these parameters. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =30%<= threshold50%
The comment is asking the PR author to ensure that a specific behavior is intentional, which violates the rule against asking for confirmation of intention. However, it does point out a specific setting ('is_accessible_to_agent=False') that might be critical to the functionality, which could be useful if rephrased to suggest checking or confirming the logic rather than asking for verification.
2. dynamiq/nodes/tools/tavily.py:15
- Draft comment:
Confirm that adding 'is_accessible_to_agent=False' to input schema fields (e.g. search_depth, topic) meets intended security/privacy needs. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
The comment is asking the PR author to confirm their intention regarding a security/privacy change. This violates the rule against asking the author to confirm their intention or ensure behavior is intended.
3. dynamiq/prompts/prompts.py:140
- Draft comment:
Good refactor removing duplicate parse_bytes_to_base64 and parse_image_url_parameters; ensure consolidated logic handles all file types as expected. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
The comment is asking the author to ensure that the consolidated logic works as expected after removing duplicate functions. This falls under the category of asking the author to ensure behavior, which is against the rules.
4. dynamiq/prompts/prompts.py:72
- Draft comment:
Typo in docstring: 'formated' should be 'formatted'. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
5. dynamiq/nodes/tools/exa_search.py:25
- Draft comment:
Added 'is_accessible_to_agent' metadata to multiple fields in ExaInputSchema. Ensure that downstream logic respects these restrictions consistently. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
The comment is asking the author to ensure that downstream logic respects the new metadata restrictions. This falls under the category of asking the author to ensure behavior, which is against the rules.
6. dynamiq/nodes/tools/tavily.py:16
- Draft comment:
Input schema fields in TavilyInputSchema now include 'is_accessible_to_agent' metadata. Confirm that access control is applied uniformly throughout. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
The comment is asking the author to confirm that access control is applied uniformly, which violates the rule against asking for confirmation of intention or behavior. It does not provide a specific code suggestion or point out a specific issue that needs addressing.
7. dynamiq/prompts/prompts.py:130
- Draft comment:
Refactored VisionMessage by moving parse_bytes_to_base64 and updating parse_image_url_parameters to use it. Verify that image data conversion remains consistent post-refactor. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
The comment is asking the PR author to verify that the image data conversion remains consistent after the refactor. This falls under the category of asking the author to ensure the behavior is intended or to double-check things, which is against the rules.
8. dynamiq/nodes/tools/exa_search.py:31
- Draft comment:
Typo in the description for 'use_autoprompt': consider changing “a Exa query” to “an Exa query” for correct article usage. - Reason this comment was not posted:
Comment was on unchanged code.
9. dynamiq/nodes/tools/exa_search.py:94
- Draft comment:
Typo in the description for 'use_autoprompt': consider changing “a Exa query” to “an Exa query” for correct article usage. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
10. dynamiq/prompts/prompts.py:73
- Draft comment:
Typo in the docstring of theformat_messagemethod: 'Returns formated copy of message' should be 'Returns formatted copy of message'. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
11. dynamiq/prompts/prompts.py:123
- Draft comment:
The docstring forVisionMessagelists the types as 'VisionTextMessage | VisionImageMessage', but the actual types used areVisionMessageTextContentandVisionMessageImageContent. Consider updating the docstring to maintain consistency. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_iZMAGDXVReyPczkF
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
|
What do you think is the best strategy for parameters. It is a tradeoff between scalability and robustness (particularly for smaller models). |
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 06666cc in 28 seconds
More details
- Looked at
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
3drafted comments based on config settings.
1. dynamiq/prompts/prompts.py:216
- Draft comment:
Prefer consistent formatting: remove extra spaces around '=' when passing keyword arguments. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%
None
2. dynamiq/prompts/prompts.py:216
- Draft comment:
Using keyword arguments for Message creation improves clarity and prevents positional misinterpretation. Also, consider removing extra spaces around '=' (i.e., use role=self.role) to align with PEP 8 style. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%
None
3. dynamiq/prompts/prompts.py:73
- Draft comment:
Typographical Error: In the docstring for Message.format_message, 'formated' should be corrected to 'formatted'. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_nLMy0cLI9DGrItha
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on bc42425 in 33 seconds
More details
- Looked at
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
3drafted comments based on config settings.
1. dynamiq/prompts/prompts.py:215
- Draft comment:
Minor formatting change: Removed extra whitespace around '=' for consistency. No logic change. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
2. dynamiq/prompts/prompts.py:215
- Draft comment:
Minor formatting fix: removed extra spaces between keyword arguments for consistency. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
This comment is purely informative and does not provide any actionable feedback or suggestions for improvement. It simply states a minor formatting change that was made, which does not align with the rules provided.
3. dynamiq/prompts/prompts.py:73
- Draft comment:
Typographical error: The docstring in Message.format_message says 'Returns formated copy of message'. Please change 'formated' to 'formatted'. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_jqQksgFQY3qZCFqf
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
olbychos
approved these changes
Feb 27, 2025
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 2f5fb47 in 38 seconds
More details
- Looked at
17lines of code in1files - Skipped
0files when reviewing. - Skipped posting
3drafted comments based on config settings.
1. dynamiq/nodes/agents/base.py:121
- Draft comment:
Formatting update only; no functional changes. Consider verifying that context values are not None for better safety. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%
None
2. dynamiq/nodes/agents/base.py:121
- Draft comment:
Improved list formatting enhances readability. Consider adding checks for None values from context (e.g. for 'input_message' or 'role') to avoid potential issues. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%
None
3. dynamiq/nodes/agents/base.py:563
- Draft comment:
Typo: The variable 'formated_prompt_blocks' is misspelled. Consider renaming it to 'formatted_prompt_blocks' for clarity. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_TnLA63jz4lhZfXjq
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 1424df0 in 1 minute and 30 seconds
More details
- Looked at
12lines of code in1files - Skipped
0files when reviewing. - Skipped posting
4drafted comments based on config settings.
1. dynamiq/nodes/agents/react.py:166
- Draft comment:
Ensure removal of the phrase 'Simply give a clear and concise answer.' from REACT_BLOCK_OUTPUT_FORMAT is intentional. Verify corresponding documentation. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
This comment violates several rules:
- It asks to "verify" and "ensure" something is intentional, which is explicitly called out as not useful
- It asks to verify documentation, which is speculative
- The change is very simple - just removing a line from a string constant
- There's no evidence that removing this line would cause any issues
Could removing this instruction to "give a clear and concise answer" make the LLM's responses worse? Should we be more cautious about modifying LLM prompts?
While prompt changes can affect LLM behavior, this particular line was redundant with the general goal of clear communication. The remaining instruction to "avoid phrases like 'based on...'" still guides the LLM toward direct answers.
Delete this comment. It asks for verification without providing evidence of any actual issues, and the change is simple enough that we can assume it was intentional.
2. dynamiq/nodes/agents/react.py:166
- Draft comment:
Removed the phrase 'Simply give a clear and concise answer.' from REACT_BLOCK_OUTPUT_FORMAT. Confirm that this change is intentional and that it doesn't lead to inconsistent guidance in your tool prompt instructions. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50%
The comment asks the author to confirm their intention, which directly violates the rule "Do NOT ask the PR author to confirm their intention, to explain, to double-check things, to ensure the behavior is intended...". The comment doesn't point out any actual issue - it just asks for confirmation.
Could this change have unintended consequences that should be pointed out? Could removing guidance about being concise lead to worse output?
Even if there could be consequences, the comment doesn't point them out - it just asks for confirmation. If there are specific issues, they should be stated directly rather than asking the author to think about them.
Delete this comment. It violates the rule about asking for confirmation and doesn't provide any concrete value or point out specific issues.
3. dynamiq/nodes/agents/react.py:84
- Draft comment:
Typo: Please change 'strickly' to 'strictly' in the instructions (line 84) to improve readability. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
4. dynamiq/nodes/agents/react.py:198
- Draft comment:
Typo in final_answer_function_schema: In the description for 'provide_final_answer', please remove the extra 'if' so that it reads 'Function should be called when you can answer the initial request'. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_zeuw9DlCBnyN8V8d
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
MaksymAI
approved these changes
Feb 27, 2025
anastasiasenyk
approved these changes
Feb 27, 2025
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 8d68160 in 1 minute and 13 seconds
More details
- Looked at
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
3drafted comments based on config settings.
1. dynamiq/nodes/agents/react.py:227
- Draft comment:
Changing default inference_mode from XML to DEFAULT may affect behavior in XML mode-specific branches. Ensure tests and documentation reflect this change. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%
The comment is asking the author to ensure that tests and documentation reflect a change in default behavior. This is a request for confirmation and testing, which violates the rules. The comment does not provide a specific code suggestion or point out a specific issue with the code itself.
2. dynamiq/nodes/agents/react.py:227
- Draft comment:
Changed default inference_mode from XML to DEFAULT. Ensure this change is intentional, as it may alter the agent's behavior (e.g., prompt instructions and output parsing). - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50%
This comment violates the rule "Do NOT ask the PR author to confirm their intention, to explain, to double-check things, to ensure the behavior is intended". The comment is just asking the author to verify their change, without providing evidence that the change is problematic. If there was a specific issue with using DEFAULT mode, the comment should point that out directly.
Maybe this is actually a breaking change that could affect downstream users? Maybe there's a good reason to be cautious about changing defaults?
While changing defaults can be impactful, the comment doesn't provide any evidence of problems. If there are specific compatibility or functionality concerns, those should be stated directly rather than just asking the author to "ensure" the change is intentional.
Delete this comment. It violates the rule against asking authors to verify their intentions. If there are specific issues with using DEFAULT mode, those should be called out directly.
3. dynamiq/nodes/agents/react.py:80
- Draft comment:
Typographical error: In the REACT_BLOCK_INSTRUCTIONS string, please replace 'strickly' with 'strictly' to improve readability and consistency. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_2K1YX214x8RoNdj6
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Important
This PR refines tool parameter handling by updating input schemas, validation, and logging across agents, tools, and prompts.
AgentInputSchemainbase.pyto includerolein context for validation.roledefault to empty string inAgentclass inbase.py.ReActAgentinreact.pyto useInferenceMode.DEFAULT.parse_xml_and_extract_info()inreact.py.is_accessible_to_agent=Falseto several fields inExaInputSchemainexa_search.pyandTavilyInputSchemaintavily.py.parse_bytes_to_base64()toVisionMessageinprompts.pyand reused it inparse_image_url_parameters().prompts.pyrelated toparse_bytes_to_base64()andparse_image_url_parameters().react.pyandexa_search.py.This description was created by
for 8d68160. It will automatically update as commits are pushed.