Skip to content

Conversation

@lifeizhou-ap
Copy link
Collaborator

@lifeizhou-ap lifeizhou-ap commented Jan 5, 2026

Summary

  • Root cause: usage is not set in the response data with the streaming models.
  • Fix: Introduced the stream function to be consistent to the other providers. Deleted unused file.

Type of Change

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

AI Assistance

  • This PR was created or reviewed with AI assistance

Testing

  • Manual testing
  • Smoke Test

Related Issues

Relates to #ISSUE_ID: #5907
Discussion: LINK (if any)

.client
.post(url)
.headers(headers)
.header("Authorization", format!("Bearer {}", token));
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this right? we have authorization types for the client, we shouldn't be setting this by hand I would think

@lifeizhou-ap lifeizhou-ap marked this pull request as ready for review January 6, 2026 04:04
Copilot AI review requested due to automatic review settings January 6, 2026 04:04
Copy link
Contributor

Copilot AI left a 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 fixes a bug where usage statistics were not being set correctly in response data when using streaming models with the GitHub Copilot provider. The fix standardizes the provider's implementation to match patterns used by other providers.

  • Refactored the post method to return Response instead of Value and use ApiClient
  • Added stream method to support proper streaming functionality
  • Added supports_streaming method to identify models that support streaming

stream_openai_compat(response, log)
}

/// Fetch supported models from GitHub Copliot; returns Err on failure, Ok(None) if not present
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

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

Typo in comment: "Copliot" should be "Copilot".

Suggested change
/// Fetch supported models from GitHub Copliot; returns Err on failure, Ok(None) if not present
/// Fetch supported models from GitHub Copilot; returns Err on failure, Ok(None) if not present

Copilot uses AI. Check for mistakes.
Copilot AI review requested due to automatic review settings January 6, 2026 04:11
Copy link
Contributor

Copilot AI left a 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 3 out of 3 changed files in this pull request and generated no new comments.

@lifeizhou-ap lifeizhou-ap merged commit a842c99 into main Jan 6, 2026
24 checks passed
@lifeizhou-ap lifeizhou-ap deleted the lifei/fix-copilot-provider-token branch January 6, 2026 04:28
zanesq added a commit that referenced this pull request Jan 6, 2026
* '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)
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.

3 participants