-
Notifications
You must be signed in to change notification settings - Fork 831
Add provider info to docs #257
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
Cross-SDK Documentation Consistency ReviewThis PR adds excellent comprehensive documentation for custom providers (BYOK) to Node.js and Go SDKs, including:
📋 Inconsistencies FoundThe Python and .NET SDKs are missing this same level of documentation: Python SDK (
.NET SDK (
💡 RecommendationTo maintain cross-SDK consistency, consider adding similar "Custom Providers" documentation sections to:
The feature appears to be implemented in the code already (since .NET has a basic BYOK example), so this is purely a documentation gap. Note: This is a suggestion to improve documentation consistency. The changes in this PR are well-written and should be merged regardless—the Python/.NET docs can be updated in a follow-up PR.
|
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
Updates the Node.js and Go SDK READMEs to document BYOK/custom provider support and how to configure provider settings.
Changes:
- Document
providerconfiguration on session creation (and resume for Go), including notes about model selection. - Add “Custom Providers” sections describing
ProviderConfigfields and defaults. - Provide example configurations for Ollama, generic OpenAI-compatible endpoints, and Azure OpenAI.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| nodejs/README.md | Adds provider to session config docs and introduces a Custom Providers section with ProviderConfig fields + TS examples. |
| go/README.md | Expands API reference to include SessionConfig/ResumeSessionConfig provider details and adds a Custom Providers section with Go examples. |
Comments suppressed due to low confidence (1)
go/README.md:114
- Similarly,
ResumeSessionConfigin the Go SDK includes more thanTools/Provider/Streaming(e.g.,OnPermissionRequest,MCPServers,CustomAgents,SkillDirectories,DisabledSkills). Please either document the remaining options or note that only the most common fields are listed here.
**ResumeSessionConfig:**
- `Tools` ([]Tool): Tools to expose when resuming
- `Provider` (\*ProviderConfig): Custom API provider configuration (BYOK). See [Custom Providers](#custom-providers) section.
- `Streaming` (bool): Enable streaming delta events
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - `apiKey?: string` - API key (optional for local providers like Ollama) | ||
| - `bearerToken?: string` - Bearer token for authentication (takes precedence over apiKey) | ||
| - `wireApi?: "completions" | "responses"` - API format for OpenAI/Azure (default: "completions") | ||
| - `azure?.apiVersion?: string` - Azure API version (default: "2024-10-21") |
Copilot
AI
Jan 29, 2026
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.
In the ProviderConfig field list, azure?.apiVersion looks like TypeScript optional-chaining syntax rather than the actual config shape. Consider documenting this as azure.apiVersion?: string (or azure?: { apiVersion?: string }) to match the ProviderConfig type and avoid confusion.
| - `azure?.apiVersion?: string` - Azure API version (default: "2024-10-21") | |
| - `azure?: { apiVersion?: string }` - Azure-specific configuration (default apiVersion: "2024-10-21") |
| **SessionConfig:** | ||
|
|
||
| - `Model` (string): Model to use ("gpt-5", "claude-sonnet-4.5", etc.). **Required when using custom provider.** | ||
| - `SessionID` (string): Custom session ID | ||
| - `Tools` ([]Tool): Custom tools exposed to the CLI |
Copilot
AI
Jan 29, 2026
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.
The new SessionConfig section reads like an exhaustive field list, but it omits several fields that exist on copilot.SessionConfig (e.g., ConfigDir, AvailableTools, ExcludedTools, OnPermissionRequest, MCPServers, CustomAgents, SkillDirectories, DisabledSkills). Either include the missing fields or clarify that this list is partial.
| session, err := client.CreateSession(&copilot.SessionConfig{ | ||
| Model: "deepseek-coder-v2:16b", // Required when using custom provider | ||
| Provider: &copilot.ProviderConfig{ | ||
| Type: "openai", | ||
| BaseURL: "http://localhost:11434/v1", // Ollama endpoint |
Copilot
AI
Jan 29, 2026
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.
These Go snippets declare session, err := ... but don’t use either value or handle the error, which makes the example not compile as-is. Consider adding minimal error handling and using session (or assign to _), and include any needed imports for os.Getenv in the subsequent examples.
No description provided.