feat: add request trace for llm #1085
Conversation
WalkthroughAdded a DEV-only Trace Request Parameters feature that reconstructs and displays the API request (endpoint, headers, body) for an assistant message, including provider-specific reconstruction, redaction of sensitive fields, renderer UI (Trace button + dialog), and i18n strings across locales. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as MessageToolbar.vue
participant Relay as MessageList.vue
participant Dialog as TraceDialog.vue
participant Presenter as ThreadPresenter
participant Provider as LLMProvider
participant Redact as redact.ts
User->>UI: Click Trace button
UI->>Relay: emit('trace', messageId)
Relay->>Dialog: set traceMessageId / open
Dialog->>Presenter: getMessageRequestPreview(messageId)
Presenter->>Presenter: resolve message, model, tools
Presenter->>Provider: getRequestPreview(messages, modelId, ...)
Provider-->>Presenter: { endpoint, headers, body }
Presenter->>Redact: redactRequestPreview(...)
Redact-->>Presenter: redacted preview
Presenter-->>Dialog: preview payload
Dialog->>User: display JSON (Monaco)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Areas needing extra attention:
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (13)
docs/trace-request-params-feature.md (2)
10-55: Add language specifier to fenced code block.The ASCII diagram should have a language specifier (e.g.,
textorplaintext) to satisfy markdown linting rules and improve rendering consistency.Apply this diff:
-``` +```text ┌─────────────────────────────────────────────────────────────┐ │ Renderer Process │
233-253: Add language specifier to fenced code block.The UI layout diagram should have a language specifier (e.g.,
textorplaintext) for consistency with markdown best practices.Apply this diff:
-``` +```text ┌─────────────────────────────────────────────────┐ │ Request Preview [×] │src/renderer/src/i18n/en-US/settings.json (1)
45-45: Consider a more descriptive label.The translation "Trace Call" is concise but might benefit from a more explicit label like "Debug Request Trace" or "Trace API Requests" to clarify what is being traced.
src/shared/types/presenters/llmprovider.presenter.d.ts (1)
133-133: Consider stronger typing for the return value.The
unknownreturn type provides minimal type safety. If the provider instances share a common base type or interface, consider using that instead for better type checking and IDE support.However, if this flexibility is intentional for dev-tools use cases where different providers may have different shapes, the current approach is acceptable.
src/shared/types/presenters/thread.presenter.d.ts (1)
188-190: Consider stronger typing for dev tools consistency.Similar to
getProviderInstance, thePromise<unknown>return type provides minimal type safety. If the preview structure is consistent (e.g., based onProviderRequestPreviewmentioned in the AI summary), consider defining and using that type here for better type checking.However, for dev-tools features where flexibility is valued over strict typing, the current approach is acceptable.
src/main/presenter/llmProviderPresenter/baseProvider.ts (1)
652-684: Type the preview and avoid per-arg ESLint disables
- Return a shared ProviderRequestPreview type instead of an inline object.
- Replace repetitive no-unused-vars disables with void markers.
Apply:
- public async getRequestPreview( - // eslint-disable-next-line @typescript-eslint/no-unused-vars - _messages: ChatMessage[], - // eslint-disable-next-line @typescript-eslint/no-unused-vars - _modelId: string, - // eslint-disable-next-line @typescript-eslint/no-unused-vars - _modelConfig: ModelConfig, - // eslint-disable-next-line @typescript-eslint/no-unused-vars - _temperature: number, - // eslint-disable-next-line @typescript-eslint/no-unused-vars - _maxTokens: number, - // eslint-disable-next-line @typescript-eslint/no-unused-vars - _mcpTools: MCPToolDefinition[] - ): Promise<{ - endpoint: string - headers: Record<string, string> - body: unknown - }> { - // Default implementation returns not implemented marker - throw new Error('Provider has not implemented getRequestPreview') - } + public async getRequestPreview( + _messages: ChatMessage[], + _modelId: string, + _modelConfig: ModelConfig, + _temperature: number, + _maxTokens: number, + _mcpTools: MCPToolDefinition[] + ): Promise<ProviderRequestPreview> { + void _messages; void _modelId; void _modelConfig; void _temperature; void _maxTokens; void _mcpTools; + throw new Error('Provider has not implemented getRequestPreview') + }Add import (outside this hunk):
import type { ProviderRequestPreview } from '@shared/provider-operations'As per coding guidelines.
src/shared/types/presenters/legacy.presenters.d.ts (2)
727-728: Do not expose raw provider instances over the public contractReturning unknown weakens the API and may tempt renderer-side usage of a non-serializable instance. Either remove from the shared interface (keep internal to main) or return a precise main-only type and ensure it is not registered over IPC.
Would you confine this to main-only usage? If it must stay, prefer a narrow return (e.g., BaseLLMProvider | null) and mark it internal.
868-870: Strongly type the preview resultReplace Promise with a shared preview type.
Apply:
- getMessageRequestPreview(messageId: string): Promise<unknown> + getMessageRequestPreview(messageId: string): Promise<ProviderRequestPreview>and import:
import type { ProviderRequestPreview } from './provider-operations'As per coding guidelines (strict typing).
src/renderer/src/stores/settings.ts (2)
370-372: Avoid duplicate TRACE_DEBUG_CHANGED listener registrationsetupTraceDebugEnabledListener() is called in both initSettings and setupProviderListener; this registers two handlers.
Apply either removal; e.g., keep it in initSettings only:
- // 设置 Trace 调试功能事件监听器 - setupTraceDebugEnabledListener() + // Set Trace debug listener + setupTraceDebugEnabledListener()- // 设置 Trace 调试功能事件监听器 - setupTraceDebugEnabledListener() + // Trace debug listener is set during initSettingsAlso translate the comments to English (see above). As per coding guidelines.
Also applies to: 809-811
1710-1717: Add cleanup for TRACE_DEBUG_CHANGED and use English comments
- Registering a listener requires a matching removal on cleanup.
- Comments/logs should be in English.
Add to cleanup (outside this hunk):
window.electron?.ipcRenderer?.removeAllListeners(CONFIG_EVENTS.TRACE_DEBUG_CHANGED)And update this comment to English:
- /////////////////////////////////////////////////////////////////////////////////////// - const setupTraceDebugEnabledListener = () => { + /////////////////////////////////////////////////////////////////////////////////////// + const setupTraceDebugEnabledListener = () => {(keep English wording “Trace debug”). As per coding guidelines.
src/main/presenter/llmProviderPresenter/providers/openAIResponsesProvider.ts (1)
1328-1395: Harden preview: type it, pre‑redact auth, and refine Azure endpoint
- Return ProviderRequestPreview (or a local alias) instead of inline shape.
- Set Authorization: 'Bearer REDACTED' in the preview to avoid accidental leaks even if upstream redaction is bypassed.
- For Azure, the responses endpoint is usually …/openai/deployments/{deployment}/responses?api-version=…; consider constructing endpoint conditionally for accuracy.
Apply:
- ): Promise<{ - endpoint: string - headers: Record<string, string> - body: unknown - }> { + ): Promise<ProviderRequestPreview<OpenAI.Responses.ResponseCreateParams>> { @@ - const headers: Record<string, string> = { + const headers: Record<string, string> = { 'Content-Type': 'application/json', - Authorization: `Bearer ${this.provider.apiKey || 'MISSING_API_KEY'}`, + Authorization: 'Bearer ***REDACTED***', ...this.defaultHeaders } @@ - const baseUrl = this.provider.baseUrl || 'https://api.openai.com/v1' - const endpoint = `${baseUrl}/responses` + const baseUrl = this.provider.baseUrl || 'https://api.openai.com/v1' + const endpoint = + this.provider.id === 'azure-openai' + ? `${baseUrl}/responses` // if baseUrl already includes /deployments/{deployment} + : `${baseUrl}/responses`Add import/alias as needed:
import type { ProviderRequestPreview } from '@shared/provider-operations'Please confirm your Azure baseUrl format so we can compute a precise preview path. Based on learnings.
src/main/lib/redact.ts (1)
1-138: Broaden redaction coverage (headers/body) and keep endpoint passthrough optional
- Consider adding keys like client_secret, client_id, refresh_token to SENSITIVE_BODY_KEYS and set-cookie, cookie to SENSITIVE_HEADER_KEYS.
- Optionally type redactRequestPreview to carry through endpoint for convenience.
Patch examples:
-const SENSITIVE_HEADER_KEYS = [ +const SENSITIVE_HEADER_KEYS = [ 'authorization', + 'proxy-authorization', + 'cookie', + 'set-cookie', 'api-key', 'x-api-key', @@ -const SENSITIVE_BODY_KEYS = ['api_key', 'apiKey', 'apikey', 'secret', 'password', 'token'] +const SENSITIVE_BODY_KEYS = [ + 'api_key', + 'apiKey', + 'apikey', + 'secret', + 'client_secret', + 'clientId', + 'client_id', + 'password', + 'token', + 'access_token', + 'refresh_token' +]Optional endpoint passthrough:
-export function redactRequestPreview(preview: { headers: Record<string, string>; body: unknown }): { - headers: Record<string, string> - body: unknown -} { - return { - headers: redactHeaders(preview.headers), - body: redactBody(preview.body) - } -} +export function redactRequestPreview<T extends { headers: Record<string, string>; body: unknown; endpoint?: string }>( + preview: T +): T { + return { + ...preview, + headers: redactHeaders(preview.headers), + body: redactBody(preview.body) + } +}src/main/presenter/llmProviderPresenter/providers/openAICompatibleProvider.ts (1)
1628-1721: Align preview typing, pre‑redact Authorization, and fine‑tune endpoint
- Return a strong type (ProviderRequestPreview<OpenAI.Chat.ChatCompletionCreateParams>).
- Use 'Bearer REDACTED' for Authorization in the preview.
- If provider.id is 'azure-openai', endpoint often includes /openai/deployments/{deployment}/chat/completions; consider reflecting that for accuracy.
Apply:
- ): Promise<{ - endpoint: string - headers: Record<string, string> - body: unknown - }> { + ): Promise<ProviderRequestPreview<OpenAI.Chat.ChatCompletionCreateParams>> { @@ - const headers: Record<string, string> = { + const headers: Record<string, string> = { 'Content-Type': 'application/json', - Authorization: `Bearer ${this.provider.apiKey || 'MISSING_API_KEY'}`, + Authorization: 'Bearer ***REDACTED***', ...this.defaultHeaders } @@ - const baseUrl = this.provider.baseUrl || 'https://api.openai.com/v1' - const endpoint = `${baseUrl}/chat/completions` + const baseUrl = this.provider.baseUrl || 'https://api.openai.com/v1' + const endpoint = + this.provider.id === 'azure-openai' + ? `${baseUrl}/chat/completions` // if baseUrl embeds /deployments/{deployment} + : `${baseUrl}/chat/completions`Add import as needed:
import type { ProviderRequestPreview } from '@shared/provider-operations'Confirm Azure baseUrl shape so we can compute an exact preview path. Based on learnings.
* Merge pull request #1079 from ThinkInAIXYZ/bugfix/refresh-model fix: custom provider add refresh-model * fix: add tool call context for better conv (#1081) * fix: update `tag_name` for release artifact urls (#1084) Signed-off-by: Rui Chen <rui@chenrui.dev> * refactor: standardize image block data structure (#1082) * refactor: standardize image block data structure with backward compatibility Normalize image_data structure in ThreadPresenter to ensure consistent mimeType handling. Update MessageBlockImage component to support legacy data formats (content as object/string/data URI) while maintaining compatibility with new image_data field. * fix: properly normalize image data URIs before persistence Extract base64 content and mime type from data URIs (data:image/jpeg;base64,...) to prevent double-encoding in renderer. This fixes image display errors where data:image/png;base64,data:image/jpeg;base64,... was being constructed. - Parse data URIs to extract real mime type and base64 content - Force URL schemes (http://, https://, imgcache://) to deepchat/image-url - Preserve provided mime types when available - Fallback to image/png only for raw base64 without metadata * fix: normalize legacy data URIs in renderer to prevent double-encoding Handle historical image_data records that may still contain full data:image/...;base64,... URIs. Extract base64 content and mime type before template binding to prevent constructing invalid data:image/png;base64,data:image/png;base64,... URIs. - Parse data URIs in both new image_data and legacy content formats - Always provide mimeType fallback for historical records - Ensure normalized data format before template consumption * feat: add request trace for llm (#1085) * feat: add trace support wip * feat: add trace dialog with monaco * feat: add i18n for trace dialog * feat: add config for trace params * fix: prevent stale previews when messageId changes * fix: toggle model config refresh (#1086) * release: 0.4.5 --------- Signed-off-by: Rui Chen <rui@chenrui.dev> Co-authored-by: Rui Chen <rui@chenrui.dev> Co-authored-by: 韦伟 <xweimvp@gmail.com>
Summary by CodeRabbit
New Features
Localization
Data
Documentation