Skip to content

Conversation

@lifeizhou-ap
Copy link
Contributor

@lifeizhou-ap lifeizhou-ap commented Oct 22, 2025

Summary

Issue: When specifying temperature with goose-gpt-5 model (eg: run a recipe), the session exits with "bad request error"
Why: gpt-5 only allows default (1) temperature
Fix: Ignore the temperature for goose-gpt-5 (same thing has been handled in the openai)

Type of Change

  • Feature
  • Bug fix
  • Refactor / Code quality
  • Performance improvement
  • Documentation
  • Tests
  • Security fix
  • Build / Release
  • Other (specify below)

@lifeizhou-ap lifeizhou-ap changed the title fix: Skip temperature for goose-gpt-5 model fix: skip temperature for goose-gpt-5 model Oct 22, 2025
@jsibbison-square
Copy link
Contributor

@lifeizhou-ap
Copy link
Contributor Author

lifeizhou-ap commented Oct 22, 2025

We should make the same change for the openai provider too: https://github.com/mnicholson110/goose/blob/88947546c8ef64f70bb2ea60c7b2774881f2db84/crates/goose/src/providers/formats/openai.rs#L625

Hi @jsibbison-square,
In the main branch, as I mentioned in this PR description, the openai provider has already been handled the gpt-5 for skipping temperature.

|| model_config.model_name.starts_with("gpt-5");

Copy link
Collaborator

@alexhancock alexhancock left a comment

Choose a reason for hiding this comment

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

nice!

}
}

// o1 models use max_completion_tokens instead of max_tokens
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: update this comment too

}

// o1 models use max_completion_tokens instead of max_tokens
// open ai reasoning models use max_completion_tokens instead of max_tokens
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we need this at all? in fact shouldn't we just check whether we have max_tokens in there and if not check if there is max_completion_tokens in there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

By reading the open ai doc here, max_token is deprecated.

We may have a separate PR to use max_completion_tokens and test it.

@lifeizhou-ap lifeizhou-ap merged commit e8e8991 into main Oct 22, 2025
14 checks passed
@lifeizhou-ap lifeizhou-ap deleted the lifei/ignore-temperature-for-gpt5 branch October 22, 2025 22:52
wpfleger96 added a commit that referenced this pull request Oct 23, 2025
* main:
  Add tech-article-explainer recipe (#5333)
  doc: added beta banner for old blog post (#5332)
  feat: add code refactor recipe (#5320)
  Security audit recipe (#5319)
  feat: add generate commit message recipe (#5326)
  fix: remove dependency on gsap library (#5330)
  feat: dynamically load ollama models (#5309)
  fix: skip temperature for goose-gpt-5 model (#5311)
katzdave added a commit that referenced this pull request Oct 23, 2025
…message

# By Better-Boy (3) and others
# Via GitHub
* 'main' of github.com:block/goose:
  Create recipe dir on save (#5337)
  docs: Update Discord link (#5335)
  [recipe workflow]: Fix `Invalid revision range` error  (#5334)
  Add tech-article-explainer recipe (#5333)
  doc: added beta banner for old blog post (#5332)
  feat: add code refactor recipe (#5320)
  Security audit recipe (#5319)
  feat: add generate commit message recipe (#5326)
  fix: remove dependency on gsap library (#5330)
  feat: dynamically load ollama models (#5309)
  fix: skip temperature for goose-gpt-5 model (#5311)
  Replace compaction notifications with system notifications (#5218)

# Conflicts:
#	crates/goose/src/agents/agent.rs
michaelneale added a commit that referenced this pull request Oct 24, 2025
* main: (77 commits)
  Fix legacy import (#5343)
  Unify loading goose messages and usechatstream determines chat state (#5306)
  Docs: goose session export and goose session import (#5267)
  Create recipe dir on save (#5337)
  docs: Update Discord link (#5335)
  [recipe workflow]: Fix `Invalid revision range` error  (#5334)
  Add tech-article-explainer recipe (#5333)
  doc: added beta banner for old blog post (#5332)
  feat: add code refactor recipe (#5320)
  Security audit recipe (#5319)
  feat: add generate commit message recipe (#5326)
  fix: remove dependency on gsap library (#5330)
  feat: dynamically load ollama models (#5309)
  fix: skip temperature for goose-gpt-5 model (#5311)
  Replace compaction notifications with system notifications (#5218)
  Diagnostics (#5323)
  Fix gemini again (#5308)
  fix: synchronize local message state after conversation compaction (#5315)
  docs: replace broken links with working links (#5266)
  Add Web Accessibility Auditor recipe to cookbook (#5318)
  ...
BlairAllan pushed a commit to BlairAllan/goose that referenced this pull request Oct 25, 2025
BlairAllan pushed a commit to BlairAllan/goose that referenced this pull request Nov 29, 2025
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.

5 participants