feat(web-ui): heatmap, recycle bin auto-purge, and config improvements#122
Conversation
📝 WalkthroughWalkthroughThis PR introduces an hourly activity heatmap visualization for session usage, adds configurable session-trash retention settings with auto-purge behavior, and improves Claude config persistence by saving and restoring the current config selection from localStorage. ChangesHourly Usage Heatmap
Session Trash Retention Settings
Claude Config Persistence
Sequence DiagramsequenceDiagram
actor User
participant UI as Panel (Usage Card)
participant Computed as sessionUsageHourlyHeatmap()
participant Logic as buildUsageHourlyHeatmap()
participant Data as Sessions Data
User->>UI: View hourly heatmap
UI->>Computed: Render with computed property
Computed->>Logic: Call with filtered sessions & range
Logic->>Data: Iterate sessions
Data-->>Logic: Return UTC weekday/hour + metrics
Logic->>Logic: Aggregate into 7×24 grid cells
Logic-->>Computed: Return grid + maxSessionCount
Computed->>Computed: Normalize intensity levels (0–4)
Computed-->>UI: Return rows with tooltips & labels
UI->>UI: Render cells with level-* classes
UI-->>User: Display color-coded heatmap + legend
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web-ui/modules/app.methods.claude-config.mjs`:
- Around line 39-43: The call to localStorage.setItem for claudeConfigs in
saveClaudeConfigs can throw like the currentClaudeConfig write does, so wrap the
first setItem (for this.claudeConfigs) in a try/catch for consistent fault
tolerance; specifically update the saveClaudeConfigs() method to catch errors
from localStorage.setItem('claudeConfigs', JSON.stringify(this.claudeConfigs))
and swallow or log the error similarly to how the currentClaudeConfig write is
handled (preserving the existing try/catch pattern around localStorage.setItem
for currentClaudeConfig).
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 892acb54-2c62-4e42-83a9-7dddeaddd212
📒 Files selected for processing (4)
web-ui/app.jsweb-ui/modules/app.methods.claude-config.mjsweb-ui/modules/app.methods.startup-claude.mjsweb-ui/partials/index/panel-config-claude.html
📜 Review details
🔇 Additional comments (4)
web-ui/partials/index/panel-config-claude.html (1)
67-68: Looks good — Xiaomi presets follow existing preset flow.Line 67 and Line 68 correctly initialize
newClaudeConfigfields and open the modal consistently with other provider presets.web-ui/modules/app.methods.claude-config.mjs (1)
7-7: Good hardening on selection persistence writes.Line 7 and Line 150 now safely degrade on storage failures without disrupting config switching/apply flows.
Also applies to: 150-150
web-ui/modules/app.methods.startup-claude.mjs (1)
315-351: Startup reconciliation persistence update is solid.The guarded
localStorage.currentClaudeConfigwrites in match/import/fallback/clear paths are consistent and help prevent refresh-state drift.web-ui/app.js (1)
501-516: Nice startup restoration flow.Line 501-Line 516 cleanly restores the selected Claude config from storage and only backfills model when needed, which should reduce refresh flicker.
SurviveM
force-pushed
the
fix/claude-config-flicker-and-xiaomi-presets
branch
from
21fc5ae to
0552a27
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@web-ui/logic.sessions.mjs`:
- Around line 355-356: The logic function buildUsageHourlyHeatmap is emitting
hardcoded Chinese weekdayLabels; change it to return locale-neutral weekday
identifiers (e.g., ISO names like ['Mon','Tue',...'Sun'] or keys like
['weekday_1'..'weekday_7'] or numeric indices) instead of ['周一'...'周日'] so the
presentation layer can localize via i18n; update the weekdayLabels value in
buildUsageHourlyHeatmap to use neutral keys and add a brief comment that the
UI/computed layer must map those keys to localized strings.
In `@web-ui/modules/app.computed.session.mjs`:
- Line 612: The fallback tooltip string is using raw `${cell.tokenTotal}` and
can render "null"/"undefined tokens"; update the fallback branch so it
normalizes tokenTotal the same way the other branch does—replace the raw
interpolation with the guarded, localized value `(cell.tokenTotal ||
0).toLocaleString('en-US')` when constructing the string that combines
weekdayLabels[dayIndex], hourLabel, cell.sessionCount, cell.messageCount, and
the token text.
- Around line 594-602: The code uses const max = result.maxSessionCount and then
divides by max when computing ratio in the grid mapping (cells => { const ratio
= cell.sessionCount > 0 ? (cell.sessionCount / max) : 0 }), which can yield
Infinity if max is 0; update the code to guard max by replacing max with
Math.max(1, max) (e.g., compute max = Math.max(1, result.maxSessionCount) before
the grid/rows mapping) so the ratio calculation and level assignment behave like
the sibling sessionUsageHeatmap pattern; ensure you reference and update the
same max variable used in the cells mapping.
- Around line 591-597: The code reads result.weekdayLabels unguarded when lang
!== 'en', which can be undefined or shorter than grid and cause a TypeError in
rows mapping; fix by normalizing/guarding weekdayLabels before use: compute a
safe array (e.g., const weekdayLabels = lang === 'en' ? ['Mon',...,'Sun'] :
(Array.isArray(result.weekdayLabels) ? result.weekdayLabels : [])) and then when
building rows (the rows = grid.map(...) block) use a fallback for missing
entries (weekdayLabels[dayIndex] ?? '') so accessing weekdayLabels[dayIndex]
cannot throw; update the use-sites weekdayLabels, result.weekdayLabels, lang,
grid and rows accordingly.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d1e07c03-10bf-41d5-9075-8aa35a3eb62b
📒 Files selected for processing (12)
tests/unit/session-usage.test.mjstests/unit/web-ui-behavior-parity.test.mjsweb-ui/app.jsweb-ui/logic.sessions.mjsweb-ui/modules/app.computed.session.mjsweb-ui/modules/app.methods.claude-config.mjsweb-ui/modules/app.methods.session-browser.mjsweb-ui/modules/app.methods.startup-claude.mjsweb-ui/modules/i18n.dict.mjsweb-ui/partials/index/panel-config-claude.htmlweb-ui/partials/index/panel-usage.htmlweb-ui/styles/sessions-usage.css
✅ Files skipped from review due to trivial changes (2)
- web-ui/partials/index/panel-config-claude.html
- web-ui/styles/sessions-usage.css
🚧 Files skipped from review as they are similar to previous changes (3)
- web-ui/modules/app.methods.claude-config.mjs
- web-ui/modules/app.methods.startup-claude.mjs
- web-ui/app.js
📜 Review details
🔇 Additional comments (5)
web-ui/modules/i18n.dict.mjs (1)
625-629: Hourly heatmap i18n coverage looks complete.Both locales include matching keys and placeholder shapes for title/subtitle/tooltip/legend, which keeps rendering and interpolation stable.
Also applies to: 1684-1688
web-ui/modules/app.methods.session-browser.mjs (1)
777-785: Good persistence fix for usage range selection.Persisting the normalized range before reload (with storage failure isolation) is a solid fix for refresh-state flicker.
tests/unit/web-ui-behavior-parity.test.mjs (1)
595-619: Parity allowlist update is appropriate.Including
sessionUsageHourlyHeatmapkeeps computed-key parity tests aligned with the new usage feature surface.web-ui/partials/index/panel-usage.html (1)
291-319: Hourly heatmap rendering block is well wired.The grid/legend bindings and tooltip wiring are consistent with the new computed data contract.
tests/unit/session-usage.test.mjs (1)
8-8: Hourly heatmap unit coverage is solid.These tests cover the important paths (shape, aggregation correctness, range filtering, empty-state defaults) for the new helper.
Also applies to: 111-153
SurviveM
force-pushed
the
fix/claude-config-flicker-and-xiaomi-presets
branch
2 times, most recently
from
e812d46 to
b6074bb
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web-ui/app.js (1)
114-115:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove the duplicate
agentsPathkey.
agentsPathis declared twice in thedata()object. JavaScript silently uses the last definition in sloppy mode; strict-mode engines and linters (no-dupe-keys) treat it as an error.🐛 Proposed fix
- agentsPath: '', agentsPath: '',🤖 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-ui/app.js` around lines 114 - 115, Duplicate agentsPath entries in the data() object cause a dupe-key error; remove the redundant agentsPath key so only one agentsPath property remains in the data() return value (keep the intended value from whichever entry is correct), e.g., edit the data() function to have a single agentsPath property and ensure surrounding syntax (commas/braces) remains valid.
🤖 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.
Outside diff comments:
In `@web-ui/app.js`:
- Around line 114-115: Duplicate agentsPath entries in the data() object cause a
dupe-key error; remove the redundant agentsPath key so only one agentsPath
property remains in the data() return value (keep the intended value from
whichever entry is correct), e.g., edit the data() function to have a single
agentsPath property and ensure surrounding syntax (commas/braces) remains valid.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f5972123-d426-4a00-a297-608c9e6a69be
📒 Files selected for processing (9)
tests/unit/session-usage.test.mjstests/unit/web-ui-behavior-parity.test.mjsweb-ui/app.jsweb-ui/logic.sessions.mjsweb-ui/modules/app.computed.session.mjsweb-ui/modules/app.methods.session-browser.mjsweb-ui/modules/i18n.dict.mjsweb-ui/partials/index/panel-usage.htmlweb-ui/styles/sessions-usage.css
✅ Files skipped from review due to trivial changes (5)
- web-ui/modules/i18n.dict.mjs
- web-ui/modules/app.methods.session-browser.mjs
- tests/unit/web-ui-behavior-parity.test.mjs
- web-ui/styles/sessions-usage.css
- tests/unit/session-usage.test.mjs
🚧 Files skipped from review as they are similar to previous changes (2)
- web-ui/partials/index/panel-usage.html
- web-ui/modules/app.computed.session.mjs
📜 Review details
🔇 Additional comments (3)
web-ui/logic.sessions.mjs (2)
355-355: Hardcoded Chinese weekday labels in logic layer — already flagged.This was raised in a previous review:
buildUsageHourlyHeatmapshould return locale-neutral identifiers (e.g.['mon','tue','wed','thu','fri','sat','sun']) and let the UI/computed layer localize them via i18n.
304-354: LGTM —buildUsageHourlyHeatmaplogic is correct.
- Source filter,
Date.parse+Number.isFiniteguard, and range-window clamp are consistent withbuildUsageHeatmap.- The weekday formula
(getUTCDay() + 6) % 7correctly maps Mon→0 … Sun→6 in UTC, matching the grid row order.Math.max(1, maxSessionCount)correctly prevents division-by-zero in the downstream level computation.Also applies to: 356-358
web-ui/app.js (1)
161-161: LGTM — localStorage init is well-guarded.The IIFE validates against the same three accepted values as
normalizeUsageRange, and thetry/catchcoversSecurityErrorfrom restricted storage contexts. The'7d'fallback is consistent with the rest of the codebase.
SurviveM
force-pushed
the
fix/claude-config-flicker-and-xiaomi-presets
branch
from
b6074bb to
e0a40fd
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web-ui/modules/app.methods.claude-config.mjs (1)
93-97:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPersist selection in
saveAndApplyConfigsuccess path too.On Line 93,
this.currentClaudeConfigis updated, but this path does not writecurrentClaudeConfigtolocalStorage. If the user refreshes before another save/switch, persisted selection can still be stale.Suggested patch
} else { this.currentClaudeConfig = name; + try { localStorage.setItem('currentClaudeConfig', name || ''); } catch (_) {} const targetTip = res.targetPath ? `（${res.targetPath}）` : ''; this.showMessage(`已保存并应用到 Claude 配置${targetTip}`, 'success'); this.closeEditConfigModal(); this.refreshClaudeModelContext(); }🤖 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-ui/modules/app.methods.claude-config.mjs` around lines 93 - 97, In the saveAndApplyConfig success path (inside saveAndApplyConfig) you set this.currentClaudeConfig = name but don't persist it; after updating this.currentClaudeConfig add a localStorage write using the same storage key the app uses for Claude selection (e.g. localStorage.setItem('currentClaudeConfig', name)) so the choice survives a page refresh, then continue to call this.showMessage, this.closeEditConfigModal, and this.refreshClaudeModelContext as before.
🤖 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.
Outside diff comments:
In `@web-ui/modules/app.methods.claude-config.mjs`:
- Around line 93-97: In the saveAndApplyConfig success path (inside
saveAndApplyConfig) you set this.currentClaudeConfig = name but don't persist
it; after updating this.currentClaudeConfig add a localStorage write using the
same storage key the app uses for Claude selection (e.g.
localStorage.setItem('currentClaudeConfig', name)) so the choice survives a page
refresh, then continue to call this.showMessage, this.closeEditConfigModal, and
this.refreshClaudeModelContext as before.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 60861557-4282-45bb-9fb2-482d2992b157
📒 Files selected for processing (12)
tests/unit/session-usage.test.mjstests/unit/web-ui-behavior-parity.test.mjsweb-ui/app.jsweb-ui/logic.sessions.mjsweb-ui/modules/app.computed.session.mjsweb-ui/modules/app.methods.claude-config.mjsweb-ui/modules/app.methods.session-browser.mjsweb-ui/modules/app.methods.startup-claude.mjsweb-ui/modules/i18n.dict.mjsweb-ui/partials/index/panel-config-claude.htmlweb-ui/partials/index/panel-usage.htmlweb-ui/styles/sessions-usage.css
✅ Files skipped from review due to trivial changes (2)
- web-ui/modules/i18n.dict.mjs
- tests/unit/web-ui-behavior-parity.test.mjs
🚧 Files skipped from review as they are similar to previous changes (5)
- web-ui/app.js
- web-ui/modules/app.methods.session-browser.mjs
- web-ui/partials/index/panel-usage.html
- tests/unit/session-usage.test.mjs
- web-ui/styles/sessions-usage.css
📜 Review details
🔇 Additional comments (5)
web-ui/partials/index/panel-config-claude.html (1)
67-68: Xiaomi presets are wired consistently with existing provider presets.The new buttons on Line 67 and Line 68 correctly prefill
newClaudeConfigand open the modal in the same pattern as the other presets.web-ui/modules/app.methods.claude-config.mjs (1)
40-43: Good hardening for storage failures in config persistence.Guarding both
claudeConfigsandcurrentClaudeConfigwrites withtry/catchon Line 40–43 makes save flow resilient in restricted/private storage environments.web-ui/modules/app.methods.startup-claude.mjs (1)
318-350: Startup selection persistence looks correct and robust.Writing/clearing
currentClaudeConfigon the match/import/fallback branches (Line 318, 327, 343, 350) closes refresh-state gaps while safely toleratinglocalStorageerrors.web-ui/logic.sessions.mjs (1)
304-358: LGTM —buildUsageHourlyHeatmapis correct and the past locale-neutrality concern is resolved.The 7×24 grid initialization, UTC-based weekday/hour bucketing, range boundaries, and
maxSessionCountclamping are all correct and consistent with the siblingbuildUsageHeatmap. Returning numericweekdayKeysinstead of hardcoded Chinese labels (as flagged in the previous review) cleanly separates logic from presentation.web-ui/modules/app.computed.session.mjs (1)
584-630: LGTM — all three previously flagged issues are resolved;sessionUsageHourlyHeatmapis correct.
- Past issue 1 (unguarded
result.weekdayLabels):weekdayLabelsis now constructed locally (lines 591–593) and never sourced fromresult, soresult.weekdayLabelscan never be undefined here. ✅- Past issue 2 (
Math.max(1, max)guard): Line 594 correctly clampsmaxto at least1. ✅- Past issue 3 (raw
cell.tokenTotalin fallback tooltip): Line 612 uses(cell.tokenTotal || 0).toLocaleString('en-US'). ✅
Add weekday-by-hour heatmap grid (7 rows x 24 columns) to the usage page, showing session distribution across weekdays and hours with color-coded intensity levels.
Save usage time range to localStorage on change and restore on init, preventing reset to default '7d' after page refresh.
Remove the "选择日期" placeholder from the day detail dropdown and auto-select the first available day when usage data loads.
- remove duplicate agentsPath key in app.js data() - wrap localStorage.setItem for claudeConfigs in try/catch - use locale-neutral weekdayKeys in logic layer, localize in computed - guard weekdayLabels fallback and max division-by-zero - normalize fallback tooltip tokenTotal to prevent null render
SurviveM
force-pushed
the
fix/claude-config-flicker-and-xiaomi-presets
branch
from
e0a40fd to
af68afb
Compare
Add auto-purge for session trash entries older than a configurable retention period (default 30 days). Purge runs automatically on each trash list load. Retention days (1-365) is persisted in localStorage and configurable from the settings panel.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@tests/unit/session-trash-state.test.mjs`:
- Around line 2476-2483: The test title is misleading: it claims keys exist "in
both locales" but only checks template usage (indexHtmlSource) and backend
symbols (cliSource, DEFAULT_SESSION_TRASH_RETENTION_DAYS,
purgeExpiredSessionTrashEntries); either rename the test to something like
"references retention i18n key and backend symbols" or add explicit assertions
that the locale dictionaries contain the 'settings.trash.retention' key for both
language objects (e.g., assert(localesEn.includes('settings.trash.retention'))
and assert(localesZh.includes('settings.trash.retention'))), keeping the
existing checks for indexHtmlSource and cliSource intact.
In `@web-ui/partials/index/panel-usage.html`:
- Around line 99-102: The select forces a fallback to
sessionUsageDailyTableRows[0].key so clearing via clearSessionsUsageDay can't be
represented; change the binding to use only sessionsUsageSelectedDayKey
(defaulting to '' when undefined) and add a dedicated empty option (e.g.,
<option :key="'day-select-empty'" value="">) so the UI can show a cleared state,
and ensure select handler selectSessionsUsageDay continues to accept and handle
'' (empty string) as the cleared value.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3a33808f-4aac-4b84-9225-284de182e232
📒 Files selected for processing (16)
cli.jstests/unit/session-trash-state.test.mjstests/unit/session-usage.test.mjstests/unit/web-ui-behavior-parity.test.mjstests/unit/web-ui-startup-init.test.mjsweb-ui/app.jsweb-ui/logic.sessions.mjsweb-ui/modules/app.computed.session.mjsweb-ui/modules/app.methods.claude-config.mjsweb-ui/modules/app.methods.session-browser.mjsweb-ui/modules/app.methods.session-trash.mjsweb-ui/modules/i18n.dict.mjsweb-ui/partials/index/panel-settings.htmlweb-ui/partials/index/panel-usage.htmlweb-ui/styles/sessions-usage.cssweb-ui/styles/settings-panel.css
✅ Files skipped from review due to trivial changes (3)
- web-ui/styles/sessions-usage.css
- web-ui/styles/settings-panel.css
- web-ui/modules/i18n.dict.mjs
🚧 Files skipped from review as they are similar to previous changes (5)
- web-ui/modules/app.methods.claude-config.mjs
- web-ui/modules/app.computed.session.mjs
- web-ui/modules/app.methods.session-browser.mjs
- tests/unit/session-usage.test.mjs
- web-ui/logic.sessions.mjs
📜 Review details
🔇 Additional comments (10)
cli.js (3)
225-225: Default retention constant addition looks good.This introduces a clear single-source default for trash retention and keeps behavior predictable.
5605-5632: Purge implementation is solid and consistent with the retention objective.The fallback handling, cutoff computation, file deletion, and index rewrite flow all look correct.
5845-5847: Auto-purge integration before listing is correctly wired.This cleanly enables retention-based cleanup while preserving explicit opt-out via
autoPurge: false.web-ui/partials/index/panel-settings.html (1)
144-158: Looks good — retention UI wiring is consistent.The new retention card, numeric constraints, and
setSessionTrashRetentionDays(...)binding align cleanly with the state/persistence flow.tests/unit/web-ui-startup-init.test.mjs (1)
34-38: Good parity setup for mounted startup tests.Adding
normalizeSessionTrashRetentionDaysto both contexts keeps these startup tests aligned with the new initialization path.Also applies to: 140-144
web-ui/partials/index/panel-usage.html (1)
290-318: Hourly heatmap template structure is clean.Row/column iteration, level classes, and tooltip/aria-label bindings are all implemented clearly.
web-ui/app.js (1)
160-160: Good state restoration flow.The retained usage range, trash retention restore, and
currentClaudeConfigrestore-before-fallback sequence are well ordered and should stabilize startup behavior.Also applies to: 365-365, 482-517
tests/unit/web-ui-behavior-parity.test.mjs (1)
333-347: Parity updates are coherent with intended feature drift.Allowlist additions and
retentionDaysnormalization keep parity checks stable while still validating core behavior.Also applies to: 512-514, 623-624, 892-901, 943-952
web-ui/modules/app.methods.session-trash.mjs (1)
273-285: Retention methods and request wiring look solid.Normalization, persistence, and forwarding
retentionDaysinloadSessionTrashare implemented cleanly.Also applies to: 309-313
tests/unit/session-trash-state.test.mjs (1)
2277-2490: Great retention coverage expansion.The new tests cover retention purge behavior, bounds normalization, persistence, and request propagation comprehensively.
| test('retention days i18n keys exist in both locales', () => { | ||
| const zhRetention = indexHtmlSource.includes("t('settings.trash.retention')") || | ||
| indexHtmlSource.includes('t(\'settings.trash.retention\')'); | ||
| assert(zhRetention, 'retention section should reference i18n key'); | ||
|
|
||
| assert(cliSource.includes('DEFAULT_SESSION_TRASH_RETENTION_DAYS'), 'backend should define default retention constant'); | ||
| assert(cliSource.includes('purgeExpiredSessionTrashEntries'), 'backend should define auto-purge function'); | ||
| }); |
There was a problem hiding this comment.
Test title overstates what is actually verified.
The test says “exist in both locales” but only checks template key usage and backend symbols, not locale dictionaries. Rename the test or add explicit locale assertions.
✏️ Minimal corrective diff
-test('retention days i18n keys exist in both locales', () => {
+test('retention settings wiring references i18n usage and backend retention hooks', () => {🤖 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 `@tests/unit/session-trash-state.test.mjs` around lines 2476 - 2483, The test
title is misleading: it claims keys exist "in both locales" but only checks
template usage (indexHtmlSource) and backend symbols (cliSource,
DEFAULT_SESSION_TRASH_RETENTION_DAYS, purgeExpiredSessionTrashEntries); either
rename the test to something like "references retention i18n key and backend
symbols" or add explicit assertions that the locale dictionaries contain the
'settings.trash.retention' key for both language objects (e.g.,
assert(localesEn.includes('settings.trash.retention')) and
assert(localesZh.includes('settings.trash.retention'))), keeping the existing
checks for indexHtmlSource and cliSource intact.
| :value="sessionsUsageSelectedDayKey || (sessionUsageDailyTableRows[0] && sessionUsageDailyTableRows[0].key) || ''" | ||
| @change="selectSessionsUsageDay(($event.target && $event.target.value) ? String($event.target.value) : '')"> | ||
| <option value="">{{ t('usage.dayDetail.pick') }}</option> | ||
| <option v-for="day in sessionUsageDailyTableRows" :key="'day-select-' + day.key" :value="day.key">{{ day.key }}</option> | ||
| </select> |
There was a problem hiding this comment.
Day selector clear-state is no longer representable.
On Line 99, forcing :value to the first row key when sessionsUsageSelectedDayKey is empty makes clearSessionsUsageDay visually ambiguous and can desync the select UI from cleared detail state.
💡 Proposed fix
-:value="sessionsUsageSelectedDayKey || (sessionUsageDailyTableRows[0] && sessionUsageDailyTableRows[0].key) || ''"
+:value="sessionsUsageSelectedDayKey || ''"
`@change`="selectSessionsUsageDay(($event.target && $event.target.value) ? String($event.target.value) : '')">
+ <option value="">{{ t('usage.dayDetail.pick') }}</option>
<option v-for="day in sessionUsageDailyTableRows" :key="'day-select-' + day.key" :value="day.key">{{ day.key }}</option>
</select>📝 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.
| :value="sessionsUsageSelectedDayKey || (sessionUsageDailyTableRows[0] && sessionUsageDailyTableRows[0].key) || ''" | |
| @change="selectSessionsUsageDay(($event.target && $event.target.value) ? String($event.target.value) : '')"> | |
| <option value="">{{ t('usage.dayDetail.pick') }}</option> | |
| <option v-for="day in sessionUsageDailyTableRows" :key="'day-select-' + day.key" :value="day.key">{{ day.key }}</option> | |
| </select> | |
| :value="sessionsUsageSelectedDayKey || ''" | |
| `@change`="selectSessionsUsageDay(($event.target && $event.target.value) ? String($event.target.value) : '')"> | |
| <option value="">{{ t('usage.dayDetail.pick') }}</option> | |
| <option v-for="day in sessionUsageDailyTableRows" :key="'day-select-' + day.key" :value="day.key">{{ day.key }}</option> | |
| </select> |
🤖 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-ui/partials/index/panel-usage.html` around lines 99 - 102, The select
forces a fallback to sessionUsageDailyTableRows[0].key so clearing via
clearSessionsUsageDay can't be represented; change the binding to use only
sessionsUsageSelectedDayKey (defaulting to '' when undefined) and add a
dedicated empty option (e.g., <option :key="'day-select-empty'" value="">) so
the UI can show a cleared state, and ensure select handler
selectSessionsUsageDay continues to accept and handle '' (empty string) as the
cleared value.
- Resolve merge conflict in claude-config.mjs (keep try/catch fix) - Normalize retentionDays in parity test HEAD baseline calls - Add autoPurge: false to e2e stale trash test API calls
SurviveM
changed the title
2 participants
Summary
Tests