-
Notifications
You must be signed in to change notification settings - Fork 5.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
JavaScriptv3: Bedrock agent runtime tool use scenario example #7355
base: main
Are you sure you want to change the base?
Conversation
…ock_agent_example
console.log(toolMessage.replace(/<[^>]+>/g, "")); | ||
for (const toolRequest of toolRequests) { | ||
if (Object.hasOwn(toolRequest, "toolUse")) { | ||
const toolUse = output_message.content[1].toolUse; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems to be using the same ToolUseId for each request. I'd change it to use the toolRequest
iterated in the loop. That should fix this issue below, see how it's using the same id each time? They should be unique, that is why you are getting that error.
Requesting tool Weather_Tool, Tool use id tooluse_m9fepPTaQ4q5pTEo3ZvEIA
Requesting tool Weather_Tool, Tool use id tooluse_m9fepPTaQ4q5pTEo3ZvEIA
Requesting tool Weather_Tool, Tool use id tooluse_m9fepPTaQ4q5pTEo3ZvEIA
Requesting tool Weather_Tool, Tool use id tooluse_m9fepPTaQ4q5pTEo3ZvEIA
Requesting tool Weather_Tool, Tool use id tooluse_m9fepPTaQ4q5pTEo3ZvEIA
ValidationException: The toolResult blocks at messages.2.content contain duplicate Ids: tooluse_m9fepPTaQ4q5pTEo3ZvEIA
} catch (error) { | ||
const errorString = error.toString(); | ||
const searchString = | ||
"ValidationException: The toolResult blocks at messages.2.content contain duplicate Ids:"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue with the loop for tool use is what I think is causing this error. Once that's fixed we should be able to remove this part
toolResultFinal.push(toolResult); | ||
} catch (err) { | ||
const toolResult = { | ||
toolUseId: toolUseID, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens to the tool result if there is an error?
"What's the weather like in Seattle? " + | ||
"What's the best kind of cat? " + | ||
"Where is the warmest city in Washington State right now? " + | ||
"What's the warmest city in California right now?\n" + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The text file says France, but the comments say California. It made me wonder when California moved to France until I figured that out.
|
||
const displayAskQuestion4 = new ScenarioOutput( | ||
"displayAskQuestion4", | ||
"Press enter to ask question number 4 (default is 'What's the warmest city in California right now?')", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The questions all say "y/n" but only ask me to press Enter. Is there a way we can remove the y/n?
import { fileURLToPath } from "node:url"; | ||
import { dirname } from "node:path"; | ||
const __filename = fileURLToPath(import.meta.url); | ||
const __dirname = dirname(__filename); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this variable used anywhere?
); | ||
if (toolUse.name === "Weather_Tool") { | ||
try { | ||
let jsonData; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this variable used anywhere?
} | ||
} | ||
} | ||
// |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Empty comment.
@@ -1,4 +1,4 @@ | |||
jinja2>=3.0.3 | |||
pyyaml>=5.3.1 | |||
typer==0.15.1 | |||
aws-doc-sdk-examples-tools @ git+https://github.com/awsdocs/aws-doc-sdk-examples-tools | |||
aws-doc-sdk-examples-tools @ git+https://github.com/awsdocs/aws-doc-sdk-examples-tools |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These requirements files look like local changes, should they be in this PR?
@@ -206,6 +206,15 @@ bedrock-runtime_Scenario_ToolUse: | |||
genai: some | |||
snippet_tags: | |||
- Bedrock.ConverseTool.dotnetv3.SendConverseRequest | |||
JavaScript: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There should be 3 keys updated in the metadata, refer to the table in the specification, looks like 2 are missing.
This pull request adds a JavaScript version of the Converse Tool scenario example.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.