feat:model-specific-configuration #466
Merged
Conversation
This file contains hidden or 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
config Add in a new ModelConfig struct, as well as a trait ProviderModelConfig which should be implemented on each Provider to propgate up model specific configuration when used by the main agent.rs loop. ModelConfig adds the context_limit variable and moves the name, temperature and max_tokens configuration within it. context_limit is set by the following order of precedence, and set at construction time 1. explicit context_limit 2. defaults by model name (see get_model_specific_limit) 3. a global default set to 200_000 Remove the global ESTIMATE_FACTOR, and make it configurable per model Add configuration via goose-cli profiles, verify goose-server environment variable based configuration
Contributor
There was a problem hiding this comment.
Copilot reviewed 5 out of 17 changed files in this pull request and generated no comments.
Files not reviewed (12)
- crates/goose-cli/src/commands/configure.rs: Evaluated as low risk
- crates/goose-cli/src/commands/session.rs: Evaluated as low risk
- crates/goose-cli/src/profile.rs: Evaluated as low risk
- crates/goose-cli/src/test_helpers.rs: Evaluated as low risk
- crates/goose-server/src/configuration.rs: Evaluated as low risk
- crates/goose-server/src/routes/reply.rs: Evaluated as low risk
- crates/goose-server/src/state.rs: Evaluated as low risk
- crates/goose/examples/image_tool.rs: Evaluated as low risk
- crates/goose/src/agent.rs: Evaluated as low risk
- crates/goose/src/providers/anthropic.rs: Evaluated as low risk
- crates/goose/src/providers/base.rs: Evaluated as low risk
- crates/goose/src/providers/configs.rs: Evaluated as low risk
Comments suppressed due to low confidence (1)
crates/goose/src/providers/mock.rs:22
- [nitpick] The parameter name model_config is ambiguous. It should be renamed to config to be consistent with the rest of the codebase.
model_config: ModelConfig::new("mock-model".to_string()),
kalvinnchau
force-pushed
the
kalvin/model-context-limits
branch
2 times, most recently
from
cd1067f to
b2de530
Compare
lifeizhou-ap
added a commit
that referenced
this pull request
Dec 17, 2024
michaelneale
added a commit
that referenced
this pull request
Dec 18, 2024
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.
add model specific configuration
This PR adds model specific configuration and updates the
goose-cliandgoose-servercrates to use the updated Provider configuration and handle new configurationchanges
ModelConfigstruct that is composed into eachProviderspecific configcontext_limit+estimate_factor,temperatureandmax_tokenscontext_limithas some defaults for known models set duringModelConfig::new()construction time.an explicitly set limit > default known model limit > DEFAULT_CONTEXT_LIMIT (128_000)ModelConfigstruct and updated relevant uses of the model configurationgoose-cliandgoose-serverto useModelConfigwhen constructingProvidersgoose-cliandgoose-serverto handle the new config parameters following the current approach in each respective crate