-
Notifications
You must be signed in to change notification settings - Fork 2.4k
feat: run sub recipe multiple times in parallel (Experimental feature in CLI) #3274
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
Conversation
0a50a86 to
cb0e6a5
Compare
* main: (23 commits) docs: VS Code MCP video (#3307) docs: fixed broken link (#3306) Add YouTube video to Netlify MCP documentation (#3302) docs: add sub-recipes topic (#3241) docs: move topics to tutorials section (#3297) site analytics (#3293) chore(release): release version 1.0.35 (#3292) docs: enhanced code editing topic (#3287) fix cu (#3291) feat: Add environment variables to override model context limits (#3260) chore(release): release version 1.0.34 (#3285) fix(devcontainer): install protoc to fix build (#3267) Enabling npx command to install on Windows Desktop (#3283) Fix: Allow native Cmd+Up/Down cursor movement when user has typed text (#3246) chore(release): release version 1.0.33 (#3284) fix Windows Env Vars (#3282) feat: bedrock image content support (#3266) Add support in goose configure for streaming http mcp tools (#3256) docs: add Alby MCP tutorial (#3217) refactor(tests): make logging test in goose-cli less flaky on macos (#3273) ...
I feel this is like an inline subrecipe with subrecipe :) (ie. nested subrecipe) Currently subrecipe block maps to one recipe. and the values are only for the subrecipe This part will be a little bit confusing with this block. This requires both In the main branch, the parallel has been implemented for different recipes as below
|
The schema level parallelism is an optional field for user to enforce the deterministic flow. I guess you are concerning the field has been defined in the recipe schema for now, and later on it is hard to remove it if we don't need it?
I’m on board with marking it experimental if we decide to include it, but I’m also up for talking more about where we see this going long term |
|
Am I understanding correctly that this new If I understand right (I'm new to sub-agent code 😬 ) but it looks like the agent first calls a tool to 'create sub recipe tasks' and then with the response it is supposed to call a 'execute tasks' tool. But ultimately if the agent decides it wants to run the recipes sequential instead of parallel or run less/more subrecipes or override the params itself it total can because the there is not a lot of enforcement to follow the subrecipe config. It mostly generates prompt engineering for the agent (apart from the value defaults we provide). If this is the case that these new schema fields are mostly optional guidance that the agent can potentially ignore and do differently then I think this is a lot of extra complexity and schema changes are painful to revert. Sub_recipes can already be run in parallel with different parameters without this PR so I think we should rely on standard prompt engineering in the prompt/system prompt rather than trying to use this schema as hints for the agent. I think there a lot of other useful things in this PR around subtask rendering in the cli and probably other refactoring but I think we should not merge the recipe schema changes 🙅 . But happy to chat and hear other opinions 😃 . |
|
conceptually - would you describe sub recipes as a way to do "sub agents" (just as that seems to be the topic of the day - ie many agents working together, would that be one way to describe this to users not familiar with goose terminology?) |
Examples of use case
|
jamadeo
left a comment
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.
My main feedback is around the need for all the flexibility of parallel vs sequential execution within a task. IMO the model should not decide how that works -- it's just getting a batch of results anyway. Sub-recipes setting a flag to specify their execution when they get a batch of parameters seems OK.
| let additional_sub_recipe = SubRecipe { | ||
| path: recipe_file_path.to_string_lossy().to_string(), | ||
| name, | ||
| timeout_in_seconds: None, |
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.
Yeah you could probably leave it out -- the sub recipes themselves will be calling tools, and they'll have their own timeouts.
One thing we definitely don't want here is a default 5-minute timeout. IMO that doesn't make much sense for tool calls in general, but that's a separate issue.
crates/goose-cli/src/session/mod.rs
Outdated
| progress_bars.log(&formatted_message); | ||
| } | ||
| } else { | ||
| } else if let Some(ref notification_type) = _notification_type { |
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.
probably should rename this without the underscore since it's used now
|
|
||
| pub const SUB_RECIPE_TASK_TOOL_NAME_PREFIX: &str = "subrecipe__create_task"; | ||
| const EXECUTION_MODE_PARALLEL: &str = "parallel"; | ||
| const EXECUTION_MODE_SEQUENTIAL: &str = "sequential"; |
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.
should this be an enum?
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.
good catch, i thought I have refactored, but missed out
| const EXECUTION_MODE_SEQUENTIAL: &str = "sequential"; | ||
|
|
||
| pub fn create_sub_recipe_task_tool(sub_recipe: &SubRecipe) -> Tool { | ||
| let input_schema = get_input_schema(sub_recipe).unwrap(); |
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.
I know it's not in this PR but this should probably handle an Err some other way than .unwrap()
| 1. PRE-CREATED TASKS: If tasks were created by subrecipe__create_task_* tools, check the execution_mode in the response: | ||
| - If execution_mode is 'parallel', use parallel execution | ||
| - If execution_mode is 'sequential', use sequential execution | ||
| - Always respect the execution_mode from task creation to maintain consistency |
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.
IMO the model shouldn't pick how the executor runs a set of tasks. The model will always get the results back together, so it only impacts the implementation details of the tasks themselves. If the model decides it needs the result of one task to run the next, it will have to separate them anyway.
| "description": "Execution strategy for multiple tasks. For pre-created tasks, respect the execution_mode from task creation. For user intent, use 'sequential' (default) unless user explicitly requests parallel execution with words like 'parallel', 'simultaneously', 'at the same time', or 'concurrently'." | ||
| }, | ||
| "tasks": { | ||
| "type": "array", |
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.
from earlier discussion -- consider whether you can just have the model pass the ID and leave it at that, since the task has already been "created"
This reverts commit 0e01630.
* main: (54 commits) UI update with sidebar and settings tabs (#3288) docs: add CLIStreamExtensionInstructions component (#3443) chore(release): release version 1.0.36 (#3436) [goose-llm] fix image content bug, add optional request_id field (#3439) fix: Set include_usage=true for OpenAI streaming (#3441) feat: `recipe list` (#2814) (#2815) docs: update github mcp config (#3433) feat: Implement streaming for OpenAI (#3413) fix: improve extension startup error messages with command details (#2694) [feat]: improve file search tools to add globsearch / grep tools (#3368) docs: typo in guide description (#3429) fix: use safe_truncate to truncate charactor (#3263) (#3264) fix: convert invalid recipe variable name to raw content (#3420) center goose mobile screenshot (#3418) docs: model context limit overrides (#3377) docs: Subagents (#3402) fix: avoid pass encoded empty string to goose run --recipe (#3361) ux: alphabetize extensions (#3416) fix: message concatenation in server session management (#3412) refactor: streamline memory directory management (#3345) ...
…hen sequential_when_repeated is true
* main: fix: Fixes structured output after streaming change to agent (#3448)
jamadeo
left a comment
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 still seems like we're going down a pretty complex road here, moving away from how models would typically call tools, but I think that conclusion should really come from experimentation. As long as we're still calling this experimental let's get it merged and keep trying things.
| path: recipe_file_path.to_string_lossy().to_string(), | ||
| name, | ||
| values: None, | ||
| sequential_when_repeated: true, |
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.
This property name is a little confusing IMO, maybe use something like allow_concurrent?
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.
Hi @jamadeo
Thank you for the review and your time!
I thought of allow_concurrent, and i feel that it might have 2 concerns:
-
allow_concurrent: falseit might confuse people that the sub-recipe cannot be running concurrently with other sub-recipes. The...when_repeatedmakes it clear that it cannot be run parallel when running repeatedly. -
By default the sub-recipe can be run in parallel. Usually if the boolean attribute is not specified, the default value is false. If
allow_concurrentis specified, people might be think theallow_concurrent: false
Naming is usually hard. Happy to change to a better name.
* main: feat(gcpvertexai): do HTTP 429 like retries for Anthropic API HTTP 529 overloaded status code (#3026) Fix a few ui edge cases - refresh occasionally crashing, chat loader over text and chat input height returning to auto (#3469) Don't default to main for build-cli (#3467) docs: add MongoDB MCP server tutorial (#2660) feat: run sub recipe multiple times in parallel (Experimental feature) (#3274) chore(release): release version 1.1.0 (#3465) chore: implement streaming for anthropic.rs firstparty provider (#3419) Fix regression: add back detail to tool-call banners (#3231) Document release process and update some just recipes (#3460) feat: add download_cli.ps1 file for windows (#3354) fix: session_file is optional (#3462) Bump more space for goose is working on it so it doesnt overlap incoming agent chat messages (#3453) Align chat input action buttons to bottom when large amount of text (#3455) docs: add Cloudflare MCP Server tutorial (#3278) feat(cli): Clear persisted session file with /clear command (#3145)
block#3274) Co-authored-by: Wendy Tang <[email protected]> Signed-off-by: Soroosh <[email protected]>
block#3274) Co-authored-by: Wendy Tang <[email protected]> Signed-off-by: Kyle Santiago <[email protected]>
block#3274) Co-authored-by: Wendy Tang <[email protected]>
block#3274) Co-authored-by: Wendy Tang <[email protected]> Signed-off-by: Adam Tarantino <[email protected]>
Adds ability to run sub-recipes multiple times with parallel execution, real-time progress tracking.
Key Features:
Changes:
Add
executionsandsequential_when_repeatedin the sub_recipe sectionvaluessection: always use the data in this section when running sub-recipesequential_when_repeated: whether it should be sequential when running this sub recipe with different parameters. optional field, default is falsesequential_when_repeated: truemeans the sub-recipe should NOT run in parallel with different params even user requested in prompts.sequential_when_repeatedis false, or not specified, it is by default to run parallel unless user requested to run sequentially in promptsNew notification Event (notification_events.rs) for real-time task updates
Enhanced executor engine with parallel worker support and error handling
Console UI components for task execution dashboard (task_execution_display/)
How it works