feat: add diagnostic logs panel with debug capture mode#176
Merged
Conversation
- Add dedicated 'Diagnostic Logs' tab in settings sidebar (between Data Management and Network) - Implement dual-layer logging architecture: - Base layer (always on): key events + warn/error with HTTP context (status, durationMs) - Debug layer (toggle-controlled): full HTTP request details for every API call - Add frontend/backend debug mode toggles with sessionStorage persistence - Add event type filtering (Sync Repos, AI Analysis, Refresh Trending, etc.) - Add real-time log updates via CustomEvent (gsm:diagnostic-log-added/cleared) - Enhance HTTP request instrumentation in githubApi, aiService, backendAdapter, webdavService - Add DELETE /api/logs and POST /api/logs/debug endpoints for backend - Remove old log export section from DataManagementPanel (moved to new panel) - Add logger.isDebugMode() guard for debug-level logs to optimize performance - Extract logAIRequestDebug() helper to reduce code duplication in aiService Technical details: - Frontend logger: 2000-entry ring buffer with write-time sanitization - Backend logger: mirrored design with Morgan HTTP access log integration - Event types auto-inferred from (module, message) patterns - Search across module + message fields - Level pills: debug/info/warn/error - Scope toggle: all/frontend/backend - HTTP summary display: 'GET /path → 200 (245ms)' format with color-coded status Files modified: - server/src/routes/logs.ts: add DELETE, GET/POST debug endpoints - server/src/services/logger.ts: add getLevel(), isDebugMode(), getModules() - src/components/SettingsPanel.tsx: add 'logs' tab with ScrollText icon - src/components/settings/DataManagementPanel.tsx: remove log export section - src/components/settings/DiagnosticLogsPanel.tsx: new component (~450 lines) - src/components/settings/index.ts: export DiagnosticLogsPanel - src/services/aiService.ts: add debug logging, extract logAIRequestDebug() - src/services/autoSync.ts: add durationMs to sync events - src/services/backendAdapter.ts: add debug logging in fetchWithTimeout/Retry - src/services/githubApi.ts: add debug logging in makeRequest - src/services/logger.ts: add CustomEvent dispatch, getModules(), isDebugMode() - src/services/webdavService.ts: add debug logging for PUT/GET - src/utils/logEventTypes.ts: new file for event type mapping Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds end-to-end diagnostic logging: backend log APIs and logger inspection, frontend logger events, a new Diagnostic Logs settings panel, log event classification, and timing/debug instrumentation across AI, GitHub, sync, backend adapter, and WebDAV services. ChangesDiagnostic Logging & Log Management
Sequence Diagram(s)sequenceDiagram
participant Browser
participant DiagnosticPanel as DiagnosticLogsPanel
participant FrontendLogger as logger (frontend)
participant BackendAPI as /api/logs
Browser->>DiagnosticPanel: open panel
DiagnosticPanel->>FrontendLogger: subscribe to gsm:diagnostic-log-added/cleared
FrontendLogger-->>DiagnosticPanel: real-time log events
DiagnosticPanel->>BackendAPI: GET /api/logs?limit=2000
BackendAPI-->>DiagnosticPanel: logs + X-Log-Count
DiagnosticPanel->>DiagnosticPanel: merge/filter/export/clear actions
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/services/webdavService.ts (2)
209-227:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove unnecessary escape characters in regex.
The forward slash (
/) does not need to be escaped in JavaScript regular expressions. The pattern/^https?:\/\/[^\/]+/can be simplified to/^https?:\/\/[^/]+/.♻️ Proposed fix
- const sanitizedPath = this.getFullPath(filename).replace(/^https?:\/\/[^\/]+/, ''); + const sanitizedPath = this.getFullPath(filename).replace(/^https?:\/\/[^/]+/, '');As per coding guidelines (ESLint: no-useless-escape).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/webdavService.ts` around lines 209 - 227, Update the regex used when computing sanitizedPath in the PUT request code path (the expression assigned to sanitizedPath that calls this.getFullPath(filename).replace(...)) to remove the unnecessary escape of the forward slash; replace the pattern currently written as /^https?:\/\/[^\/]+/ with the simplified /^https?:\/\/[^/]+/ so it conforms to ESLint no-useless-escape rules and avoids redundant backslashes while keeping behavior the same.
316-332:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove unnecessary escape characters in regex.
Same issue as the uploadFile method above: the forward slash does not need to be escaped in the regular expression.
♻️ Proposed fix
- const sanitizedPath = this.getFullPath(filename).replace(/^https?:\/\/[^\/]+/, ''); + const sanitizedPath = this.getFullPath(filename).replace(/^https?:\/\/[^/]+/, '');As per coding guidelines (ESLint: no-useless-escape).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/webdavService.ts` around lines 316 - 332, The regex used when building sanitizedPath in webdavService (sanitizedPath = this.getFullPath(filename).replace(/^https?:\/\/[^\/]+/, '')) contains an unnecessary escaped forward slash; update the pattern used in the replace call (and mirror the same fix in uploadFile if present) to remove the pointless escape so ESLint no-useless-escape is satisfied while preserving the intent of stripping the protocol+host from this.getFullPath(filename).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/settings/DiagnosticLogsPanel.tsx`:
- Around line 81-82: The backend debug toggle (backendDebug state in
DiagnosticLogsPanel.tsx) is currently initialized from sessionStorage and
updated optimistically; instead call GET /api/logs/debug on mount to initialize
backendDebug from the server (use that response to setBackendDebug) and remove
relying solely on sessionStorage, and when toggling send POST /api/logs/debug
and only update sessionStorage and call setBackendDebug after the POST succeeds
(revert UI or show error if POST fails); update the code paths that read/write
'gsm:backend-debug' (including the toggle handler and any persistence logic
around setBackendDebug) to reflect this flow.
- Around line 168-175: The memoized availableModules constant is computed from
allEntries but never used; remove the availableModules useMemo or wire it into
the module filtering UI so the lint error goes away. Locate the useMemo named
availableModules (references allEntries) and either delete that block and any
related unused imports, or connect availableModules to the module filter
component/state (e.g., pass it as options to the module selector or use it to
populate a filter dropdown and apply selection to the entries list) so the value
is consumed.
- Around line 270-279: Change the unnecessary let and unused destructure in this
block: make frontendLogs a const instead of let since it is never reassigned,
and replace the unused "_" binding in the find callback by using a skipped
element in the array pattern (e.g., ([, v]) => v === minLevel) when computing
minLevelName; update references to the symbols levelOrder, minLevelName, and
frontendLogs accordingly.
- Around line 463-475: The debug-pill disabling logic currently uses disabled =
level === 'debug' && !frontendDebug && selectedScope !== 'backend', which
prevents showing backend debug entries in the 'all' scope; update the condition
to allow debug when backend debug is visible in 'all' (e.g. disabled = level ===
'debug' && !frontendDebug && !(selectedScope === 'backend' || (selectedScope ===
'all' && backendDebug))). Modify the logic where LEVEL_COLORS, selectedScope,
frontendDebug, backendDebug, selectedLevels and toggleLevel are referenced so
the debug button is enabled when backendDebug is true and scope is 'all'.
In `@src/services/aiService.ts`:
- Around line 769-770: The catch block currently declares catch (error) but does
not include the error when calling logger.warn ('ai', 'AI semantic search
failed, falling back to enhanced basic search', ...), so either remove the
unused "error" parameter or include its details in the log; update the handler
in src/services/aiService.ts for the method containing this catch by adding the
error to the logger call (e.g., include error or error.message/stack in the
metadata) or change to catch { ... } if you truly want to ignore the error, and
keep the existing fields apiType, model, configId and durationMs when modifying
the logger.warn invocation.
---
Outside diff comments:
In `@src/services/webdavService.ts`:
- Around line 209-227: Update the regex used when computing sanitizedPath in the
PUT request code path (the expression assigned to sanitizedPath that calls
this.getFullPath(filename).replace(...)) to remove the unnecessary escape of the
forward slash; replace the pattern currently written as /^https?:\/\/[^\/]+/
with the simplified /^https?:\/\/[^/]+/ so it conforms to ESLint
no-useless-escape rules and avoids redundant backslashes while keeping behavior
the same.
- Around line 316-332: The regex used when building sanitizedPath in
webdavService (sanitizedPath =
this.getFullPath(filename).replace(/^https?:\/\/[^\/]+/, '')) contains an
unnecessary escaped forward slash; update the pattern used in the replace call
(and mirror the same fix in uploadFile if present) to remove the pointless
escape so ESLint no-useless-escape is satisfied while preserving the intent of
stripping the protocol+host from this.getFullPath(filename).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f4adbe3e-98c5-4423-846b-3cefbd1f195a
📒 Files selected for processing (13)
server/src/routes/logs.tsserver/src/services/logger.tssrc/components/SettingsPanel.tsxsrc/components/settings/DataManagementPanel.tsxsrc/components/settings/DiagnosticLogsPanel.tsxsrc/components/settings/index.tssrc/services/aiService.tssrc/services/autoSync.tssrc/services/backendAdapter.tssrc/services/githubApi.tssrc/services/logger.tssrc/services/webdavService.tssrc/utils/logEventTypes.ts
💤 Files with no reviewable changes (1)
- src/components/settings/DataManagementPanel.tsx
- Remove unnecessary regex escape in webdavService.ts (2 places) - Initialize backendDebug state from server on mount instead of sessionStorage - Remove unused availableModules useMemo from DiagnosticLogsPanel - Change frontendLogs from let to const in export handler - Fix debug pill disabled logic to allow backend debug in 'all' scope - Remove unused error parameter in aiService catch block Fixes 5 actionable CodeRabbit review comments.
Owner
Author
|
@coderabbitai full review |
Contributor
✅ Actions performedFull review triggered. |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/settings/DiagnosticLogsPanel.tsx (1)
518-544: ⚡ Quick winEvent-type dropdown stays open on outside click.
showEventTypeDropdownonly toggles via its own button, so clicking elsewhere (or pressing Escape) leaves it open and overlapping other controls. Consider closing it on outside click / Escape.♻️ Add an outside-click/Escape handler
+ const eventTypeRef = useRef<HTMLDivElement>(null); + useEffect(() => { + if (!showEventTypeDropdown) return; + const onDocClick = (e: MouseEvent) => { + if (eventTypeRef.current && !eventTypeRef.current.contains(e.target as Node)) { + setShowEventTypeDropdown(false); + } + }; + const onKey = (e: KeyboardEvent) => { + if (e.key === 'Escape') setShowEventTypeDropdown(false); + }; + document.addEventListener('mousedown', onDocClick); + document.addEventListener('keydown', onKey); + return () => { + document.removeEventListener('mousedown', onDocClick); + document.removeEventListener('keydown', onKey); + }; + }, [showEventTypeDropdown]);Then attach the ref to the wrapper:
- <div className="relative"> + <div className="relative" ref={eventTypeRef}>(
useRefis already imported.)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/settings/DiagnosticLogsPanel.tsx` around lines 518 - 544, The event-type dropdown controlled by showEventTypeDropdown only toggles via its button and doesn't close on outside click or Escape; fix this by wrapping the dropdown in a ref (useRef) attached to the outer wrapper div, add a useEffect that listens for mousedown/touchstart to detect clicks outside that ref and for keydown to detect Escape, and call setShowEventTypeDropdown(false) when either occurs; ensure the effect cleans up listeners on unmount and only runs when showEventTypeDropdown is true so toggleEventType, availableEventTypes, selectedEventTypes, EVENT_TYPE_LABELS and language behavior remains unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/components/settings/DiagnosticLogsPanel.tsx`:
- Around line 518-544: The event-type dropdown controlled by
showEventTypeDropdown only toggles via its button and doesn't close on outside
click or Escape; fix this by wrapping the dropdown in a ref (useRef) attached to
the outer wrapper div, add a useEffect that listens for mousedown/touchstart to
detect clicks outside that ref and for keydown to detect Escape, and call
setShowEventTypeDropdown(false) when either occurs; ensure the effect cleans up
listeners on unmount and only runs when showEventTypeDropdown is true so
toggleEventType, availableEventTypes, selectedEventTypes, EVENT_TYPE_LABELS and
language behavior remains unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9f1ce32b-6552-4630-aa9f-dc21b8240942
📒 Files selected for processing (13)
server/src/routes/logs.tsserver/src/services/logger.tssrc/components/SettingsPanel.tsxsrc/components/settings/DataManagementPanel.tsxsrc/components/settings/DiagnosticLogsPanel.tsxsrc/components/settings/index.tssrc/services/aiService.tssrc/services/autoSync.tssrc/services/backendAdapter.tssrc/services/githubApi.tssrc/services/logger.tssrc/services/webdavService.tssrc/utils/logEventTypes.ts
💤 Files with no reviewable changes (1)
- src/components/settings/DataManagementPanel.tsx
- Add useRef to imports - Add eventTypeRef for dropdown wrapper - Add useEffect to handle mousedown outside and Escape key - Attach ref to dropdown wrapper div Addresses CodeRabbit nitpick comment.
- Sort logs by time descending (newest first) - Add pagination with 'Load More' button (100 entries per page) - Fix debug pill: always clickable, regardless of debug mode state - Replace inline expansion with click-to-modal detail view - Add Chrome DevTools-style tabs in detail modal (General, Timing, Request Header, Request Body, Response Header, Response Body) - Add floating global debug indicator (bottom-right) with click to disable - Auto-refresh backend logs every 10 seconds - Reset pagination when filters change
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/settings/DiagnosticLogsPanel.tsx`:
- Around line 373-377: The single-scope branches in the useMemo for allEntries
return entries and backendEntries unsorted, causing frontend/backend views to
show oldest-first; modify the useMemo (the selectedScope check inside the
useMemo that computes allEntries) to return a new array sorted descending by
timestamp for each scope (i.e., when selectedScope === 'frontend' return
[...entries].sort((a,b)=>b.timestamp.localeCompare(a.timestamp)) and similarly
for 'backend' using backendEntries) so that visibleEntries =
filteredEntries.slice(0, visibleCount) presents the newest logs first.
- Around line 221-231: The DataBlock component currently declares an unused prop
t causing an ESLint no-unused-vars failure; remove t from the DataBlock
props/interface and from its parameter list (i.e., update the React.FC type and
the function signature to only accept { data, emptyText }), and also remove the
corresponding t={t} prop at each call site where DataBlock is rendered (the four
places passing t) so the component and callers no longer reference the unused t
symbol.
- Around line 233-255: The DebugModeIndicator component defined as
DebugModeIndicator in DiagnosticLogsPanel only renders while DiagnosticLogsPanel
is mounted (SettingsPanel mounts it when displayTab === 'logs'), so the
indicator is not global; either (A) update the comment/label to reflect that
DebugModeIndicator is scoped to the Logs tab/DiagnosticLogsPanel, or (B)
move/lift the DebugModeIndicator out of DiagnosticLogsPanel into a higher-level
component (e.g. the parent that renders SettingsPanel or App root) so it
persists across routes — when moving, keep the same props (frontendDebug,
backendDebug, onToggleFrontend, t) and update any references in SettingsPanel to
remove the local instance; ensure the title/onClick handlers (onToggleFrontend)
continue to work after relocation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: eaefdbf5-05f0-48d6-abaa-63ea279f05f2
📒 Files selected for processing (1)
src/components/settings/DiagnosticLogsPanel.tsx
…ed prop, move indicator to App - Sort frontend/backend entries descending in allEntries useMemo branches - Remove unused 't' prop from DataBlock component - Move DebugModeIndicator to App.tsx (global, always visible) - Create standalone DebugModeIndicator.tsx reading from sessionStorage - Remove DebugModeIndicator from DiagnosticLogsPanel (now in App.tsx)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/DebugModeIndicator.tsx`:
- Around line 11-12: The frontend logger's level isn't restored at app startup
so DebugModeIndicator (frontendDebug from sessionStorage 'gsm:frontend-debug')
can show FE/DEBUG while logger remains at default 'info'; fix by restoring the
persisted level on app init instead of waiting for DiagnosticLogsPanel: read
sessionStorage.getItem('gsm:frontend-debug') in App (or during logger module
initialization in src/services/logger.ts) and call logger.setLevel('debug') when
the flag is 'true' so the actual logger level matches DebugModeIndicator's
frontendDebug state (remove or keep the DiagnosticLogsPanel useState initializer
but ensure it no longer needs to be the only place setting logger level).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 47097685-ca4f-4887-b5b8-c2f8f574a98f
📒 Files selected for processing (3)
src/App.tsxsrc/components/DebugModeIndicator.tsxsrc/components/settings/DiagnosticLogsPanel.tsx
…ndicator navigation - Add 'fork' (刷新复刻) and 'workflow' (执行 Workflow) event types - Add logging to ForkTimeline for refresh, sync, and workflow trigger - Capture request/response headers and bodies in debug logs (backendAdapter.ts and githubApi.ts) - Debug indicator click: disable all debug + navigate to settings/logs tab - Restore persisted frontend debug level at app startup (CodeRabbit fix)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds a dedicated "Diagnostic Logs" settings tab with an in-app log viewer and debug capture mode, replacing the old log export section that was embedded in Data Management.
New Features
GET /path → 200 (245ms)with color-coded status codesArchitecture
Dual-layer logging system:
Performance optimizations:
if (logger.isDebugMode())guardlogAIRequestDebug()helper to reduce code duplicationChanges
Backend
server/src/routes/logs.ts- Add DELETE /api/logs and POST /api/logs/debug endpointsserver/src/services/logger.ts- Add getLevel(), isDebugMode(), getModules() methodsFrontend Components
src/components/settings/DiagnosticLogsPanel.tsx- New component (~450 lines)src/components/SettingsPanel.tsx- Add 'logs' tab with ScrollText iconsrc/components/settings/DataManagementPanel.tsx- Remove old log export sectionServices (HTTP request instrumentation)
src/services/githubApi.ts- Debug logging in makeRequest, completion logs for trending/releasessrc/services/aiService.ts- Debug logging in requestText, lifecycle logs for analysissrc/services/backendAdapter.ts- Debug logging in fetchWithTimeout/Retrysrc/services/webdavService.ts- Debug logging for PUT/GET operationssrc/services/autoSync.ts- Add durationMs to sync eventsUtilities
src/utils/logEventTypes.ts- Event type mapping (13 types with i18n labels)Screenshots
Testing
Related Issues
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Removed