feat: kanban dashboard i18n, request/response body capture, log auto-cleanup, and GHCR publish - #6400
feat: kanban dashboard i18n, request/response body capture, log auto-cleanup, and GHCR publish#6400molicherry wants to merge 16 commits into
Conversation
…al storage - Add 8 configurable capture flags (StoreRequestBodyEnabled, StoreResponseBodyEnabled, StoreRequestHeadersEnabled, StoreResponseHeadersEnabled, StoreProviderRequestBodyEnabled, StoreProviderResponseBodyEnabled) + 2 provider body flags - HeaderCapture middleware captures request/response headers+bodies from BodyStorage and via ResponseWriter wrapping; sensitive headers (Authorization, etc.) redacted - Provider format capture: request body after conversion in TextHelper, response body before conversion in all 8 relay helpers (text, audio, image, claude, gemini, rerank, embedding, responses) - File-based external storage: bodies > 4KB written to data/bodies/YYYYMMDD/ to avoid DB bloat; small bodies stored inline in logs.other JSON - GET /api/log/body/*path endpoint to serve stored body files - All flags configurable via admin UI (OptionMap) - Streaming responses skipped for body capture to avoid OOM AI-generated code
…response capture - Add type definitions for request_headers, request_body, response_headers, response_body, provider_request_body, provider_response_body to LogOtherData - Add InspectHeaderBlock and InspectBodyBlock components to details-dialog for admin-only display of captured headers/bodies - Inline JSON bodies rendered in collapsible pre block with pretty-print; file-referenced bodies show disk path - Add i18n keys for inspect labels (en.json) AI-generated code
…ettings - Add 6 boolean toggles for request/response header and body capture (StoreRequestBodyEnabled, StoreResponseBodyEnabled, StoreRequestHeadersEnabled, StoreResponseHeadersEnabled, StoreProviderRequestBodyEnabled, StoreProviderResponseBodyEnabled) - Update OperationsSettings type and default values - Pass bodyCaptureSettings prop through section-registry to LogSettingsSection component - Save all changed settings on form submit AI-generated code
- Add BodyFileRetentionDays config (default 7 days, 0 = keep forever) - StartBodyFileCleanup() goroutine runs hourly, deletes directories older than retention days under data/bodies/ - Cleanup is O(directories) thanks to date-sharded YYYYMMDD structure - Register BodyFileRetentionDays in OptionMap (configurable via API) AxonHub uses CleanupOptions with per-resource-type retention days. This implements the same concept: body files auto-removed after N days so they don't accumulate indefinitely. AI-generated code
- Add LogRetentionDays option for automatic DB log record cleanup - Background goroutine deletes logs older than configured days (hourly) - Reuses existing CountOldLog/DeleteOldLogBatch for cross-DB safety - Frontend: Retention Days input in Operations > Log Maintenance - Add GHCR multi-arch publish workflow (tag-triggered + manual)
…daily overview, and performance ranking Backend: - Add GET /api/data/channel endpoint (GROUP BY channel_id, model_name, created_at) - Add all_time=true param to /api/data and /api/data/self (bypass time range filter) Frontend - Token consumption charts (Item 1): - Add token-based chart specs: spec_model_token_line, spec_token_pie, spec_token_rank_bar - Add MetricMode toggle (Call Count / Token Consumption) in ModelCharts with persistence Frontend - Channel cross-analysis (Item 2): - New ChannelCharts component with Requests/Tokens/Quota metric selector - Admin-only, aggregates quota_data by channel_id over time Frontend - All-Time stats (Item 3): - Add 'All Time' preset to time range selector (days=-1) - Handle all_time param in API calls and filter dialog Frontend - Daily overview chart (Item 4): - New DailyOverviewPanel with dual-Y-axis combo chart (bar: Count, line: Tokens) - Added to overview dashboard section Frontend - Performance ranking & trends (Item 5): - New PerformanceRanking: horizontal bar ranking by throughput with confidence badges - New PerformanceTrends: 30-day line chart with Throughput/Latency/TTFT metric selector - Both admin-only, using existing perf-metrics API i18n: Add 6 new English translation keys
…s instead of hook
… * Body, Failed to load)
WalkthroughThis change adds configurable request and provider payload inspection with filesystem offloading and log retention cleanup, extends dashboard quota analytics with all-time, token, channel, daily, and performance charts, updates localized settings and inspection UI, and introduces multi-architecture GHCR publishing automation. ChangesRequest inspection and log maintenance
Dashboard analytics
Release automation
Estimated code review effort: 5 (Critical) | ~120 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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}
There was a problem hiding this comment.
Actionable comments posted: 10
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
web/src/features/dashboard/components/models/models-filter-dialog.tsx (1)
138-155: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winHandle
defaultTimeRangeDays === -1inbuildDefaultDashboardFilters
getRollingDateRange(-1)creates a future/empty range, and the dialog-only guard doesn’t cover the other callers that usebuildDefaultDashboardFiltersdirectly. Special-case-1there and leavestart_timestamp/end_timestampunset.🤖 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 `@web/src/features/dashboard/components/models/models-filter-dialog.tsx` around lines 138 - 155, Update buildDefaultDashboardFilters to special-case defaultTimeRangeDays === -1 by leaving start_timestamp and end_timestamp unset; remove the dialog-only date-range correction from handleReset so all callers share this behavior, while preserving rolling-range generation for other values.model/option.go (1)
294-346: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
BodyFileRetentionDays/LogRetentionDayscases are unreachable — retention settings never take effect.These two
casebranches live inside theswitch keyblock guarded byif strings.HasSuffix(key, "Enabled") || key == "DefaultCollapseSidebar" || key == "DefaultUseAutoGroup" || key == "SMTPForceAuthLogin" || key == "SMTPInsecureSkipVerify"at line 294. Since neither key ends in"Enabled"nor matches the explicit comparisons, this guard is always false for them, so the branches never execute — dead code.Consequence:
common.OptionMap[key] = valuestill stores the raw string, butcommon.BodyFileRetentionDays/common.LogRetentionDays(the actual variables read bycommon/log_cleanup.goandcommon/body_file.go) are never updated, whether viaUpdateOption(live save) orloadOptionsFromDatabase()(startup replay of persisted config). The admin-configurable retention feature this PR adds is effectively non-functional — cleanup always runs against the compiled defaults.🐛 Proposed fix: move the cases out of the "Enabled" guarded switch
case "LogConsumeEnabled": common.LogConsumeEnabled = boolValue case "StoreRequestBodyEnabled": common.StoreRequestBodyEnabled = boolValue case "StoreResponseBodyEnabled": common.StoreResponseBodyEnabled = boolValue case "StoreRequestHeadersEnabled": common.StoreRequestHeadersEnabled = boolValue case "StoreResponseHeadersEnabled": common.StoreResponseHeadersEnabled = boolValue case "StoreProviderRequestBodyEnabled": common.StoreProviderRequestBodyEnabled = boolValue case "StoreProviderResponseBodyEnabled": common.StoreProviderResponseBodyEnabled = boolValue - case "BodyFileRetentionDays": - days, err := strconv.Atoi(value) - if err == nil && days >= 0 { - common.BodyFileRetentionDays = days - } - case "LogRetentionDays": - days, err := strconv.Atoi(value) - if err == nil && days >= 0 { - common.LogRetentionDays = days - } case "DisplayInCurrencyEnabled":Then add them to the unconditional
switch key { ... }block further down (nearcase "EmailDomainWhitelist":):switch key { case "EmailDomainWhitelist": common.EmailDomainWhitelist = strings.Split(value, ",") + case "BodyFileRetentionDays": + days, err := strconv.Atoi(value) + if err == nil && days >= 0 { + common.BodyFileRetentionDays = days + } + case "LogRetentionDays": + days, err := strconv.Atoi(value) + if err == nil && days >= 0 { + common.LogRetentionDays = days + } case "SMTPServer": common.SMTPServer = value🤖 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 `@model/option.go` around lines 294 - 346, Move the BodyFileRetentionDays and LogRetentionDays handling out of the Enabled-specific switch guarded by the boolean-key condition and into the unconditional switch in the option update flow. Preserve the existing non-negative integer validation and assignments to common.BodyFileRetentionDays and common.LogRetentionDays so both UpdateOption and loadOptionsFromDatabase apply persisted retention values.
🟡 Minor comments (11)
web/src/features/dashboard/lib/charts.ts-1060-1078 (1)
1060-1078: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winBind each series to a specific axis. The
commonchart defines left/right y-axes, but neither series is mapped withseriesId/seriesIndex, so both can fall back to the left scale and leave the right axis unused. BindCountto the left axis andTokensto the right axis.🤖 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 `@web/src/features/dashboard/lib/charts.ts` around lines 1060 - 1078, Bind the Count series to the left y-axis and the Tokens series to the right y-axis in the common chart configuration. Update the series definitions using their existing identifiers (seriesId or seriesIndex) so each series explicitly references the intended axis, while preserving the current axis definitions.web/src/features/dashboard/components/models/channel-charts.tsx-58-60 (1)
58-60: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winLocalize the channel label.
channelLabelreturns a literalChannel #${id}that is rendered in the legend, tooltip keys, and color domain but never passed throught(). As per coding guidelines (所有面向用户的文案必须支持 i18n；React 组件使用useTranslation()的t()), the "Channel" prefix should be translatable.🤖 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 `@web/src/features/dashboard/components/models/channel-charts.tsx` around lines 58 - 60, Update channelLabel and its callers so the user-facing “Channel” prefix is generated through the component’s useTranslation() t() function, while preserving the channel ID formatting for legend, tooltip, and color-domain values.Source: Coding guidelines
web/src/features/dashboard/components/models/channel-charts.tsx-189-222 (1)
189-222: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPadding drops sparse historical buckets. When
sortedTimes.length < MAX_CHART_TREND_POINTS, rebuildingchartTimesfromlastTimecan push real buckets outside the synthetic window, so older sparse points render as zeros instead of their actual values. Build the padded range from the real data span instead of anchoring only to the latest point.🤖 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 `@web/src/features/dashboard/components/models/channel-charts.tsx` around lines 189 - 222, The padding logic in the chart time-series construction should preserve sparse historical buckets instead of rebuilding solely from the latest timestamp. Update the chartTimes calculation near the channelValues loop to derive the synthetic range from the real data span, including the earliest and latest data points, while retaining existing real bucket values through timeChannelMap lookups.web/src/i18n/locales/ru.json-722-723 (1)
722-723: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse complete Russian wording for the non-streaming restriction.
"Только не-потоковые."is incomplete and awkward without a noun. Use wording such as"Только для непотоковых запросов."so the limitation is clear.🤖 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 `@web/src/i18n/locales/ru.json` around lines 722 - 723, Update the Russian translations for both “Capture raw response body” entries in the locale object to use complete wording for the non-streaming restriction, replacing the fragment “Только не-потоковые.” with phrasing such as “Только для непотоковых запросов.”web/src/i18n/locales/ru.json-1855-1855 (1)
1855-1855: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winPreserve “body” in the error translation.
"Не удалось загрузить файл"loses that this is specifically a request/response body file, making the usage-log error less actionable. Translate it as"Не удалось загрузить файл тела"or equivalent.🤖 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 `@web/src/i18n/locales/ru.json` at line 1855, Update the “Failed to load body file” translation in ru.json to preserve the meaning that the failed file is specifically a body file, using “Не удалось загрузить файл тела” or an equivalent Russian translation.web/src/i18n/locales/ru.json-4316-4316 (1)
4316-4316: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDo not omit the consume-log and database-bloat details.
This translation drops that raw request/response data is stored in consume logs and that disk offloading avoids database bloat. Preserve those details so administrators understand the storage behavior.
🤖 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 `@web/src/i18n/locales/ru.json` at line 4316, Update the Russian translation for the consume-log storage message to explicitly mention raw request and response data, that it is stored in consume logs, and that bodies larger than 4 KB are written to disk to avoid database bloat.web/src/i18n/locales/vi.json-4316-4316 (1)
4316-4316: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMention consume logs and database-bloat prevention.
This translation loses where the captured data is stored and why large bodies are written to disk.
Proposed translation
- "Store raw request and response data in consume logs for debugging. Large bodies (>4KB) are written to disk to avoid database bloat.": "Lưu dữ liệu yêu cầu/phản hồi để gỡ lỗi. Dữ liệu lớn (>4KB) được ghi vào đĩa.", + "Store raw request and response data in consume logs for debugging. Large bodies (>4KB) are written to disk to avoid database bloat.": "Lưu dữ liệu yêu cầu và phản hồi thô trong nhật ký tiêu thụ để gỡ lỗi. Dữ liệu lớn (>4 KB) được ghi vào đĩa để tránh làm phình cơ sở dữ liệu.",🤖 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 `@web/src/i18n/locales/vi.json` at line 4316, Update the Vietnamese translation for the consume-log message to explicitly mention that raw request and response data is stored in consume logs and that bodies larger than 4KB are written to disk to prevent database bloat.web/src/i18n/locales/vi.json-2538-2538 (1)
2538-2538: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winInclude the configured retention period in the translation.
The current text says records older than “something” are deleted, but omits that the threshold is the configured number of days.
Proposed translation
- "Log records older than this many days will be deleted hourly. 0 = disabled (keep forever).": "Bản ghi nhật ký cũ hơn sẽ bị xóa mỗi giờ. 0 = tắt (giữ vĩnh viễn).", + "Log records older than this many days will be deleted hourly. 0 = disabled (keep forever).": "Các bản ghi nhật ký cũ hơn số ngày này sẽ bị xóa mỗi giờ. 0 = tắt (giữ vĩnh viễn).",🤖 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 `@web/src/i18n/locales/vi.json` at line 2538, Update the Vietnamese translation for the retention-period message so it explicitly states that records older than the configured number of days are deleted hourly, while preserving the existing “0 = disabled (keep forever)” meaning.web/src/i18n/locales/vi.json-724-724 (1)
724-724: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve the request-body and upstream JSON meaning.
The Vietnamese text drops both “request body” and “actual JSON sent to upstream,” making this capture option ambiguous.
Proposed translation
- "Capture request body AFTER format conversion (actual JSON sent to upstream).": "Ghi lại nội dung sau khi chuyển đổi (JSON thực tế gửi lên nguồn).", + "Capture request body AFTER format conversion (actual JSON sent to upstream).": "Ghi lại nội dung yêu cầu SAU khi chuyển đổi (JSON thực tế được gửi lên upstream).",🤖 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 `@web/src/i18n/locales/vi.json` at line 724, Update the Vietnamese translation for the locale key "Capture request body AFTER format conversion (actual JSON sent to upstream)." to explicitly preserve both meanings: capturing the request body after format conversion and the actual JSON sent to the upstream service.web/src/i18n/locales/zh-TW.json-3559-3560 (1)
3559-3560: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse consistent provider terminology.
The existing
Providertranslation uses供應商(Line 3554), but these new labels use提供者. Use the established terminology to keep the UI consistent.- "Provider Request Body": "提供者請求主體", - "Provider Response Body": "提供者回應主體", + "Provider Request Body": "供應商請求主體", + "Provider Response Body": "供應商回應主體",🤖 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 `@web/src/i18n/locales/zh-TW.json` around lines 3559 - 3560, Update the “Provider Request Body” and “Provider Response Body” translations in the locale data to use the established `供應商` terminology from the existing Provider translation, replacing `提供者` while preserving the rest of each label.web/src/features/system-settings/maintenance/log-settings-section.tsx-474-482 (1)
474-482: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winDo not coerce an empty retention field to zero.
Number('')becomes0, so clearing this field silently disables automatic deletion when saved. Keep blank invalid (or require an explicit0) instead.🤖 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 `@web/src/features/system-settings/maintenance/log-settings-section.tsx` around lines 474 - 482, Update the retention field’s onChange handling in the Input component so an empty value is not converted to numeric zero by Number(event.target.value). Preserve a blank value as invalid or otherwise prevent saving it, while continuing to accept explicit numeric values including 0.
🧹 Nitpick comments (3)
common/body_file.go (1)
144-151: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid this single-caller package helper.
isAllDigitsis only used byCleanupOldBodyFiles; inline the check or replace it with standard-library parsing in the caller. As per coding guidelines, “avoid package-level helpers with only one caller unless required by … complex business logic.”🤖 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 `@common/body_file.go` around lines 144 - 151, Remove the single-caller package-level helper isAllDigits and move its digit validation directly into CleanupOldBodyFiles, or use an appropriate standard-library parsing function there. Preserve the existing behavior for empty and non-digit strings while eliminating the standalone helper.Source: Coding guidelines
model/log_cleanup.go (1)
16-37: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winBound the cleanup context with a timeout.
context.Background()has no deadline, so a stalled batch delete can block this background job indefinitely. The batch-delete helpers already handle the database-specific paths.🤖 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 `@model/log_cleanup.go` around lines 16 - 37, Update cleanOldDbLogs to create a timeout-bounded context instead of using context.Background(), applying the timeout to both CountOldLog and DeleteOldLogBatch calls. Ensure the context is canceled when cleanup finishes while preserving the existing batch loop and error propagation.Source: Path instructions
web/src/features/usage-logs/components/dialogs/details-dialog.tsx (1)
1246-1262: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse English source strings instead of dotted inspect keys.
The new
inspect.*keys conflict with the required flat English-source-key convention;Loading...is also rendered withoutt().
web/src/features/usage-logs/components/dialogs/details-dialog.tsx#L1246-L1262: pass English labels such asRequest Bodyto the block and translate witht(label).web/src/features/usage-logs/components/dialogs/details-dialog.tsx#L1397-L1422: replaceinspect.file_referenceand hardcodedLoading...with translated English source strings.web/src/i18n/locales/en.json#L5227-L5231: replace dotted keys with flat English source-string keys.As per coding guidelines, “use
useTranslation()andt('English key'), with flat locale JSON files … using English source strings as keys.”🤖 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 `@web/src/features/usage-logs/components/dialogs/details-dialog.tsx` around lines 1246 - 1262, Update web/src/features/usage-logs/components/dialogs/details-dialog.tsx at lines 1246-1262 to pass English source labels such as “Request Body” to InspectBodyBlock and translate them via t(label); at lines 1397-1422 replace inspect.file_reference and the un-translated “Loading...” text with English source strings passed through t(). Update web/src/i18n/locales/en.json at lines 5227-5231 to use matching flat English source-string keys instead of dotted inspect.* keys.Source: Coding guidelines
🤖 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 @.github/workflows/ghcr-publish.yml:
- Around line 117-122: Gate the “Create & push latest manifest” step so it runs
only for confirmed stable release tags, excluding prerelease tags and
dispatch-triggered builds. Preserve the existing manifest creation command for
eligible stable releases.
- Around line 36-46: The tag is generated in multiple places and is unavailable
through env-context expressions. In .github/workflows/ghcr-publish.yml lines
36-46, expose the resolved tag as a job output from the version step; update
lines 72-74 to consume that output for the build and summary, and update lines
101-108 so the manifest job consumes the same output instead of recalculating
the date-based tag. Ensure all jobs use one consistently generated release tag.
In `@common/constants.go`:
- Around line 113-121: Wire provider-request capture through every relay path
using the existing context key and StoreProviderRequestBodyEnabled behavior:
update Claude, Gemini, embeddings, image, rerank, responses, and audio handlers,
plus TextHelper pass-through requests, to store the exact request payload sent
upstream before forwarding. Match the existing implementation in
relay/compatible_handler.go and preserve the disabled-setting behavior.
In `@controller/usedata.go`:
- Around line 71-84: Update GetQuotaDataGroupByChannel to parse the all_time
query parameter and, when it is true, avoid applying the zero start/end
timestamp range by using the established all-time handling used by
GetAllQuotaDates and GetUserQuotaDates. Preserve the existing timestamp-based
query behavior for non-all-time requests and continue returning errors through
common.ApiError.
In `@middleware/body_log.go`:
- Around line 69-76: Bound or spool response-body capture before data is
accumulated, using BodyFileThreshold or the established capture mechanism rather
than unbounded memory buffering. Update middleware/body_log.go:69-76 around
responseBodyWriter, relay/audio_handler.go:59-64,
relay/embedding_handler.go:79-85, relay/gemini_handler.go:308-314,
relay/image_handler.go:101-107, and relay/rerank_handler.go:91-97 to avoid
unrestricted full-body reads; update both capture branches in
relay/claude_handler.go:211-225, relay/compatible_handler.go:208-225,
relay/gemini_handler.go:190-205, and relay/responses_handler.go:137-152. Ensure
forwarding remains timely while RecordConsumeLog receives only bounded or
spooled captured data.
- Around line 11-16: Extend the header-redaction logic centered on
sensitiveRequestHeaders to cover Cookie and Set-Cookie, and apply it
case-insensitively before storing either request or response header map. Ensure
both session-credential headers are replaced with the existing redacted
representation in capture logs.
In `@router/relay-router.go`:
- Line 65: Update each middleware chain in router/relay-router.go at lines 65,
77-85, 182, 194, and 205 so response-header redaction runs before HeaderCapture.
Ensure the redaction covers sensitive headers such as Set-Cookie and
provider-issued credentials, then retain HeaderCapture only after that
protection is applied.
In `@web/src/features/dashboard/components/overview/daily-overview-panel.tsx`:
- Around line 64-81: Convert the Date values returned by getRollingDateRange in
the timeRange useMemo to Unix seconds numbers before passing them to
getUserQuotaDates. Update start_timestamp and end_timestamp using the
established getTime()/1000 conversion with flooring, while preserving the
existing query key and date-range behavior.
In `@web/src/features/dashboard/index.tsx`:
- Around line 418-424: The metricMode change handler must preserve the current
modelFilters instead of routing through handleChartPreferencesChange, which
rebuilds default filters and triggers a refetch. Update the onMetricModeChange
path and its preference persistence so only the client-side metricMode setting
changes while the applied custom or quick time range remains intact.
In `@web/src/features/dashboard/types.ts`:
- Around line 279-280: Remove the duplicate MetricMode type declaration near the
later section of the module, keeping the existing declaration earlier in the
file as the single source of truth.
---
Outside diff comments:
In `@model/option.go`:
- Around line 294-346: Move the BodyFileRetentionDays and LogRetentionDays
handling out of the Enabled-specific switch guarded by the boolean-key condition
and into the unconditional switch in the option update flow. Preserve the
existing non-negative integer validation and assignments to
common.BodyFileRetentionDays and common.LogRetentionDays so both UpdateOption
and loadOptionsFromDatabase apply persisted retention values.
In `@web/src/features/dashboard/components/models/models-filter-dialog.tsx`:
- Around line 138-155: Update buildDefaultDashboardFilters to special-case
defaultTimeRangeDays === -1 by leaving start_timestamp and end_timestamp unset;
remove the dialog-only date-range correction from handleReset so all callers
share this behavior, while preserving rolling-range generation for other values.
---
Minor comments:
In `@web/src/features/dashboard/components/models/channel-charts.tsx`:
- Around line 58-60: Update channelLabel and its callers so the user-facing
“Channel” prefix is generated through the component’s useTranslation() t()
function, while preserving the channel ID formatting for legend, tooltip, and
color-domain values.
- Around line 189-222: The padding logic in the chart time-series construction
should preserve sparse historical buckets instead of rebuilding solely from the
latest timestamp. Update the chartTimes calculation near the channelValues loop
to derive the synthetic range from the real data span, including the earliest
and latest data points, while retaining existing real bucket values through
timeChannelMap lookups.
In `@web/src/features/dashboard/lib/charts.ts`:
- Around line 1060-1078: Bind the Count series to the left y-axis and the Tokens
series to the right y-axis in the common chart configuration. Update the series
definitions using their existing identifiers (seriesId or seriesIndex) so each
series explicitly references the intended axis, while preserving the current
axis definitions.
In `@web/src/features/system-settings/maintenance/log-settings-section.tsx`:
- Around line 474-482: Update the retention field’s onChange handling in the
Input component so an empty value is not converted to numeric zero by
Number(event.target.value). Preserve a blank value as invalid or otherwise
prevent saving it, while continuing to accept explicit numeric values including
0.
In `@web/src/i18n/locales/ru.json`:
- Around line 722-723: Update the Russian translations for both “Capture raw
response body” entries in the locale object to use complete wording for the
non-streaming restriction, replacing the fragment “Только не-потоковые.” with
phrasing such as “Только для непотоковых запросов.”
- Line 1855: Update the “Failed to load body file” translation in ru.json to
preserve the meaning that the failed file is specifically a body file, using “Не
удалось загрузить файл тела” or an equivalent Russian translation.
- Line 4316: Update the Russian translation for the consume-log storage message
to explicitly mention raw request and response data, that it is stored in
consume logs, and that bodies larger than 4 KB are written to disk to avoid
database bloat.
In `@web/src/i18n/locales/vi.json`:
- Line 4316: Update the Vietnamese translation for the consume-log message to
explicitly mention that raw request and response data is stored in consume logs
and that bodies larger than 4KB are written to disk to prevent database bloat.
- Line 2538: Update the Vietnamese translation for the retention-period message
so it explicitly states that records older than the configured number of days
are deleted hourly, while preserving the existing “0 = disabled (keep forever)”
meaning.
- Line 724: Update the Vietnamese translation for the locale key "Capture
request body AFTER format conversion (actual JSON sent to upstream)." to
explicitly preserve both meanings: capturing the request body after format
conversion and the actual JSON sent to the upstream service.
In `@web/src/i18n/locales/zh-TW.json`:
- Around line 3559-3560: Update the “Provider Request Body” and “Provider
Response Body” translations in the locale data to use the established `供應商`
terminology from the existing Provider translation, replacing `提供者` while
preserving the rest of each label.
---
Nitpick comments:
In `@common/body_file.go`:
- Around line 144-151: Remove the single-caller package-level helper isAllDigits
and move its digit validation directly into CleanupOldBodyFiles, or use an
appropriate standard-library parsing function there. Preserve the existing
behavior for empty and non-digit strings while eliminating the standalone
helper.
In `@model/log_cleanup.go`:
- Around line 16-37: Update cleanOldDbLogs to create a timeout-bounded context
instead of using context.Background(), applying the timeout to both CountOldLog
and DeleteOldLogBatch calls. Ensure the context is canceled when cleanup
finishes while preserving the existing batch loop and error propagation.
In `@web/src/features/usage-logs/components/dialogs/details-dialog.tsx`:
- Around line 1246-1262: Update
web/src/features/usage-logs/components/dialogs/details-dialog.tsx at lines
1246-1262 to pass English source labels such as “Request Body” to
InspectBodyBlock and translate them via t(label); at lines 1397-1422 replace
inspect.file_reference and the un-translated “Loading...” text with English
source strings passed through t(). Update web/src/i18n/locales/en.json at lines
5227-5231 to use matching flat English source-string keys instead of dotted
inspect.* keys.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f9bc1f14-15d8-43b7-9e17-0ce3ae6699d6
📒 Files selected for processing (50)
.github/workflows/ghcr-publish.ymlVERSIONcommon/body_file.gocommon/constants.gocommon/log_cleanup.gocontroller/log_body.gocontroller/usedata.gomain.gomiddleware/body_log.gomodel/log.gomodel/log_cleanup.gomodel/option.gomodel/usedata.gorelay/audio_handler.gorelay/claude_handler.gorelay/compatible_handler.gorelay/embedding_handler.gorelay/gemini_handler.gorelay/image_handler.gorelay/rerank_handler.gorelay/responses_handler.gorouter/api-router.gorouter/relay-router.goweb/src/features/dashboard/api.tsweb/src/features/dashboard/components/models/channel-charts.tsxweb/src/features/dashboard/components/models/log-stat-cards.tsxweb/src/features/dashboard/components/models/model-charts.tsxweb/src/features/dashboard/components/models/models-filter-dialog.tsxweb/src/features/dashboard/components/models/performance-ranking.tsxweb/src/features/dashboard/components/models/performance-trends.tsxweb/src/features/dashboard/components/overview/daily-overview-panel.tsxweb/src/features/dashboard/components/overview/overview-dashboard.tsxweb/src/features/dashboard/constants.tsweb/src/features/dashboard/index.tsxweb/src/features/dashboard/lib/charts.tsweb/src/features/dashboard/lib/filters.tsweb/src/features/dashboard/types.tsweb/src/features/system-settings/maintenance/log-settings-section.tsxweb/src/features/system-settings/operations/index.tsxweb/src/features/system-settings/operations/section-registry.tsxweb/src/features/system-settings/types.tsweb/src/features/usage-logs/components/dialogs/details-dialog.tsxweb/src/features/usage-logs/types.tsweb/src/i18n/locales/en.jsonweb/src/i18n/locales/fr.jsonweb/src/i18n/locales/ja.jsonweb/src/i18n/locales/ru.jsonweb/src/i18n/locales/vi.jsonweb/src/i18n/locales/zh-TW.jsonweb/src/i18n/locales/zh.json
| - name: Resolve tag | ||
| id: version | ||
| run: | | ||
| if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then | ||
| TAG="${GITHUB_REF#refs/heads/}-$(date +'%Y%m%d')-${GITHUB_SHA::7}" | ||
| else | ||
| TAG=${GITHUB_REF#refs/tags/} | ||
| fi | ||
| echo "TAG=${TAG}" >> $GITHUB_ENV | ||
| echo "tag=${TAG}" >> $GITHUB_OUTPUT | ||
| echo "${TAG}" > VERSION |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
workflow='.github/workflows/ghcr-publish.yml'
# This should produce no matches after replacing dynamic env-context reads
# with a shared job/step output.
if rg -n '\$\{\{[[:space:]]*env\.TAG[[:space:]]*\}\}' "$workflow"; then
exit 1
fi
# Confirm TAG has one producer and consumers reference its output.
rg -n 'GITHUB_OUTPUT|needs\.[A-Za-z0-9_-]+\.outputs\.[A-Za-z0-9_-]+' "$workflow"Repository: QuantumNous/new-api
Length of output: 368
🏁 Script executed:
#!/bin/bash
set -eu
workflow='.github/workflows/ghcr-publish.yml'
echo '--- file outline ---'
ast-grep outline "$workflow" --view expanded || true
echo '--- relevant sections ---'
sed -n '1,140p' "$workflow" | cat -n
echo '--- env.TAG references ---'
rg -n '\$\{\{[[:space:]]*env\.TAG[[:space:]]*\}\}|GITHUB_ENV|GITHUB_OUTPUT|needs\.' "$workflow"Repository: QuantumNous/new-api
Length of output: 5899
🌐 Web query:
GitHub Actions GITHUB_ENV available in expression context env.TAG documentation
💡 Result:
The GITHUB_ENV environment file and the env context operate at different stages of the GitHub Actions workflow lifecycle, which is why a variable written to GITHUB_ENV is not immediately available in the env expression context within the same job [1][2]. 1. Understanding GITHUB_ENV: The GITHUB_ENV environment file is used to define environment variables that become available to subsequent steps within the same job [3][2]. When you write a value to this file (e.g., echo "TAG=v1.0" >> $GITHUB_ENV), the assignment is processed by the runner [1][2]. 2. Understanding the env Context: The env context is evaluated by GitHub Actions before the job is sent to the runner [1][2]. Because the env context is interpolated during this initial processing phase, it cannot see or access environment variables that are dynamically created on the runner by a step during execution [1][2]. 3. Accessing Values: - To use a value created via GITHUB_ENV in a later step within the same job, you should reference it as a standard environment variable (e.g., $TAG in a bash script) rather than via the ${{ env.TAG }} expression syntax [1][2]. - If you need to use a value in a conditional (like an if clause) or another part of the workflow processed before the runner, you cannot use variables generated via GITHUB_ENV [1][2]. In such cases, you must use outputs (via GITHUB_OUTPUT) or define the variable in the workflow file using the env key [3][2]. In summary, you cannot use ${{ env.TAG }} to access a variable created in a previous step via GITHUB_ENV because the env context does not track changes made to the environment at runtime [1][2].
Citations:
- 1: https://docs.github.com/en/actions/how-tos/write-workflows/choose-what-workflows-do/use-variables
- 2: https://docs.github.com/actions/learn-github-actions/variables
- 3: https://stackoverflow.com/questions/57968497/how-do-i-set-an-env-var-with-a-bash-expression-in-github-actions
Generate the release tag once and reuse it everywhere. GITHUB_ENV does not feed ${{ env.TAG }}, so the build step and summary expand an empty tag here. The manifest job also recalculates the date-based tag independently, which can drift across midnight. Expose one job output and consume it in both jobs.
📍 Affects 1 file
.github/workflows/ghcr-publish.yml#L36-L46(this comment).github/workflows/ghcr-publish.yml#L72-L74.github/workflows/ghcr-publish.yml#L101-L108
🤖 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 @.github/workflows/ghcr-publish.yml around lines 36 - 46, The tag is
generated in multiple places and is unavailable through env-context expressions.
In .github/workflows/ghcr-publish.yml lines 36-46, expose the resolved tag as a
job output from the version step; update lines 72-74 to consume that output for
the build and summary, and update lines 101-108 so the manifest job consumes the
same output instead of recalculating the date-based tag. Ensure all jobs use one
consistently generated release tag.
| - name: Create & push latest manifest | ||
| run: | | ||
| docker buildx imagetools create \ | ||
| -t ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:latest \ | ||
| ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:latest-amd64 \ | ||
| ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:latest-arm64 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not promote prereleases to latest.
This runs for every tag; publishing v1.0.0-rc.21-inspect.1 would overwrite latest with a release candidate. Gate this step to confirmed stable releases and skip it for dispatch builds.
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 120-120: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 121-121: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[warning] 122-122: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 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 @.github/workflows/ghcr-publish.yml around lines 117 - 122, Gate the “Create
& push latest manifest” step so it runs only for confirmed stable release tags,
excluding prerelease tags and dispatch-triggered builds. Preserve the existing
manifest creation command for eligible stable releases.
| // StoreProviderRequestBodyEnabled captures the request body AFTER format conversion | ||
| // (i.e., the actual JSON sent to the upstream provider). Stored as "provider_request_body" | ||
| // in the consume log's Other JSON. Defaults to false. | ||
| var StoreProviderRequestBodyEnabled = false | ||
|
|
||
| // StoreProviderResponseBodyEnabled captures the raw response body FROM the upstream provider | ||
| // BEFORE format conversion back to user format. Only non-streaming. Stored as | ||
| // "provider_response_body" in Other JSON. Defaults to false. | ||
| var StoreProviderResponseBodyEnabled = false |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Wire provider-request capture through every relay path.
model/log.go persists this context key globally, but among the supplied handlers only relay/compatible_handler.go sets it. Claude, Gemini, embeddings, image, rerank, responses, and audio paths omit provider requests; TextHelper also omits pass-through requests. Enabling this setting therefore produces incomplete inspection logs.
🤖 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 `@common/constants.go` around lines 113 - 121, Wire provider-request capture
through every relay path using the existing context key and
StoreProviderRequestBodyEnabled behavior: update Claude, Gemini, embeddings,
image, rerank, responses, and audio handlers, plus TextHelper pass-through
requests, to store the exact request payload sent upstream before forwarding.
Match the existing implementation in relay/compatible_handler.go and preserve
the disabled-setting behavior.
| func GetQuotaDataGroupByChannel(c *gin.Context) { | ||
| startTimestamp, _ := strconv.ParseInt(c.Query("start_timestamp"), 10, 64) | ||
| endTimestamp, _ := strconv.ParseInt(c.Query("end_timestamp"), 10, 64) | ||
| // 判断时间跨度是否超过 1 个月 | ||
| if endTimestamp-startTimestamp > 2592000 { | ||
| c.JSON(http.StatusOK, gin.H{ | ||
| "success": false, | ||
| "message": "时间跨度不能超过 1 个月", | ||
| }) | ||
| dates, err := model.GetQuotaDataGroupByChannel(startTimestamp, endTimestamp) | ||
| if err != nil { | ||
| common.ApiError(c, err) | ||
| return | ||
| } | ||
| c.JSON(http.StatusOK, gin.H{ | ||
| "success": true, | ||
| "message": "", | ||
| "data": dates, | ||
| }) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
GetQuotaDataGroupByChannel ignores all_time, breaking the "All Time" channel view.
The dashboard client sends all_time: 'true' with start_timestamp=0/end_timestamp=0 when the All Time preset is active (web/src/features/dashboard/components/models/channel-charts.tsx Lines 107-111). This handler only parses the timestamps, so the downstream query runs created_at >= 0 and created_at <= 0 and returns no rows. GetAllQuotaDates and GetUserQuotaDates already special-case all_time; this handler must do the same.
🐛 Proposed fix
func GetQuotaDataGroupByChannel(c *gin.Context) {
- startTimestamp, _ := strconv.ParseInt(c.Query("start_timestamp"), 10, 64)
- endTimestamp, _ := strconv.ParseInt(c.Query("end_timestamp"), 10, 64)
+ var startTimestamp int64
+ var endTimestamp int64
+ if c.Query("all_time") == "true" {
+ startTimestamp = 0
+ endTimestamp = time.Now().Unix()
+ } else {
+ startTimestamp, _ = strconv.ParseInt(c.Query("start_timestamp"), 10, 64)
+ endTimestamp, _ = strconv.ParseInt(c.Query("end_timestamp"), 10, 64)
+ }
dates, err := model.GetQuotaDataGroupByChannel(startTimestamp, endTimestamp)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func GetQuotaDataGroupByChannel(c *gin.Context) { | |
| startTimestamp, _ := strconv.ParseInt(c.Query("start_timestamp"), 10, 64) | |
| endTimestamp, _ := strconv.ParseInt(c.Query("end_timestamp"), 10, 64) | |
| // 判断时间跨度是否超过 1 个月 | |
| if endTimestamp-startTimestamp > 2592000 { | |
| c.JSON(http.StatusOK, gin.H{ | |
| "success": false, | |
| "message": "时间跨度不能超过 1 个月", | |
| }) | |
| dates, err := model.GetQuotaDataGroupByChannel(startTimestamp, endTimestamp) | |
| if err != nil { | |
| common.ApiError(c, err) | |
| return | |
| } | |
| c.JSON(http.StatusOK, gin.H{ | |
| "success": true, | |
| "message": "", | |
| "data": dates, | |
| }) | |
| } | |
| func GetQuotaDataGroupByChannel(c *gin.Context) { | |
| var startTimestamp int64 | |
| var endTimestamp int64 | |
| if c.Query("all_time") == "true" { | |
| startTimestamp = 0 | |
| endTimestamp = time.Now().Unix() | |
| } else { | |
| startTimestamp, _ = strconv.ParseInt(c.Query("start_timestamp"), 10, 64) | |
| endTimestamp, _ = strconv.ParseInt(c.Query("end_timestamp"), 10, 64) | |
| } | |
| dates, err := model.GetQuotaDataGroupByChannel(startTimestamp, endTimestamp) | |
| if err != nil { | |
| common.ApiError(c, err) | |
| return | |
| } | |
| c.JSON(http.StatusOK, gin.H{ | |
| "success": true, | |
| "message": "", | |
| "data": dates, | |
| }) | |
| } |
🤖 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 `@controller/usedata.go` around lines 71 - 84, Update
GetQuotaDataGroupByChannel to parse the all_time query parameter and, when it is
true, avoid applying the zero start/end timestamp range by using the established
all-time handling used by GetAllQuotaDates and GetUserQuotaDates. Preserve the
existing timestamp-based query behavior for non-all-time requests and continue
returning errors through common.ApiError.
| // Sensitive request header names whose values are redacted in capture. | ||
| var sensitiveRequestHeaders = map[string]bool{ | ||
| "authorization": true, | ||
| "proxy-authorization": true, | ||
| "x-api-key": true, | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Redact session credentials from both header maps.
Cookie currently falls through request capture, while response capture stores Set-Cookie unredacted. Enabling header capture can therefore persist session credentials in consume logs. Apply one case-insensitive redaction policy before storing either map.
Also applies to: 81-89
🤖 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 `@middleware/body_log.go` around lines 11 - 16, Extend the header-redaction
logic centered on sensitiveRequestHeaders to cover Cookie and Set-Cookie, and
apply it case-insensitively before storing either request or response header
map. Ensure both session-credential headers are replaced with the existing
redacted representation in capture logs.
| // --- Response: wrap writer --- | ||
| var wrapper *responseBodyWriter | ||
| if common.StoreResponseBodyEnabled { | ||
| wrapper = &responseBodyWriter{ | ||
| ResponseWriter: c.Writer, | ||
| buf: &bytes.Buffer{}, | ||
| } | ||
| c.Writer = wrapper |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Bound capture memory before buffering response bodies. BodyFileThreshold is enforced only when RecordConsumeLog runs, after these paths have already read or accumulated the complete response in RAM. Large downloads or long-lived streams can therefore exhaust memory and delay forwarding.
middleware/body_log.go#L69-L76: spool or cap client-response capture instead of retaining an unboundedbytes.Buffer.relay/audio_handler.go#L59-L64: avoid unrestricted full-body reads.relay/claude_handler.go#L211-L225: use bounded/spooled capture for both branches.relay/compatible_handler.go#L208-L225: use bounded/spooled capture for both branches.relay/embedding_handler.go#L79-L85: avoid unrestricted full-body reads.relay/gemini_handler.go#L190-L205: use bounded/spooled capture for both branches.relay/gemini_handler.go#L308-L314: avoid unrestricted full-body reads.relay/image_handler.go#L101-L107: avoid unrestricted full-body reads.relay/rerank_handler.go#L91-L97: avoid unrestricted full-body reads.relay/responses_handler.go#L137-L152: use bounded/spooled capture for both branches.
📍 Affects 9 files
middleware/body_log.go#L69-L76(this comment)relay/audio_handler.go#L59-L64relay/claude_handler.go#L211-L225relay/compatible_handler.go#L208-L225relay/embedding_handler.go#L79-L85relay/gemini_handler.go#L190-L205relay/gemini_handler.go#L308-L314relay/image_handler.go#L101-L107relay/rerank_handler.go#L91-L97relay/responses_handler.go#L137-L152
🤖 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 `@middleware/body_log.go` around lines 69 - 76, Bound or spool response-body
capture before data is accumulated, using BodyFileThreshold or the established
capture mechanism rather than unbounded memory buffering. Update
middleware/body_log.go:69-76 around responseBodyWriter,
relay/audio_handler.go:59-64, relay/embedding_handler.go:79-85,
relay/gemini_handler.go:308-314, relay/image_handler.go:101-107, and
relay/rerank_handler.go:91-97 to avoid unrestricted full-body reads; update both
capture branches in relay/claude_handler.go:211-225,
relay/compatible_handler.go:208-225, relay/gemini_handler.go:190-205, and
relay/responses_handler.go:137-152. Ensure forwarding remains timely while
RecordConsumeLog receives only bounded or spooled captured data.
| playgroundRouter.Use(middleware.RouteTag("relay")) | ||
| playgroundRouter.Use(middleware.SystemPerformanceCheck()) | ||
| playgroundRouter.Use(middleware.UserAuth(), middleware.Distribute()) | ||
| playgroundRouter.Use(middleware.UserAuth(), middleware.Distribute(), middleware.HeaderCapture()) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Redact sensitive response headers before enabling capture.
middleware/body_log.go stores every response header, unlike request headers. This can persist Set-Cookie or provider-issued credentials in long-lived usage logs.
router/relay-router.go#L65-L65: use HeaderCapture only after response-header redaction covers sensitive headers.router/relay-router.go#L77-L85: use HeaderCapture only after response-header redaction covers sensitive headers.router/relay-router.go#L182-L182: use HeaderCapture only after response-header redaction covers sensitive headers.router/relay-router.go#L194-L194: use HeaderCapture only after response-header redaction covers sensitive headers.router/relay-router.go#L205-L205: use HeaderCapture only after response-header redaction covers sensitive headers.
📍 Affects 1 file
router/relay-router.go#L65-L65(this comment)router/relay-router.go#L77-L85router/relay-router.go#L182-L182router/relay-router.go#L194-L194router/relay-router.go#L205-L205
🤖 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 `@router/relay-router.go` at line 65, Update each middleware chain in
router/relay-router.go at lines 65, 77-85, 182, 194, and 205 so response-header
redaction runs before HeaderCapture. Ensure the redaction covers sensitive
headers such as Set-Cookie and provider-issued credentials, then retain
HeaderCapture only after that protection is applied.
| const defaultDays = MAX_CHART_TREND_POINTS | ||
| const timeRange = useMemo(() => { | ||
| const { start, end } = getRollingDateRange(defaultDays) | ||
| return { start_timestamp: start, end_timestamp: end } | ||
| }, [defaultDays]) | ||
|
|
||
| const chartDataQuery = useQuery({ | ||
| queryKey: ['dashboard', 'overview', 'daily-overview-chart', timeRange], | ||
| queryFn: async () => { | ||
| const result = await getUserQuotaDates({ | ||
| start_timestamp: timeRange.start_timestamp, | ||
| end_timestamp: timeRange.end_timestamp, | ||
| default_time: 'day', | ||
| }) | ||
| return result.success ? (result.data ?? []) : [] | ||
| }, | ||
| staleTime: 60 * 1000, | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
start_timestamp/end_timestamp are passed as Date, but the API contract expects Unix seconds (number).
getRollingDateRange returns Date objects, so timeRange.start_timestamp/end_timestamp are Dates. getUserQuotaDates (web/src/features/dashboard/api.ts) types these params as number (seconds), matching how channel-charts.tsx (Math.floor(startTs.getTime() / 1000)) and log-stat-cards.tsx build them. This is a TypeScript type error (fails the required type check) and at runtime the Date serializes to an ISO string, so the backend receives malformed params and the panel silently renders nothing.
🐛 Convert to Unix seconds
const timeRange = useMemo(() => {
const { start, end } = getRollingDateRange(defaultDays)
- return { start_timestamp: start, end_timestamp: end }
+ return {
+ start_timestamp: Math.floor(start.getTime() / 1000),
+ end_timestamp: Math.floor(end.getTime() / 1000),
+ }
}, [defaultDays])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const defaultDays = MAX_CHART_TREND_POINTS | |
| const timeRange = useMemo(() => { | |
| const { start, end } = getRollingDateRange(defaultDays) | |
| return { start_timestamp: start, end_timestamp: end } | |
| }, [defaultDays]) | |
| const chartDataQuery = useQuery({ | |
| queryKey: ['dashboard', 'overview', 'daily-overview-chart', timeRange], | |
| queryFn: async () => { | |
| const result = await getUserQuotaDates({ | |
| start_timestamp: timeRange.start_timestamp, | |
| end_timestamp: timeRange.end_timestamp, | |
| default_time: 'day', | |
| }) | |
| return result.success ? (result.data ?? []) : [] | |
| }, | |
| staleTime: 60 * 1000, | |
| }) | |
| const defaultDays = MAX_CHART_TREND_POINTS | |
| const timeRange = useMemo(() => { | |
| const { start, end } = getRollingDateRange(defaultDays) | |
| return { | |
| start_timestamp: Math.floor(start.getTime() / 1000), | |
| end_timestamp: Math.floor(end.getTime() / 1000), | |
| } | |
| }, [defaultDays]) | |
| const chartDataQuery = useQuery({ | |
| queryKey: ['dashboard', 'overview', 'daily-overview-chart', timeRange], | |
| queryFn: async () => { | |
| const result = await getUserQuotaDates({ | |
| start_timestamp: timeRange.start_timestamp, | |
| end_timestamp: timeRange.end_timestamp, | |
| default_time: 'day', | |
| }) | |
| return result.success ? (result.data ?? []) : [] | |
| }, | |
| staleTime: 60 * 1000, | |
| }) |
🤖 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 `@web/src/features/dashboard/components/overview/daily-overview-panel.tsx`
around lines 64 - 81, Convert the Date values returned by getRollingDateRange in
the timeRange useMemo to Unix seconds numbers before passing them to
getUserQuotaDates. Update start_timestamp and end_timestamp using the
established getTime()/1000 conversion with flooring, while preserving the
existing query key and date-range behavior.
Source: Coding guidelines
| metricMode={chartPreferences.metricMode} | ||
| onMetricModeChange={(mode) => | ||
| handleChartPreferencesChange({ | ||
| ...chartPreferences, | ||
| metricMode: mode, | ||
| }) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Switching metric mode resets the applied time-range filter.
onMetricModeChange routes through handleChartPreferencesChange, which calls setModelFilters(buildDefaultDashboardFilters(preferences)) (Lines 256-263). Since metricMode is a purely client-side display toggle over already-fetched data (ModelCharts selects count vs token specs from the same processChartData output), rebuilding filters discards any custom/quick time range the user applied and forces a data refetch on every count↔tokens switch. Consider updating/persisting metricMode without rebuilding modelFilters.
🤖 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 `@web/src/features/dashboard/index.tsx` around lines 418 - 424, The metricMode
change handler must preserve the current modelFilters instead of routing through
handleChartPreferencesChange, which rebuilds default filters and triggers a
refetch. Update the onMetricModeChange path and its preference persistence so
only the client-side metricMode setting changes while the applied custom or
quick time range remains intact.
| export type MetricMode = 'count' | 'tokens' | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Duplicate MetricMode declaration — this fails type check.
MetricMode is already declared at Line 199 in this same module. Declaring it again here produces TS2300: Duplicate identifier 'MetricMode' and breaks the build. Remove this second definition.
🐛 Proposed fix
-export type MetricMode = 'count' | 'tokens'
-📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export type MetricMode = 'count' | 'tokens' |
🤖 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 `@web/src/features/dashboard/types.ts` around lines 279 - 280, Remove the
duplicate MetricMode type declaration near the later section of the module,
keeping the existing declaration earlier in the file as the single source of
truth.
Source: Coding guidelines
1 participant
Summary
15 commits from inspect branch, including:
Summary by CodeRabbit