Merged
Conversation
Contributor
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds a Settings Dashboard feature: records per-message usage (including cached tokens), introduces a deepchat_usage_stats table, backfills historical messages, records live usage on finalize/error, exposes presenter queries, and ships a Vue dashboard UI with charts, calendar heatmap, and provider/model breakdowns. Changes
Sequence Diagram(s)sequenceDiagram
participant App as App / UI
participant Lifecycle as Lifecycle Manager
participant NAP as NewAgentPresenter
participant SQLite as SQLitePresenter
participant Messages as deepchat_messages
participant Stats as deepchat_usage_stats
App->>Lifecycle: App starts
Lifecycle->>NAP: startUsageStatsBackfill() (async, non-blocking)
activate NAP
NAP->>SQLite: listAssistantUsageCandidates()
SQLite->>Messages: query assistant messages + sessions
Messages-->>SQLite: return rows
SQLite-->>NAP: return candidates
loop per message candidate
NAP->>NAP: parse metadata, resolve provider/model
NAP->>NAP: build usage record
NAP->>SQLite: upsert(record)
SQLite->>Stats: INSERT ON CONFLICT
Stats-->>SQLite: OK
end
NAP->>NAP: set backfill status completed
deactivate NAP
App->>NAP: getUsageDashboard()
activate NAP
NAP->>SQLite: getSummary(), getDailyCalendarRows(), getProviderBreakdownRows()
SQLite->>Stats: aggregate queries
Stats-->>SQLite: results
SQLite-->>NAP: aggregated data
NAP-->>App: UsageDashboardData
deactivate NAP
sequenceDiagram
participant LLM as LLM Provider
participant Stream as Event Stream
participant Acc as Accumulator
participant Store as MessageStore
participant SQLite as SQLitePresenter
LLM->>Stream: emit usage event (input_tokens, output_tokens, cached_tokens...)
Stream->>Acc: forward usage
Acc->>Acc: store cachedInputTokens in metadata
Acc->>Store: finalize assistant message with metadata
activate Store
Store->>Store: parse metadata, resolve provider/model
Store->>SQLite: upsert deepchat_usage_stats (source='live')
SQLite-->>Store: OK
deactivate Store
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ed336bf56f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return | ||
| } | ||
|
|
||
| void newAgentPresenter.startUsageStatsBackfill().catch((error) => { |
There was a problem hiding this comment.
In the AFTER_START phase, this launches startUsageStatsBackfill() as fire-and-forget, but the existing legacyImportHook (priority 20) also starts import asynchronously and returns immediately; this means both jobs run concurrently on first upgrade. Since runUsageStatsBackfill() takes a one-time snapshot with deepchatMessagesTable.listAssistantUsageCandidates() and then marks backfill completed, any legacy messages imported after that snapshot never get rows in deepchat_usage_stats, so migrated users can see permanently undercounted dashboard totals.
Useful? React with 👍 / 👎.
![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: 20
🧹 Nitpick comments (8)
src/renderer/src/i18n/ko-KR/routes.json (1)
19-19: Consider using Korean for "Dashboard".Other route labels in this file are translated to Korean. For consistency, consider using "대시보드" (Korean transliteration) instead of leaving it in English.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/src/i18n/ko-KR/routes.json` at line 19, The value for the route key "settings-dashboard" is in English; update the JSON entry for "settings-dashboard" in src/renderer/src/i18n/ko-KR/routes.json to use the Korean translation "대시보드" so it matches the other route labels (i.e., replace "Dashboard" with "대시보드" for the "settings-dashboard" key).src/renderer/src/i18n/ja-JP/routes.json (1)
19-19: Consider using Japanese katakana for "Dashboard".Other English loanwords in this file are transliterated to katakana (e.g., "プレイグラウンド" for Playground, "プロバイダー" for Provider). For consistency, consider using "ダッシュボード" instead of "Dashboard".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/src/i18n/ja-JP/routes.json` at line 19, Replace the English string value for the "settings-dashboard" key in the routes.json i18n file with the katakana transliteration "ダッシュボード" to match the other loanword translations (e.g., "プレイグラウンド", "プロバイダー") and maintain consistency across Japanese locale strings.src/renderer/src/i18n/he-IL/routes.json (1)
19-19: Consider translating "Dashboard" to Hebrew for consistency.Other route labels in this file are translated to Hebrew (e.g., "הגדרות", "ספקים", "נתונים"), but "Dashboard" remains in English. Consider using a Hebrew equivalent like "לוח בקרה" or "לוח מחוונים" for a consistent user experience.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/src/i18n/he-IL/routes.json` at line 19, The "settings-dashboard" route label is left in English; update the translation value for the key "settings-dashboard" to a Hebrew equivalent (e.g., "לוח בקרה" or "לוח מחוונים") so it matches the other translated route labels; edit the value for the JSON key "settings-dashboard" in the routes localization file (the entry currently "settings-dashboard": "Dashboard") and replace "Dashboard" with the chosen Hebrew string, ensuring valid JSON quoting and encoding.src/main/presenter/sqlitePresenter/tables/deepchatMessages.ts (1)
197-215: Consider making usage-candidate scanning index-backed and chunked.Line 197 currently returns all assistant rows sorted by
created_atin one shot. This can get expensive as message history grows. Consider adding an index on(role, created_at)and iterating in batches for backfill paths.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/presenter/sqlitePresenter/tables/deepchatMessages.ts` around lines 197 - 215, The current listAssistantUsageCandidates method returns all assistant rows in one shot which will not scale; add a DB index on (role, created_at) (e.g., via migration or table DDL for deepchat_messages) and change listAssistantUsageCandidates to scan in chunks using a deterministic cursor (use created_at and id) with ORDER BY created_at ASC and a LIMIT batchSize, iterating until no more rows (or expose a paginated API that accepts last_seen_created_at and last_seen_id); reference the method listAssistantUsageCandidates, table deepchat_messages, and the (role, created_at) index when implementing these changes.src/shadcn/components/ui/chart/utils.ts (1)
7-7: Consider bounding the cache size to prevent unbounded memory growth.A process-wide Map without eviction can grow for long-running sessions with high-cardinality tooltip payloads.
♻️ Suggested bounded cache pattern
const cache = new Map<string, string>() +const MAX_CACHE_ENTRIES = 500 @@ - cache.set(serializedKey, div.innerHTML) + if (cache.size >= MAX_CACHE_ENTRIES) { + const oldestKey = cache.keys().next().value as string | undefined + if (oldestKey) cache.delete(oldestKey) + } + cache.set(serializedKey, div.innerHTML)Also applies to: 41-41
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shadcn/components/ui/chart/utils.ts` at line 7, The global Map named cache in utils.ts can grow without bound; replace it with a bounded cache implementation (e.g., an LRU or simple capped Map) and introduce a MAX_CACHE_SIZE constant, evicting the oldest entry when size > MAX_CACHE_SIZE to prevent unbounded memory growth; update/reuse the new bounded cache API wherever the original cache is referenced (and apply the same change to the other Map instance noted at the second occurrence) so callers (same get/set/delete semantics) continue to work.src/main/presenter/usageStats.ts (1)
150-168: Edge case:cachedInputTokensmay exceedinputTokenswheninputTokensis 0.Line 160 only clamps
cachedInputTokenswheninputTokens > 0. IfinputTokensis 0 butrawCachedhas a positive value (due to data inconsistency),cachedInputTokenswill be unclamped.Consider always clamping to ensure
cachedInputTokens <= inputTokens:♻️ Proposed fix
const rawCached = toNonNegativeInteger(metadata.cachedInputTokens) ?? 0 -const cachedInputTokens = inputTokens > 0 ? Math.min(rawCached, inputTokens) : rawCached +const cachedInputTokens = Math.min(rawCached, inputTokens)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/presenter/usageStats.ts` around lines 150 - 168, The clamp for cachedInputTokens in normalizeUsageCounts is currently only applied when inputTokens > 0, which allows rawCached to exceed inputTokens when inputTokens is 0; change the logic in normalizeUsageCounts so cachedInputTokens is always clamped to be no greater than inputTokens (e.g., set cachedInputTokens = Math.min(rawCached, inputTokens) after computing inputTokens and rawCached) while keeping the existing non-negative conversions via toNonNegativeInteger.src/main/presenter/newAgentPresenter/index.ts (1)
1417-1425: Side effect in getter method may cause unexpected behavior.
getUsageStatsBackfillStatushas a hidden side effect on lines 1421-1423 where it writes back to config when the status is 'failed' with a timeout error. This violates the principle of least surprise for a getter method.Consider either:
- Moving this persistence to
normalizeUsageStatsBackfillStatuswith a callback- Making this a separate recovery/normalization step called explicitly
♻️ Proposed refactor to separate concerns
private getUsageStatsBackfillStatus(): UsageStatsBackfillStatus { - const normalized = this.normalizeUsageStatsBackfillStatus( + const raw = this.configPresenter.getSetting<UsageStatsBackfillStatus>(DASHBOARD_STATS_BACKFILL_KEY) + const normalized = this.normalizeUsageStatsBackfillStatus(raw) + + // Persist timeout recovery as a separate explicit step + if (normalized.status === 'failed' && normalized.error === 'Usage stats backfill timed out') { + this.setUsageStatsBackfillStatus(normalized) + } + + return normalized +} - this.configPresenter.getSetting<UsageStatsBackfillStatus>(DASHBOARD_STATS_BACKFILL_KEY) - ) - if (normalized.status === 'failed' && normalized.error === 'Usage stats backfill timed out') { - this.configPresenter.setSetting(DASHBOARD_STATS_BACKFILL_KEY, normalized) - } - return normalized -}Actually, the current code is already structured this way. The concern is that calling
getSettingfollowed bysetSettingin a getter is a side effect. Consider adding a comment to document this intentional behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/presenter/newAgentPresenter/index.ts` around lines 1417 - 1425, getUsageStatsBackfillStatus currently performs an unexpected write via configPresenter.setSetting when normalizeUsageStatsBackfillStatus returns a timed-out failed status; remove that side effect from the getter by extracting the persistence into a dedicated recovery/normalization function (e.g., create recoverOrPersistUsageStatsBackfillStatus or extend normalizeUsageStatsBackfillStatus to accept a callback) and have callers invoke that explicit recovery step instead of relying on getUsageStatsBackfillStatus; ensure references to normalizeUsageStatsBackfillStatus, configPresenter.setSetting, and DASHBOARD_STATS_BACKFILL_KEY are updated to the new method so the getter becomes side-effect free.src/main/presenter/sqlitePresenter/tables/deepchatUsageStats.ts (1)
118-165: Upsert overwritescreated_aton conflict - consider preserving the original value.The
ON CONFLICT DO UPDATEclause currently replacescreated_atwith the new value. Typically,created_atshould remain immutable to reflect when the record was first inserted. Onlyupdated_atshould change on updates.♻️ Proposed fix to preserve original created_at
ON CONFLICT(message_id) DO UPDATE SET session_id = excluded.session_id, usage_date = excluded.usage_date, provider_id = excluded.provider_id, model_id = excluded.model_id, input_tokens = excluded.input_tokens, output_tokens = excluded.output_tokens, total_tokens = excluded.total_tokens, cached_input_tokens = excluded.cached_input_tokens, estimated_cost_usd = excluded.estimated_cost_usd, source = excluded.source, - created_at = excluded.created_at, updated_at = excluded.updated_at`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/presenter/sqlitePresenter/tables/deepchatUsageStats.ts` around lines 118 - 165, The upsert currently overwrites created_at on conflict in the upsert method of deepchat_usage_stats; change the ON CONFLICT(...) DO UPDATE clause in upsert (function upsert) so created_at is preserved by either removing created_at from the SET list or explicitly setting created_at = deepchat_usage_stats.created_at (the existing table value), while keeping updated_at = excluded.updated_at and the other fields updated from excluded; update the SQL string used in upsert to implement this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/presenter/deepchatAgentPresenter/messageStore.ts`:
- Around line 388-432: persistUsageStats currently lets exceptions from
parseMessageMetadata, resolveUsageProviderId/ModelId, buildUsageStatsRecord, or
usageStatsTable.upsert bubble up and potentially break message lifecycle writes;
wrap the body of persistUsageStats in a try/catch that catches any error, logs
it (without throwing) and returns early so analytics failures are swallowed.
Specifically, surround calls to parseMessageMetadata, resolveUsageProviderId,
resolveUsageModelId, buildUsageStatsRecord and usageStatsTable.upsert inside the
try block, and on catch use the presenter logger to record the error and exit
the function gracefully. Ensure existing early-return checks (e.g.,
deepchatUsageStatsTable, deepchatMessagesTable, role check, compaction check,
missing provider/model, and falsy usageRecord) remain in place before or inside
the guarded block so normal skips still occur.
In `@src/renderer/settings/components/DashboardSettings.vue`:
- Around line 882-894: The finally block in loadDashboard() calls
syncNostalgiaRotation() and scheduleRefresh() even if the component has been
unmounted (getUsageDashboard() may resolve after unmount); add a mounted check
and skip scheduling when unmounted: introduce a boolean (e.g., isUnmounted or
isMounted) set via onUnmounted/onMounted, then in loadDashboard()’s finally
return early if unmounted before calling syncNostalgiaRotation() and
scheduleRefresh(); apply the same guard to the other similar async function
referenced around lines 1157-1166.
In `@src/renderer/src/i18n/da-DK/settings.json`:
- Around line 135-208: The dashboard i18n block contains several untranslated
English strings; update the values for keys under "dashboard" (e.g., "badge",
"actions.refresh", "error.title", "error.description", "empty.title",
"empty.description", "summary.recordingStartedAt",
"summary.recordingStartedAtDescription", "calendar.title",
"calendar.description", "calendar.legend", "calendar.tooltip", and any other
English text in "summary" and "calendar") to proper Danish translations so the
entire "dashboard" section is fully localized and consistent with surrounding
Danish entries.
In `@src/renderer/src/i18n/fa-IR/routes.json`:
- Line 19: Replace the English label for the dashboard route by updating the
"settings-dashboard" value in the fa-IR routes JSON to its Persian translation
(e.g., "داشبورد") so the navigation label is localized consistently; locate the
"settings-dashboard" key and change its value from "Dashboard" to the Persian
string.
In `@src/renderer/src/i18n/fa-IR/settings.json`:
- Around line 189-262: Several dashboard strings remain in English; translate
all English values under the "dashboard" object to fa-IR to ensure consistency.
Specifically update keys including "badge", "title", "actions.refresh",
"error.title", "error.description", all "summary" labels and descriptions (e.g.,
"totalTokens", "cachedTokens", "cacheHitRate", "estimatedCost",
"recordingStartedAt", "withDeepChatDays*", "tokenUsage", "nostalgia*", etc.),
the "calendar" object ("title", "description", "legend", "tooltip"), "breakdown"
object ("providerTitle", "providerDescription", "modelTitle",
"modelDescription", "messages", "empty"), and "unavailable" — replace the
English phrases with proper Persian translations while preserving placeholders
like {days}, {date}, {count}, and {tokens}.
In `@src/renderer/src/i18n/fr-FR/routes.json`:
- Line 19: Replace the English value for the JSON key "settings-dashboard" with
the French translation "Tableau de bord" so the entry becomes localized
consistently with the rest of the file; update the value for
"settings-dashboard" in routes.json (the i18n entry) to "Tableau de bord".
In `@src/renderer/src/i18n/fr-FR/settings.json`:
- Around line 189-262: The dashboard French locale contains many untranslated
keys—update the values for dashboard.badge and dashboard.title to "Tableau de
bord d'utilisation", dashboard.actions.refresh to "Actualiser",
dashboard.error.title/description to French equivalents, dashboard.empty.title
to "Pas encore de statistiques d'utilisation" and translate the untranslated
summary keys (e.g., summary.totalTokens, summary.cachedTokens,
summary.cacheHitRate, summary.estimatedCost, summary.recordingStartedAt,
withDeepChatDaysSentence, nostalgia* labels/details) and calendar keys
(calendar.title, calendar.description, calendar.legend, calendar.tooltip) so all
strings in the dashboard block are proper French translations consistent with
the file's style.
In `@src/renderer/src/i18n/he-IL/settings.json`:
- Around line 189-262: The dashboard locale block contains untranslated English
keys (e.g., dashboard.badge, dashboard.title, dashboard.actions.refresh,
dashboard.error.title, dashboard.error.description, dashboard.empty.title,
dashboard.summary.totalTokens, dashboard.summary.cachedTokens,
dashboard.summary.cacheHitRate, dashboard.summary.estimatedCost,
dashboard.summary.recordingStartedAt, dashboard.calendar.title,
dashboard.calendar.description, dashboard.calendar.legend,
dashboard.calendar.tooltip); update these values to Hebrew translations
consistent with the surrounding file (translate the badge/title "Usage
Dashboard", "Refresh", error and empty titles/descriptions, all summary
labels/descriptions, and calendar strings) so the dashboard section is fully
localized for he-IL while preserving interpolation tokens like {days}, {count},
and {date}.
In `@src/renderer/src/i18n/ja-JP/settings.json`:
- Around line 190-261: The locale file contains untranslated English strings
(e.g., keys "badge", "title", "actions.refresh", "error.title",
"error.description", "summary.totalTokens" and related "summary.*" labels,
"calendar.title"/"calendar.description", "breakdown.*", "unavailable") causing
inconsistent UX; translate those values into Japanese, keeping placeholders
intact (e.g., "{days}", "{count}", "{date}", "{tokens}"), and ensure phrasing
matches existing Japanese style in other entries (use consistent terminology for
"token", "推定コスト", "キャッシュ", "リフレッシュ"/"更新" etc.) so each key above is replaced
with an appropriate Japanese string while preserving interpolation tokens and
punctuation.
In `@src/renderer/src/i18n/ko-KR/settings.json`:
- Around line 189-262: The dashboard localization mixes English and Korean;
update the ko-KR "dashboard" block to provide Korean translations (or remove the
English keys to rely on fallback) for the untranslated keys: "badge", "title",
"actions.refresh", "error.title", "error.description", "calendar.title",
"calendar.description", "calendar.legend", "calendar.tooltip", and
"unavailable", and verify summary labels like "totalTokens", "cachedTokens",
"cacheHitRate", "estimatedCost", "recordingStartedAt" are all properly
translated or intentionally left out; edit the JSON entries under the
"dashboard" object (e.g., dashboard.badge, dashboard.actions.refresh,
dashboard.error.title, dashboard.calendar.title, dashboard.unavailable) to
supply consistent Korean text.
In `@src/renderer/src/i18n/pt-BR/settings.json`:
- Around line 189-262: Translate the remaining English strings in the pt-BR
localization under the JSON object keys in "dashboard" (e.g., dashboard.badge,
dashboard.title, dashboard.actions.refresh, dashboard.error.title,
dashboard.error.description, dashboard.empty.title,
dashboard.summary.totalTokens, dashboard.summary.cachedTokens,
dashboard.summary.cachedTokensDescription, dashboard.summary.cacheHitRate,
dashboard.summary.cacheHitRateDescription, dashboard.summary.recordingStartedAt,
dashboard.calendar.title, dashboard.calendar.description,
dashboard.calendar.legend, dashboard.calendar.tooltip) into Brazilian Portuguese
(for example "Usage Dashboard" -> "Painel de Uso", "Refresh" -> "Atualizar",
"Failed to load dashboard" -> "Falha ao carregar o painel", etc.), ensuring
phrasing matches existing translated entries and keeping placeholder tokens like
{days}, {count}, and {date} intact.
In `@src/renderer/src/i18n/ru-RU/routes.json`:
- Line 19: The "settings-dashboard" route label in ru-RU locale is still in
English; open src/renderer/src/i18n/ru-RU/routes.json and replace the value for
the "settings-dashboard" key ("Dashboard") with a Russian translation (e.g.,
"Панель" or "Панель управления") so the navigation text is fully localized.
In `@src/renderer/src/i18n/ru-RU/settings.json`:
- Around line 189-262: Several dashboard locale keys remain in English; update
the listed keys to Russian to match the rest of the locale. Replace "badge" and
"title" with a Russian equivalent (e.g., "Панель использования"), change
"actions.refresh" to "Обновить", translate "error.title" and
"error.description", "empty.title", and all summary keys such as "totalTokens",
"cachedTokens", "cachedTokensDescription", "cacheHitRate", "estimatedCost" and
their descriptions, plus the calendar keys ("calendar.title",
"calendar.description", "calendar.legend", "calendar.tooltip") into Russian;
ensure you update the exact JSON keys shown in the diff (badge, title,
actions.refresh, error.title, error.description, empty.title, summary.* keys,
calendar.*) so the ru-RU settings.json contains Russian strings for those
entries.
In `@src/renderer/src/i18n/zh-CN/settings.json`:
- Around line 248-257: Replace the remaining English headings in the zh-CN
locale: change the "title" value "Contribution Calendar" under the contribution
calendar block to a Chinese equivalent (e.g., "贡献日历"), and update
"breakdown.providerTitle" ("Provider 排行") and "breakdown.modelTitle" ("Model
排行") to fully Chinese terms (e.g., "提供者排行" and "模型排行" or preferred translations)
so the keys "title", "breakdown.providerTitle", and "breakdown.modelTitle" are
fully localized.
In `@src/renderer/src/i18n/zh-TW/settings.json`:
- Around line 189-262: The dashboard locale includes English text in the
calendar block; replace the English values for keys calendar.title,
calendar.description and calendar.tooltip (and any other remaining English
strings in the dashboard object such as any stray "Contribution Calendar"
occurrences) with proper zh-TW translations so the UI is fully localized (e.g.,
use a zh-TW title like "貢獻日曆", a description like "最近 365 天按日統計的 token 消耗。", and
a tooltip format like "{date}：{tokens} token" or its localized equivalent),
ensuring all dashboard.* keys contain only zh-TW strings.
In `@src/shadcn/components/ui/chart/ChartContainer.vue`:
- Around line 30-33: The context and slot are exposing the raw uniqueId while
the DOM and selectors use the computed chartId; update the call to
provideChartContext and the slot props so they receive chartId instead of
uniqueId (use the computed chartId value generated from props.id ||
uniqueId.replace(/:/g, "")). Specifically, change the argument passed to
provideChartContext (and the value emitted to default slot) from uniqueId to
chartId so useChart().id matches the DOM data-chart attribute and downstream
components like ChartLegendContent can build selectors successfully.
In `@src/shadcn/components/ui/chart/ChartLegendContent.vue`:
- Around line 19-24: The computed payload mapping in the payload constant
currently overrides each item's unique key with props.nameKey when provided,
which causes duplicate v-for keys; update the payload computation (the computed
named payload that reads config.value) to keep the original entry key as the
unique key (remove the fallback to props.nameKey so key remains the entry key)
and still include props.nameKey only where needed (e.g., as a separate
displayName field if desired) so v-for keys remain unique and DOM patching is
correct.
In `@src/shadcn/components/ui/chart/ChartStyle.vue`:
- Around line 22-29: The template builds CSS rules using an unquoted attribute
selector ([data-chart=${id}]) and doesn't guard against a missing id; update the
conditional and the template string so the style is only rendered when id is
present and the selector value is quoted. Specifically, change the v-if to check
both id and colorConfig.length (e.g., v-if="id && colorConfig.length") and
update the mapping that produces `${prefix} [data-chart=${id}] {` to include
quotes around the id value (e.g., `${prefix} [data-chart="${id}"] {`) so valid
runtime IDs and empty/undefined id are handled safely.
In `@src/shadcn/components/ui/chart/ChartTooltipContent.vue`:
- Around line 97-99: The span in ChartTooltipContent.vue uses v-if="value",
which hides valid zero values; update the condition on the span that renders {{
value.toLocaleString() }} (the element referencing the component prop/variable
value) to check for null/undefined instead (e.g., use value != null) so 0 will
render while still excluding absent values.
In `@src/shadcn/components/ui/chart/utils.ts`:
- Around line 10-12: serializeKey currently uses JSON.stringify with
Object.keys(key).sort(), which only sorts top-level properties and produces
unstable cache keys for nested objects/arrays; replace it by creating a
deterministic deep-sort routine (e.g., deepSortObject / stableSerialize) that
recursively sorts object keys and normalizes arrays before serialization, then
JSON.stringify the normalized structure in serializeKey so nested payloads
produce stable keys; update any other callers or duplicate logic at the other
spots noted (lines ~33-34) to use the same deep-sort helper.
---
Nitpick comments:
In `@src/main/presenter/newAgentPresenter/index.ts`:
- Around line 1417-1425: getUsageStatsBackfillStatus currently performs an
unexpected write via configPresenter.setSetting when
normalizeUsageStatsBackfillStatus returns a timed-out failed status; remove that
side effect from the getter by extracting the persistence into a dedicated
recovery/normalization function (e.g., create
recoverOrPersistUsageStatsBackfillStatus or extend
normalizeUsageStatsBackfillStatus to accept a callback) and have callers invoke
that explicit recovery step instead of relying on getUsageStatsBackfillStatus;
ensure references to normalizeUsageStatsBackfillStatus,
configPresenter.setSetting, and DASHBOARD_STATS_BACKFILL_KEY are updated to the
new method so the getter becomes side-effect free.
In `@src/main/presenter/sqlitePresenter/tables/deepchatMessages.ts`:
- Around line 197-215: The current listAssistantUsageCandidates method returns
all assistant rows in one shot which will not scale; add a DB index on (role,
created_at) (e.g., via migration or table DDL for deepchat_messages) and change
listAssistantUsageCandidates to scan in chunks using a deterministic cursor (use
created_at and id) with ORDER BY created_at ASC and a LIMIT batchSize, iterating
until no more rows (or expose a paginated API that accepts last_seen_created_at
and last_seen_id); reference the method listAssistantUsageCandidates, table
deepchat_messages, and the (role, created_at) index when implementing these
changes.
In `@src/main/presenter/sqlitePresenter/tables/deepchatUsageStats.ts`:
- Around line 118-165: The upsert currently overwrites created_at on conflict in
the upsert method of deepchat_usage_stats; change the ON CONFLICT(...) DO UPDATE
clause in upsert (function upsert) so created_at is preserved by either removing
created_at from the SET list or explicitly setting created_at =
deepchat_usage_stats.created_at (the existing table value), while keeping
updated_at = excluded.updated_at and the other fields updated from excluded;
update the SQL string used in upsert to implement this change.
In `@src/main/presenter/usageStats.ts`:
- Around line 150-168: The clamp for cachedInputTokens in normalizeUsageCounts
is currently only applied when inputTokens > 0, which allows rawCached to exceed
inputTokens when inputTokens is 0; change the logic in normalizeUsageCounts so
cachedInputTokens is always clamped to be no greater than inputTokens (e.g., set
cachedInputTokens = Math.min(rawCached, inputTokens) after computing inputTokens
and rawCached) while keeping the existing non-negative conversions via
toNonNegativeInteger.
In `@src/renderer/src/i18n/he-IL/routes.json`:
- Line 19: The "settings-dashboard" route label is left in English; update the
translation value for the key "settings-dashboard" to a Hebrew equivalent (e.g.,
"לוח בקרה" or "לוח מחוונים") so it matches the other translated route labels;
edit the value for the JSON key "settings-dashboard" in the routes localization
file (the entry currently "settings-dashboard": "Dashboard") and replace
"Dashboard" with the chosen Hebrew string, ensuring valid JSON quoting and
encoding.
In `@src/renderer/src/i18n/ja-JP/routes.json`:
- Line 19: Replace the English string value for the "settings-dashboard" key in
the routes.json i18n file with the katakana transliteration "ダッシュボード" to match
the other loanword translations (e.g., "プレイグラウンド", "プロバイダー") and maintain
consistency across Japanese locale strings.
In `@src/renderer/src/i18n/ko-KR/routes.json`:
- Line 19: The value for the route key "settings-dashboard" is in English;
update the JSON entry for "settings-dashboard" in
src/renderer/src/i18n/ko-KR/routes.json to use the Korean translation "대시보드" so
it matches the other route labels (i.e., replace "Dashboard" with "대시보드" for the
"settings-dashboard" key).
In `@src/shadcn/components/ui/chart/utils.ts`:
- Line 7: The global Map named cache in utils.ts can grow without bound; replace
it with a bounded cache implementation (e.g., an LRU or simple capped Map) and
introduce a MAX_CACHE_SIZE constant, evicting the oldest entry when size >
MAX_CACHE_SIZE to prevent unbounded memory growth; update/reuse the new bounded
cache API wherever the original cache is referenced (and apply the same change
to the other Map instance noted at the second occurrence) so callers (same
get/set/delete semantics) continue to work.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a35b7c47-fecf-496f-be8c-d01330f61641
📒 Files selected for processing (57)
docs/specs/settings-dashboard/plan.mddocs/specs/settings-dashboard/spec.mddocs/specs/settings-dashboard/tasks.mdpackage.jsonsrc/main/presenter/deepchatAgentPresenter/accumulator.tssrc/main/presenter/deepchatAgentPresenter/messageStore.tssrc/main/presenter/deepchatAgentPresenter/process.tssrc/main/presenter/lifecyclePresenter/hooks/after-start/usageStatsBackfillHook.tssrc/main/presenter/lifecyclePresenter/hooks/index.tssrc/main/presenter/llmProviderPresenter/providers/openAICompatibleProvider.tssrc/main/presenter/llmProviderPresenter/providers/openAIResponsesProvider.tssrc/main/presenter/newAgentPresenter/index.tssrc/main/presenter/sqlitePresenter/index.tssrc/main/presenter/sqlitePresenter/tables/deepchatMessages.tssrc/main/presenter/sqlitePresenter/tables/deepchatUsageStats.tssrc/main/presenter/usageStats.tssrc/renderer/settings/components/DashboardSettings.vuesrc/renderer/settings/main.tssrc/renderer/src/i18n/da-DK/routes.jsonsrc/renderer/src/i18n/da-DK/settings.jsonsrc/renderer/src/i18n/en-US/routes.jsonsrc/renderer/src/i18n/en-US/settings.jsonsrc/renderer/src/i18n/fa-IR/routes.jsonsrc/renderer/src/i18n/fa-IR/settings.jsonsrc/renderer/src/i18n/fr-FR/routes.jsonsrc/renderer/src/i18n/fr-FR/settings.jsonsrc/renderer/src/i18n/he-IL/routes.jsonsrc/renderer/src/i18n/he-IL/settings.jsonsrc/renderer/src/i18n/ja-JP/routes.jsonsrc/renderer/src/i18n/ja-JP/settings.jsonsrc/renderer/src/i18n/ko-KR/routes.jsonsrc/renderer/src/i18n/ko-KR/settings.jsonsrc/renderer/src/i18n/pt-BR/routes.jsonsrc/renderer/src/i18n/pt-BR/settings.jsonsrc/renderer/src/i18n/ru-RU/routes.jsonsrc/renderer/src/i18n/ru-RU/settings.jsonsrc/renderer/src/i18n/zh-CN/routes.jsonsrc/renderer/src/i18n/zh-CN/settings.jsonsrc/renderer/src/i18n/zh-HK/routes.jsonsrc/renderer/src/i18n/zh-HK/settings.jsonsrc/renderer/src/i18n/zh-TW/routes.jsonsrc/renderer/src/i18n/zh-TW/settings.jsonsrc/shadcn/components/ui/chart/ChartContainer.vuesrc/shadcn/components/ui/chart/ChartLegendContent.vuesrc/shadcn/components/ui/chart/ChartStyle.vuesrc/shadcn/components/ui/chart/ChartTooltipContent.vuesrc/shadcn/components/ui/chart/index.tssrc/shadcn/components/ui/chart/utils.tssrc/shared/types/agent-interface.d.tssrc/shared/types/core/llm-events.tssrc/shared/types/presenters/new-agent.presenter.d.tstest/main/presenter/deepchatAgentPresenter/accumulator.test.tstest/main/presenter/deepchatAgentPresenter/messageStore.test.tstest/main/presenter/llmProviderPresenter/coreEvents.test.tstest/main/presenter/llmProviderPresenter/openAIResponsesProvider.test.tstest/main/presenter/newAgentPresenter/usageDashboard.test.tstest/renderer/components/DashboardSettings.test.ts
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 by CodeRabbit
New Features
Dependencies
Internationalization