Simplify request logger a bit #1755
Conversation
Remove postOptions, since we should log params from the real request, and duplicated 'tools'.
There was a problem hiding this comment.
Pull Request Overview
This pull request refactors how request parameters are logged by consolidating tools and postOptions fields from ILoggedPendingRequest into the existing body field. This simplification reduces redundancy since tools and other request options were already present in the body (of type IEndpointBody).
Key changes:
- Removed the
toolsfield fromILoggedPendingRequestinterface and its import dependencies - Removed the redundant
postOptionsparameter from logged chat requests - Updated all logging call sites to pass request options within the
bodyfield instead - Updated all consumers to read from
body.tools,body.max_tokens, andbody.predictioninstead of separate top-level fields
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/platform/requestLogger/node/requestLogger.ts | Removed tools field from ILoggedPendingRequest interface and cleaned up unused imports |
| src/platform/endpoint/vscode-node/extChatEndpoint.ts | Changed to pass requestOptions within body object instead of separate postOptions and tools fields |
| src/extension/prompt/vscode-node/requestLoggerImpl.ts | Removed unused ILoggedPendingRequest import and updated all references to access tools/prediction/max_tokens through body field |
| src/extension/prompt/node/chatMLFetcher.ts | Removed redundant postOptions and tools parameters from logged request, keeping only body |
| src/extension/conversation/vscode-node/feedbackReporter.ts | Updated to access tools through body.tools instead of top-level tools field |
| src/extension/byok/vscode-node/geminiNativeProvider.ts | Wrapped tools array in body object when logging requests |
| src/extension/byok/vscode-node/anthropicProvider.ts | Wrapped tools array in body object when logging requests |
| body: { | ||
| tools: options.tools?.map((tool): OpenAiFunctionTool => ({ | ||
| type: 'function', | ||
| function: { | ||
| name: tool.name, | ||
| description: tool.description, | ||
| parameters: tool.inputSchema | ||
| } | ||
| })) | ||
| }, |
There was a problem hiding this comment.
The body object only contains the tools field but could contain additional request parameters that may already be present in options. Consider whether other relevant fields from the request options should also be included in the body object for logging consistency (e.g., max_tokens, temperature, prediction). This would ensure comprehensive logging of the request parameters.
| description: tool.description, | ||
| parameters: tool.inputSchema | ||
| } | ||
| })) |
There was a problem hiding this comment.
Similar to the Anthropic provider, the body object only contains the tools field. Consider whether other relevant fields from the request options should also be included in the body object for logging consistency (e.g., max_tokens, temperature, prediction). This would ensure comprehensive logging of the request parameters across all providers.
| })) | |
| })), | |
| max_tokens: options.maxTokens, | |
| temperature: options.temperature, | |
| prediction: options.prediction |
Remove postOptions, since we should log params from the real request, and duplicated 'tools'.