Skip to content

Conversation

@Dan-Wuensch
Copy link
Contributor

Resolves #4173

Changes included:

  • Configurability of AWS Bedrock retry settings via environment variables
  • New default retry settings for AWS Bedrock based on observed testing (these help mitigate failures due to running out of retries within the throttling window while don't push it too far)
  • Add missing Databricks retry settings to providers documentation

Testing steps performed:

@Dan-Wuensch Dan-Wuensch requested a review from a team as a code owner August 25, 2025 14:53
@Dan-Wuensch
Copy link
Contributor Author

I observed the following test is failing, but checked and it is also failing on main:

failures:

---- providers::base::tests::test_provider_metadata_context_limits stdout ----

thread 'providers::base::tests::test_provider_metadata_context_limits' panicked at crates/goose/src/providers/base.rs:564:9:
assertion `left == right` failed
  left: 200000
 right: 128000


failures:
    providers::base::tests::test_provider_metadata_context_limits

@taniandjerry
Copy link
Contributor

Awesome job on your first contribution! Let me tag the dev team to take a look ❤️ @DOsinga @michaelneale @alexhancock @zanesq @jamadeo @katzdave

"anthropic.claude-3-5-sonnet-20241022-v2:0",
"anthropic.claude-3-7-sonnet-20250219-v1:0",
"anthropic.claude-sonnet-4-20250514-v1:0",
"anthropic.claude-sonnet-4-20250514-v1:0",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe change the default model too?

Copy link
Contributor Author

@Dan-Wuensch Dan-Wuensch Aug 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the default Goose install script use latest instead of version pinning?

If so, I worry about disruption to existing users by changing the model since their prompts may work worse (or better!) on a new model. Could require tuning prompt(s) and running evals to adopt in some cases

That upgrade concern probably only applies to pipeline / automatic use cases not desktop CLI/GUI though. Idk what percentage of adoption that represents, and then of course Bedrock would only be a subset of that

What do you think about me logging an Issue for the model update instead, to have that be a separate more visible change?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, we're working on that. right now it is a bit of a mess which model a conversation will use. we're working on making this a real property of the session so that if you change the global settings, it won't change it for existing conversations.

either way, changing the default here, should only change which model is suggested when you select the provider

Copy link
Contributor Author

@Dan-Wuensch Dan-Wuensch Aug 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah ok! In that case I'll push up a commit for updating default. Want to use latest Sonnet version?

@Dan-Wuensch
Copy link
Contributor Author

Demo of new default AWS Bedrock model recommendation:

Screenshot 2025-08-25 at 12 46 00 PM

@Dan-Wuensch
Copy link
Contributor Author

Dan-Wuensch commented Aug 25, 2025

Tangential thought: Does BEDROCK_KNOWN_MODELS affect any validation of the user's model entry? We could look at including some models from other vendors than Anthropic at some point. Not a big concern to me as long as it will accept pasting model IDs from outside the list, which it looks like it does to me.

@DOsinga
Copy link
Collaborator

DOsinga commented Aug 25, 2025

These are just suggested models. @angelahning is looking into showing the full list in a filterable way

@blackgirlbytes
Copy link
Contributor

@Dan-Wuensch there's a DCO check that's failing. Here's how to fix it 😄 https://github.com/block/goose/pull/4316/checks?check_run_id=48836635962

@Dan-Wuensch Dan-Wuensch force-pushed the feat/issue-4173-configurable-bedrock-retry branch from 05106c4 to 4174002 Compare August 25, 2025 19:51
@Dan-Wuensch
Copy link
Contributor Author

@blackgirlbytes thank you! 🙌 Just force pushed a rebase with the sign-off included for those three commits.

@michaelneale
Copy link
Collaborator

looking good a cargo fmt and maybe a script/cargo-lint.sh would help to check it is ok -

Signed-off-by: Dan Wuensch <[email protected]>
@Dan-Wuensch
Copy link
Contributor Author

Good catch! There was a piece in the model list that needed to be reformatted. Pushed up re-formatted now

@Dan-Wuensch
Copy link
Contributor Author

@blackgirlbytes checks and approvals are looking good! Except Semgrep appears to have gotten stuck. What's the next step in the process for these changes? 😄

@Dan-Wuensch Dan-Wuensch force-pushed the feat/issue-4173-configurable-bedrock-retry branch from 3db8773 to 994824b Compare September 3, 2025 15:03
@angiejones angiejones self-assigned this Sep 4, 2025

### Provider Retries

Configurable retry parameters for LLM providers.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!!

@angiejones angiejones merged commit 627696a into block:main Sep 4, 2025
11 checks passed
@Dan-Wuensch Dan-Wuensch deleted the feat/issue-4173-configurable-bedrock-retry branch September 4, 2025 17:01
michaelneale added a commit that referenced this pull request Sep 5, 2025
* main:
  feat: make tests for rmcp based developer server consistent with former implementation (#4519)
  worlds simplest logging to see where things are blocked (#3888)
  docs: update quickstart and install topics (#4378)
  feat: Add configurable Bedrock retry parameters (#4316)
  remove localstorage getconfig fallbacks (#4432)
  chore(deps-dev): bump electron from 37.2.6 to 37.4.0 in /ui/desktop (#4516)
  Fix databricks streaming errors  (#4506)
  docs: malware check for uvx and npx extensions (#4508)
katzdave added a commit that referenced this pull request Sep 8, 2025
* 'main' of github.com:block/goose:
  docs: add ampersand to link (#4560)
  Add video link to README for user guidance (#4553)
  docs: social channels (#4552)
  feat: simplify navigation, make reload work (#4498)
  docs: new recipe warning (#4545)
  Add AGENTS.md for AI coding assistant support (#4539)
  docs: non-interactive compact now (#4543)
  fixed css classes and added some accessibility fixes (#4492)
  feat(acp): Read files (#4531)
  Add YouTube Short to Auto Visualiser Tutorial (#4536)
  Fix/settings page (#4520)
  update to RMCP 0.6.2 (#4523)
  docs: nested goosehints (#4528)
  feat: Agent Client Protocol implementation of goose (#4511)
  feat: make tests for rmcp based developer server consistent with former implementation (#4519)
  worlds simplest logging to see where things are blocked (#3888)
  docs: update quickstart and install topics (#4378)
  feat: Add configurable Bedrock retry parameters (#4316)
  remove localstorage getconfig fallbacks (#4432)
  chore(deps-dev): bump electron from 37.2.6 to 37.4.0 in /ui/desktop (#4516)
tiensi added a commit to tiensi/goose that referenced this pull request Sep 8, 2025
* main: (43 commits)
  feat: add auto-compact threshold configuration UI (block#4178)
  Add container detection to developer extension (block#4559)
  docs: add ampersand to link (block#4560)
  Add video link to README for user guidance (block#4553)
  docs: social channels (block#4552)
  feat: simplify navigation, make reload work (block#4498)
  docs: new recipe warning (block#4545)
  Add AGENTS.md for AI coding assistant support (block#4539)
  docs: non-interactive compact now (block#4543)
  fixed css classes and added some accessibility fixes (block#4492)
  feat(acp): Read files (block#4531)
  Add YouTube Short to Auto Visualiser Tutorial (block#4536)
  Fix/settings page (block#4520)
  update to RMCP 0.6.2 (block#4523)
  docs: nested goosehints (block#4528)
  feat: Agent Client Protocol implementation of goose (block#4511)
  feat: make tests for rmcp based developer server consistent with former implementation (block#4519)
  worlds simplest logging to see where things are blocked (block#3888)
  docs: update quickstart and install topics (block#4378)
  feat: Add configurable Bedrock retry parameters (block#4316)
  ...
This was referenced Sep 9, 2025
thebristolsound pushed a commit to thebristolsound/goose that referenced this pull request Sep 11, 2025
Signed-off-by: Dan Wuensch <[email protected]>
Co-authored-by: angiejones <[email protected]>
Signed-off-by: Matt Donovan <[email protected]>
HikaruEgashira pushed a commit to HikaruEgashira/goose that referenced this pull request Oct 3, 2025
Signed-off-by: Dan Wuensch <[email protected]>
Co-authored-by: angiejones <[email protected]>
Signed-off-by: HikaruEgashira <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add model retry values to config.yaml

7 participants