Send context signals in routing predict request#4267
Send context signals in routing predict request#4267aashna wants to merge 1 commit intomicrosoft:mainfrom
Conversation
📬 CODENOTIFYThe following users are being notified based on files changed in this PR: @bhavyausMatched files:
@bryanchen-dMatched files:
@vijayupadyaMatched files:
@pwang347Matched files:
@karthiknadigMatched files:
@eleanorjboydMatched files:
|
There was a problem hiding this comment.
Pull request overview
This PR forwards additional optional “context signals” through the Claude/Copilot plumbing to enable richer routing/model decisions in later versions, and adds supporting infrastructure around Claude sessions, agents, and MCP contributions.
Changes:
- Add/propagate new Claude session metadata/state (folder info, usage handler, sessionId extraction) and improve session discovery/parsing.
- Introduce MCP server contributor registry + initial IDE diagnostics MCP tool and tests.
- Expand agent/prompt infrastructure (Ask/Explore/Edit agents), plus build/test tooling updates (proposed d.ts checks, postinstall copies).
Reviewed changes
Copilot reviewed 115 out of 910 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/extension/chatSessions/claude/node/sessionParser/claudeCodeSessionService.ts | Refactors project folder discovery, session URI creation, and subagent loading for better session parsing/discovery. |
| src/extension/chatSessions/claude/node/mcpServers/index.ts | Adds node MCP server index to trigger contributor self-registration. |
| src/extension/chatSessions/claude/node/hooks/loggingHooks.ts | Simplifies Claude hook logging and removes capturing-token grouping logic. |
| src/extension/chatSessions/claude/node/claudeSessionTitleService.ts | Adds service to persist custom session titles into JSONL files. |
| src/extension/chatSessions/claude/node/claudeSessionStateService.ts | Extends per-session state (folder info, usage handler) and makes model lookup synchronous. |
| src/extension/chatSessions/claude/node/claudeProjectFolders.ts | Adds shared project folder discovery/slug computation helper. |
| src/extension/chatSessions/claude/node/claudeLanguageModelServer.ts | Adds sessionId extraction, request capture logging, beta-header filtering, and usage reporting. |
| src/extension/chatSessions/claude/node/claudeCodeModels.ts | Adds dynamic model provider registration and endpoint caching/refresh handling. |
| src/extension/chatSessions/claude/common/toolPermissionHandlers/index.ts | Registers the new AskUserQuestion permission handler module. |
| src/extension/chatSessions/claude/common/toolPermissionHandlers/askUserQuestionHandler.ts | Switches AskUserQuestion handling to the core askQuestions tool invocation path. |
| src/extension/chatSessions/claude/common/toolInvocationFormatter.ts | Updates tool invocation formatting/completion, adds subagent task formatting support. |
| src/extension/chatSessions/claude/common/test/ideMcpServer.spec.ts | Adds unit tests for the IDE diagnostics MCP handler. |
| src/extension/chatSessions/claude/common/mcpServers/index.ts | Adds common MCP server index and registers ide MCP server contributor. |
| src/extension/chatSessions/claude/common/mcpServers/ideMcpServer.ts | Adds IDE MCP server contributor + diagnostics tool and handler formatting. |
| src/extension/chatSessions/claude/common/claudeSessionUri.ts | Adds helper for standardized Claude session URIs. |
| src/extension/chatSessions/claude/common/claudeMcpServerRegistry.ts | Adds MCP server contributor registry and DI-based builder. |
| src/extension/chatSessions/claude/common/claudeFolderInfo.ts | Introduces ClaudeFolderInfo interface used for deterministic cwd/dir handling. |
| src/extension/chatSessions/claude/AGENTS.md | Documents folder mgmt, tool handlers, and MCP registry approach for Claude integration. |
| src/extension/chat/vscode-node/hooksOutputChannel.ts | Adds output channel implementation for chat hooks logging. |
| src/extension/chat/vscode-node/chatHookTelemetry.ts | Adds telemetry for configured/executed hooks and hook results. |
| src/extension/byok/vscode-node/test/geminiNativeProvider.spec.ts | Updates tests for new OTel service parameter in provider constructor. |
| src/extension/byok/vscode-node/ollamaProvider.ts | Adjusts known-model caching and switches to OpenAIEndpoint creation. |
| src/extension/byok/vscode-node/customOAIProvider.ts | Adds optional streaming capability flag to custom OAI model config. |
| src/extension/byok/vscode-node/azureProvider.ts | Passes through optional streaming capability flag to model info. |
| src/extension/byok/node/test/openAIEndpoint.spec.ts | Updates test model metadata to include vendor field. |
| src/extension/byok/node/test/azureOpenAIEndpoint.spec.ts | Updates test model metadata to include vendor field. |
| src/extension/byok/node/openAIEndpoint.ts | Wires in chat web socket manager and simplifies request retry behavior. |
| src/extension/byok/common/byokProvider.ts | Extends capabilities (adaptive thinking/streaming), adds vendor and API info tweaks. |
| src/extension/agents/vscode-node/test/planAgentProvider.spec.ts | Updates plan agent tests for new core default model setting and tool list changes. |
| src/extension/agents/vscode-node/test/mockOctoKitService.ts | Extends mock OctoKit service with outage status support. |
| src/extension/agents/vscode-node/test/githubOrgInstructionsProvider.spec.ts | Skips several polling-related tests. |
| src/extension/agents/vscode-node/test/githubOrgCustomAgentProvider.spec.ts | Updates agent frontmatter flags and adds tests for new boolean properties; skips some change-event tests. |
| src/extension/agents/vscode-node/test/githubOrgChatResourcesService.spec.ts | Skips polling suite. |
| src/extension/agents/vscode-node/test/askAgentProvider.spec.ts | Adds unit tests for newly introduced AskAgentProvider behavior. |
| src/extension/agents/vscode-node/promptFileContrib.ts | Registers Ask/Explore agents and conditionally registers Edit Mode agent. |
| src/extension/agents/vscode-node/githubOrgCustomAgentProvider.ts | Changes agent frontmatter fields to disable-model-invocation/user-invocable flags. |
| src/extension/agents/vscode-node/githubOrgChatResourcesService.ts | Disables periodic polling logic (commented out), leaving only initial poll. |
| src/extension/agents/vscode-node/exploreAgentProvider.ts | Adds Explore agent provider with fallback model list and generated body. |
| src/extension/agents/vscode-node/editModeAgentProvider.ts | Adds Edit Mode agent provider with strict allowlist editing rules and handoff. |
| src/extension/agents/vscode-node/askAgentProvider.ts | Adds Ask agent provider with read-only tools and settings customization. |
| src/extension/agents/vscode-node/agentTypes.ts | Introduces shared agent config/types and YAML frontmatter builder. |
| src/extension/agents/vscode-node/agentCustomizationSkillProvider.ts | Simplifies provider wiring, removes config/experimentation gating, and updates reference path comments. |
| src/extension/agents/node/adapters/anthropicAdapter.ts | Updates Anthropic adapter event shapes (caller/container fields, usage typing). |
| src/extension/agents/copilotcli/vscode-node/contribution.ts | Removes unused Copilot CLI contribution class. |
| src/extension/agents/copilotcli/node/userInputHelpers.ts | Removes legacy user input helper conversion utilities. |
| src/extension/agents/copilotcli/node/test/permissionHelpers.spec.ts | Removes Copilot CLI permission helper tests. |
| src/extension/agents/claude/node/test/claudeSessionStateService.spec.ts | Removes Claude session state tests (old location). |
| src/extension/agents/claude/node/test/claudeCodeAgent.spec.ts | Removes Claude agent tests (old location). |
| src/extension/agentDebug/vscode-node/toolResultContentRenderer.ts | Adds renderer to stringify tool result parts without expensive TSX rendering. |
| src/extension/agentDebug/node/agentDebugEventServiceImpl.ts | Adds ring-buffer-based debug event service implementation. |
| src/extension/agentDebug/common/toolResultRenderer.ts | Adds common interface for tool result rendering across layers. |
| src/extension/agentDebug/common/agentDebugViewLogic.ts | Adds framework-agnostic logic for agent debug panel (tree building/filtering/formatting). |
| src/extension/agentDebug/common/agentDebugTypes.ts | Adds serializable debug event type contracts for future API. |
| src/extension/agentDebug/common/agentDebugEventService.ts | Adds debug event service interface mirroring future provider shape. |
| script/postinstall.ts | Copies Copilot CLI definition files into the SDK folder during postinstall. |
| script/build/vscodeDtsUpdate.js | Adds script to update proposed VS Code d.ts files at a resolved commit SHA. |
| script/build/vscodeDtsCheck.js | Adds script/CI check to ensure proposed VS Code d.ts files are up to date. |
| script/build/moveProposedDts.js | Adds helper to move downloaded vscode.*.ts files into src/extension. |
| script/applyLocalDts.sh | Replaces local-copy flow with guidance to use the new dts update workflow. |
| docs/monitoring/otel-collector-config.yaml | Adds OTel collector config for local monitoring (App Insights + Jaeger). |
| docs/monitoring/docker-compose.yaml | Adds docker compose stack for local OTel collector + Jaeger. |
| chat-lib/package.json | Bumps copilot-api, adds undici, bumps anthropic sdk version. |
| build/release.yml | Enables publishing VSIX to GitHub releases. |
| build/pre-release.yml | Enables publishing VSIX to GitHub releases for pre-release. |
| assets/prompts/skills/project-setup-info-local/SKILL.md | Adds project setup skill reference (local) including MCP server guidance. |
| assets/prompts/skills/project-setup-info-context7/SKILL.md | Adds project setup skill reference using context7 tools. |
| assets/prompts/skills/install-vscode-extension/SKILL.md | Adds skill doc for installing VS Code extensions via command tooling. |
| assets/prompts/skills/get-search-view-results/SKILL.md | Adds skill doc for extracting Search view results via VS Code command. |
| assets/prompts/skills/agent-customization/references/workspace-instructions.md | Adds guidance on nested AGENTS.md usage for monorepos. |
| assets/prompts/skills/agent-customization/references/skills.md | Updates skills reference (locations, frontmatter options, slash behavior). |
| assets/prompts/skills/agent-customization/references/prompts.md | Updates prompts reference (frontmatter options, fallback models, precedence). |
| assets/prompts/skills/agent-customization/references/instructions.md | Updates instruction reference (applyTo examples and cautions). |
| assets/prompts/skills/agent-customization/references/hooks.md | Adds hooks reference documentation. |
| assets/prompts/skills/agent-customization/references/agents.md | Updates agents reference and frontmatter field naming/typos. |
| assets/prompts/skills/agent-customization/SKILL.md | Updates agent-customization skill content and references to new docs structure. |
| assets/prompts/savePrompt.prompt.md | Removes legacy savePrompt prompt. |
| assets/prompts/init.prompt.md | Updates init prompt workflow to reference agent-customization and templates. |
| assets/prompts/create-skill.prompt.md | Adds prompt to guide creation of a SKILL.md. |
| assets/prompts/create-prompt.prompt.md | Adds prompt to guide creation of a .prompt.md. |
| assets/prompts/create-instructions.prompt.md | Adds prompt to guide creation of a .instructions.md. |
| assets/prompts/create-hook.prompt.md | Adds prompt to guide creation of a hooks .json file. |
| assets/prompts/create-agent.prompt.md | Adds prompt to guide creation of a .agent.md. |
| README.md | Updates README messaging and links/images to emphasize autonomous agents and customization. |
| CONTRIBUTING.md | Improves Windows setup instructions formatting and shortcut clarity. |
| .vscodeignore | Ensures Copilot SDK definition YAML files are included in packaged extension. |
| .vscode/settings.json | Removes setting enabling generateTests codeLens. |
| .vscode/mcp.json | Adds local MCP server wiring for vscode-playwright-mcp in dev environment. |
| .github/workflows/pr.yml | Adds CI step to ensure proposed API types are up to date. |
| .github/prompts/updateGithubCopilotSDK.prompt.md | Adds upgrade prompt for the @github/copilot package update workflow. |
| .github/prompts/updateCopilotCLIToolMapping.prompt.md | Adds prompt for updating Copilot CLI tool mapping/types/tests. |
| .claude/skills/launch | Adds link to shared launch skill under .agents/skills. |
| .claude/agents/anthropic-sdk-upgrader.md | Enhances anthropic sdk upgrader instructions with API surface diffing step. |
| .agents/skills/launch/SKILL.md | Adds a detailed skill doc for automating VS Code Insiders via agent-browser. |
Files not reviewed (1)
- chat-lib/package-lock.json: Language not supported
| const invocation = new ChatToolInvocationPart(toolUse.name, toolUse.id, false as unknown as string); | ||
| if (complete !== undefined) { | ||
| invocation.isConfirmed = complete; | ||
| invocation.isComplete = complete; | ||
| } |
There was a problem hiding this comment.
The ChatToolInvocationPart constructor call is using an unsafe cast (false as unknown as string). This is likely masking an actual signature mismatch and can cause runtime issues if the 3rd parameter is not the expected type. Update this to match the real constructor signature (e.g., pass the correct typed value or omit the parameter if optional) and remove the cast; then keep isConfirmed/isComplete assignments consistent with the API’s expected types.
| const filtered = headerValue | ||
| .split(',') | ||
| .map(b => b.trim()) | ||
| .filter(b => b && SUPPORTED_ANTHROPIC_BETAS.some(supported => b.startsWith(supported + '-'))); |
There was a problem hiding this comment.
The implementation only allows beta entries with a version suffix (e.g. context-management-YYYY-MM-DD) but rejects exact matches like context-management, even though the comment/docstring describes prefix-based allowlisting. Consider accepting both exact matches and version-suffixed values (e.g., b === supported || b.startsWith(supported + '-')) or update the documentation to reflect the stricter behavior.
| .filter(b => b && SUPPORTED_ANTHROPIC_BETAS.some(supported => b.startsWith(supported + '-'))); | |
| .filter(b => b && SUPPORTED_ANTHROPIC_BETAS.some(supported => b === supported || b.startsWith(supported + '-'))); |
| } | ||
|
|
||
| const nonce = apiKey.slice(0, dotIndex); | ||
| const sessionId = apiKey.slice(dotIndex + 1); |
There was a problem hiding this comment.
If the header value is nonce. (trailing dot), sessionId becomes the empty string and is treated as a valid sessionId. That can lead to state collisions (e.g., storing per-session state under an empty key). Consider validating that sessionId is non-empty (and possibly well-formed) before returning it; otherwise return sessionId: undefined (legacy behavior) or valid: false.
| const sessionId = apiKey.slice(dotIndex + 1); | |
| const sessionId = apiKey.slice(dotIndex + 1); | |
| // Reject keys with a trailing dot and no session ID (e.g. "nonce.") | |
| if (!sessionId) { | |
| return { valid: false, sessionId: undefined }; | |
| } |
| if (args.uri) { | ||
| let fileUri: URI; | ||
| try { | ||
| fileUri = URI.parse(args.uri); | ||
| } catch { | ||
| throw new Error(`Invalid URI: "${args.uri}". Expected an absolute path (e.g., /path/to/file.ts) or a URI with a scheme (e.g., file:///path/to/file.ts, untitled:Untitled-1).`); | ||
| } | ||
| entries = [[fileUri, diagnosticsService.getDiagnostics(fileUri)]]; | ||
| } else { | ||
| entries = diagnosticsService.getAllDiagnostics(); | ||
| } |
There was a problem hiding this comment.
The error message claims absolute paths are accepted, but URI.parse('/abs/path') produces a URI with an empty scheme rather than a file URI, which typically won’t match keys used by diagnostics services (commonly file://...). Consider detecting bare absolute paths and converting them via URI.file(args.uri) (and handling Windows-style paths as well) so uri: \"/path/to/file\" behaves as documented.
| export function extractSessionId(headers: http.IncomingHttpHeaders, expectedNonce: string): ExtractSessionIdResult { | ||
| let apiKey: string | undefined; | ||
|
|
||
| // Check x-api-key header (used by SDK) | ||
| const apiKeyHeader = headers['x-api-key']; |
There was a problem hiding this comment.
New parsing utilities (extractSessionId, filterSupportedBetas) introduce non-trivial edge cases (missing headers, legacy nonce-only format, malformed nonce.sessionId, exact vs versioned beta values). Add focused Vitest unit tests to lock down expected behavior and prevent regressions.
| @@ -89,7 +89,7 @@ function isValidFile(type: PromptsType, fileName: string): boolean { | |||
| export class GitHubOrgChatResourcesService extends Disposable implements IGitHubOrgChatResourcesService { | |||
| private static readonly CACHE_ROOT = 'github'; | |||
|
|
|||
| private readonly _pollingSubscriptions = this._register(new DisposableStore()); | |||
| // private readonly _pollingSubscriptions = this._register(new DisposableStore()); | |||
There was a problem hiding this comment.
The polling infrastructure is partially disabled via commented-out code, leaving startPolling to do a single initial poll but not track/own the returned disposables at the service level. Consider either (1) restoring the interval + proper disposal registration, or (2) refactoring the API to reflect one-shot behavior (and removing the dead/commented code) so callers have clear lifecycle expectations.
| // private readonly _pollingSubscriptions = this._register(new DisposableStore()); |
| // TODO: re-enable polling | ||
| // Set up interval polling | ||
| const intervalId = setInterval(() => poll(), intervalMs); | ||
| disposables.add(toDisposable(() => clearInterval(intervalId))); | ||
| // const intervalId = setInterval(() => poll(), intervalMs); | ||
| // disposables.add(toDisposable(() => clearInterval(intervalId))); | ||
|
|
||
| this._pollingSubscriptions.add(disposables); | ||
| // this._pollingSubscriptions.add(disposables); |
There was a problem hiding this comment.
The polling infrastructure is partially disabled via commented-out code, leaving startPolling to do a single initial poll but not track/own the returned disposables at the service level. Consider either (1) restoring the interval + proper disposal registration, or (2) refactoring the API to reflect one-shot behavior (and removing the dead/commented code) so callers have clear lifecycle expectations.
| @@ -0,0 +1,101 @@ | |||
| --- | |||
| name: updateGithubCopilotSDK | |||
| description: Use this to update the Github Copilot CLI/SDK | |||
There was a problem hiding this comment.
Correct capitalization of 'Github' to 'GitHub'.
| description: Use this to update the Github Copilot CLI/SDK | |
| description: Use this to update the GitHub Copilot CLI/SDK |
aashna
force-pushed
the
aashnagarg/add-routing-context-signals
branch
from
c1eddff to
bd7b13f
Compare
| const routedModel = await this._routerDecisionFetcher.getRoutedModel(prompt, availableModels, preferredModels); | ||
| // Build context signals for the router to enable richer future routing decisions | ||
| const contextSignals: RoutingContextSignals = { | ||
| chat_location: chatRequestLocationToString(chatRequest?.location), |
There was a problem hiding this comment.
chat_location will be omitted for panel chat because chatRequest.location is often undefined (panel is treated as the default earlier in this method). This makes the new signal inconsistent with the documented values (panel/editor/terminal). Consider mapping undefined to 'panel' (and also mapping ChatLocation.Other to a stable string like 'other' instead of String(location) which can become a numeric enum value).
| chat_location: chatRequestLocationToString(chatRequest?.location), | |
| chat_location: | |
| location === ChatLocation.Panel ? 'panel' : | |
| location === ChatLocation.Editor ? 'editor' : | |
| location === ChatLocation.Terminal ? 'terminal' : | |
| location === ChatLocation.Other ? 'other' : | |
| undefined, |
| reference_count: chatRequest?.references?.length, | ||
| prompt_char_count: prompt.length, |
There was a problem hiding this comment.
reference_count is currently chatRequest?.references?.length, which will include image attachments (and potentially other non-#file/@workspace references). If this signal is intended to represent only #file/@workspace references (per PR description), filter out image references at least (e.g., exclude refs whose value has an image/* mimeType), and ideally count only the relevant reference kinds.
| turn_number: entry ? (entry.endpoints.length > 0 ? 2 : 1) : 1, | ||
| }; |
There was a problem hiding this comment.
turn_number is derived from entry.endpoints.length, which is a cache of instantiated endpoints (and in practice will make turn_number stick at 1 for the first request and 2 for all subsequent requests). If this is meant to be the conversation turn index, compute it from the chat history (e.g., number of prior user turns in chatRequest.history + 1) rather than from the endpoint cache.
| body: JSON.stringify({ prompt: query, available_models: availableModels, preferred_models: preferredModels, ...contextSignals }) | ||
| }); |
There was a problem hiding this comment.
New routing context fields are now forwarded via the request body, but RouterDecisionFetcher tests only assert the base { prompt, available_models, preferred_models } payload. Add a unit test that calls getRoutedModel(..., contextSignals) and asserts the serialized body includes the expected signal fields and omits undefined values, to prevent regressions in signal forwarding.
aashna
force-pushed
the
aashnagarg/add-routing-context-signals
branch
3 times, most recently
from
7a101ec to
4918148
Compare
Add RoutingContextSignals interface with optional fields (has_image, chat_location, turn_number, session_id, previous_model, reference_count, prompt_char_count) and spread them into the router request body. AutomodeService populates signals from the ChatRequest context before each routing call.
aashna
force-pushed
the
aashnagarg/add-routing-context-signals
branch
from
4918148 to
ccb8a4d
Compare
| try { | ||
| const result = await this._routerDecisionFetcher.getRouterDecision(prompt, token.session_token, token.available_models); | ||
| const contextSignals: RoutingContextSignals = { | ||
| chat_location: chatRequestLocationToString(chatRequest?.location), |
There was a problem hiding this comment.
The router always runs in panel right now
| const contextSignals: RoutingContextSignals = { | ||
| chat_location: chatRequestLocationToString(chatRequest?.location), | ||
| session_id: conversationId !== 'unknown' ? conversationId : undefined, | ||
| has_image: hasImage(chatRequest), |
There was a problem hiding this comment.
We will never run the router with images so this will always be false
| reference_count: chatRequest?.references?.length, | ||
| prompt_char_count: prompt.length, | ||
| previous_model: entry?.endpoint?.model, | ||
| turn_number: entry ? 2 : 1, |
There was a problem hiding this comment.
this can only be 2 or 1? Can't there be 100 turns
Forward optional context fields from the client to the routing service to enable richer routing decisions in future model versions.
Signals sent:
All fields are optional and the server silently ignores unknown fields. Current models do not use these signals.