-
Notifications
You must be signed in to change notification settings - Fork 2.4k
chore: refactor cli() function to reduce line count #6272
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
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.
Pull request overview
This PR successfully refactors the monolithic cli() function from 557 lines to 131 lines by extracting command handling logic into focused helper functions. The refactoring maintains all existing behavior while significantly improving code maintainability.
Key changes:
- Extracted 12 helper functions for different command handlers (MCP, session, run, schedule, bench, recipe, term, etc.)
- Introduced 3 parameter structs (
InteractiveSessionArgs,RunCommandArgs,ParseRunInputArgs) to avoid clippy'stoo_many_argumentswarnings - Simplified the main
cli()function to a clean match statement that delegates to specialized handlers
|
this PR is auto generated and will be refined. will move out of draft when ready for human review |
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.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
| retry_config: recipe_info.as_ref().and_then(|r| r.retry_config.clone()), | ||
| output_format: output_opts.output_format, | ||
| }) | ||
| .await; |
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 not for now, but we should really use a, eh, builder pattern for build_session(SessionBuilderConfig
Split the main CLI function into smaller helper functions: - get_command_name(): Extract command name for telemetry - handle_mcp_command(): MCP server dispatch - handle_session_subcommand(): Session list/remove/export/diagnostics - handle_interactive_session(): Interactive session with telemetry - log_session_completion(): Shared session completion logging - parse_run_input(): Parse run command input sources - handle_run_command(): Execute run command - handle_schedule_command(): Schedule subcommand dispatch - handle_bench_command(): Benchmark subcommand dispatch - handle_recipe_subcommand(): Recipe subcommand dispatch - handle_term_subcommand(): Terminal subcommand dispatch - handle_default_session(): Default session when no command
* 'main' of github.com:block/goose: refactor: when changing provider/model,load existing provider/model (#6334) chore: refactor configure_extensions_dialog to reduce line count (#6277) chore: refactor handle_configure to reduce line count (#6276) chore: refactor interactive session to reduce line count (#6274) chore: refactor docx_tool to reduce function size (#6273) chore: refactor cli() function to reduce line count (#6272) make sure the models are using streaming properly (#6331) feat: add a max tokens env var (#6264) docs: slash commands topic (#6333) fix(ci): prevent gh-pages branch bloat (#6340) chore(deps): bump qs and body-parser in /documentation (#6338) Skip the smoke tests for dependabot PRs (#6337)
Summary
Refactors the main
cli()function incrates/goose-cli/src/cli.rsby:#[command(flatten)]patternChanges
Helper Functions
Split the monolithic
cli()function into smaller, well-named helper functions:get_command_name(): Extract command name for telemetryhandle_mcp_command(): MCP server dispatchhandle_session_subcommand(): Session list/remove/export/diagnosticshandle_interactive_session(): Interactive session with telemetrylog_session_completion(): Shared session completion loggingparse_run_input(): Parse run command input sourceshandle_run_command(): Execute run commandhandle_schedule_command(): Schedule subcommand dispatchhandle_bench_command(): Benchmark subcommand dispatchhandle_recipe_subcommand(): Recipe subcommand dispatchhandle_term_subcommand(): Terminal subcommand dispatchhandle_default_session(): Default session when no commandOption Structs
Consolidated related arguments into logical groupings:
SessionOptionsdebug,max_tool_repetitions,max_turnsExtensionOptionsextensions,remote_extensions,streamable_http_extensions,builtinsInputOptionsinstructions,input_text,recipe,system,params,additional_sub_recipes,explain,render_recipeOutputOptionsquiet,output_formatModelOptionsprovider,modelRunBehaviorinteractive,no_session,resume,scheduled_job_idBenefits
SessionOptionsandExtensionOptionsare shared between Session and Run commandshandle_run_commandreduced from 11 args to 7 struct argsTesting
cargo test -p goose-cli)Related
Part of TSK-696: Simplify clippy baseline system