feat: split session usage into separate tab#78
Conversation
📝 WalkthroughWalkthroughThe PR refactors the usage feature from a sessions panel subtab into a top-level main navigation tab, restructuring the UI layout, updating session data loading behavior, and adjusting tests to reflect the new information hierarchy. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web-ui/session-helpers.mjs (1)
18-46:⚠️ Potential issue | 🟠 MajorUsage analytics currently inherit hidden session filters.
On Line 44, usage relies on
loadSessions(), but that loader applies session-browse filters/query (Line 97-103). Since usage tab doesn’t surface those filters, users can see partial analytics without any indication. Please decouple usage data loading from session list filters (or explicitly expose active filters in usage UI).Also applies to: 97-103
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web-ui/session-helpers.mjs` around lines 18 - 46, The usage tab is calling loadSessions() (via enteringSessionDataTab) which applies session-browse filters, so usage analytics inherit hidden filters; fix by decoupling loading logic—either add a new method (e.g., loadUsageData or loadSessions({ignoreSessionFilters: true})) and call that when nextTab === 'usage' (while keeping existing sessionsLoadedOnce for sessions), or update loadSessions to accept an options flag to bypass session-browse filters; update the entry point in the block that checks enteringSessionDataTab to call the new loadUsageData / loadSessions({ignoreSessionFilters:true}) when the target is 'usage' and ensure a separate loaded-once guard (e.g., usageLoadedOnce) is used so usage loading does not reuse session filter state.
🧹 Nitpick comments (3)
web-ui/partials/index/layout-header.html (1)
262-263: Main title nested ternary is getting brittle.Consider replacing this with a computed/lookup map for tab titles. It’ll be easier to maintain as tabs evolve.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web-ui/partials/index/layout-header.html` around lines 262 - 263, The nested ternary expression that computes the header title from mainTab (the expression using mainTab === 'config' ? '配置中心' : (mainTab === 'sessions' ? '会话浏览' : (mainTab === 'usage' ? 'Usage' : (mainTab === 'market' ? '技能市场' : '设置'))))) is brittle; replace it with a lookup map or computed property (e.g., a tabsTitleMap object or a computed getter like titlesForMainTab) that maps each mainTab key to its label and falls back to a default, then use that single lookup (titlesForMainTab[mainTab] || titlesForMainTab.default) in the template to simplify maintenance and add new tabs easily.web-ui/modules/config-mode.computed.mjs (1)
49-55: Consider centralizing main-tab label mapping.This mapping now exists in multiple places (here and header template), which can drift over time. A shared tab-label map/computed would reduce maintenance risk.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web-ui/modules/config-mode.computed.mjs` around lines 49 - 55, The inspectorMainTabLabel computed duplicates tab-to-label logic; centralize this mapping into a shared constant or computed (e.g., TAB_LABELS map and a shared computed getMainTabLabel) and have inspectorMainTabLabel return TAB_LABELS[this.mainTab] || '未知'; update the header template and any other places that currently duplicate the mapping to use the same TAB_LABELS/getMainTabLabel to avoid drift (reference symbols: inspectorMainTabLabel, mainTab, header template).web-ui/session-helpers.mjs (1)
108-117: Extract repeated “clear active session state” logic.These blocks are functionally the same and repeated across three paths. Pulling them into one helper will reduce maintenance drift and make future fixes safer.
Also applies to: 125-134, 151-160
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web-ui/session-helpers.mjs` around lines 108 - 117, Extract the repeated "clear active session state" block (the statements setting this.sessionsList, this.activeSession, this.activeSessionMessages, resetSessionDetailPagination(), resetSessionPreviewMessageRender(), this.activeSessionDetailClipped, cancelSessionTimelineSync(), this.sessionTimelineActiveKey and clearSessionTimelineRefs(this)) into a single helper function (e.g., clearActiveSessionState or resetActiveSessionState) and replace the three duplicated blocks (around the branches at the locations noted) with a call to that helper; implement it either as a private method on the same object/class or a module-level function that accepts the instance (this) to preserve context, and ensure any referenced helper calls like resetSessionDetailPagination, resetSessionPreviewMessageRender, cancelSessionTimelineSync, and clearSessionTimelineRefs are invoked from the same scope so behavior remains identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@web-ui/session-helpers.mjs`:
- Around line 18-46: The usage tab is calling loadSessions() (via
enteringSessionDataTab) which applies session-browse filters, so usage analytics
inherit hidden filters; fix by decoupling loading logic—either add a new method
(e.g., loadUsageData or loadSessions({ignoreSessionFilters: true})) and call
that when nextTab === 'usage' (while keeping existing sessionsLoadedOnce for
sessions), or update loadSessions to accept an options flag to bypass
session-browse filters; update the entry point in the block that checks
enteringSessionDataTab to call the new loadUsageData /
loadSessions({ignoreSessionFilters:true}) when the target is 'usage' and ensure
a separate loaded-once guard (e.g., usageLoadedOnce) is used so usage loading
does not reuse session filter state.
---
Nitpick comments:
In `@web-ui/modules/config-mode.computed.mjs`:
- Around line 49-55: The inspectorMainTabLabel computed duplicates tab-to-label
logic; centralize this mapping into a shared constant or computed (e.g.,
TAB_LABELS map and a shared computed getMainTabLabel) and have
inspectorMainTabLabel return TAB_LABELS[this.mainTab] || '未知'; update the header
template and any other places that currently duplicate the mapping to use the
same TAB_LABELS/getMainTabLabel to avoid drift (reference symbols:
inspectorMainTabLabel, mainTab, header template).
In `@web-ui/partials/index/layout-header.html`:
- Around line 262-263: The nested ternary expression that computes the header
title from mainTab (the expression using mainTab === 'config' ? '配置中心' :
(mainTab === 'sessions' ? '会话浏览' : (mainTab === 'usage' ? 'Usage' : (mainTab ===
'market' ? '技能市场' : '设置'))))) is brittle; replace it with a lookup map or
computed property (e.g., a tabsTitleMap object or a computed getter like
titlesForMainTab) that maps each mainTab key to its label and falls back to a
default, then use that single lookup (titlesForMainTab[mainTab] ||
titlesForMainTab.default) in the template to simplify maintenance and add new
tabs easily.
In `@web-ui/session-helpers.mjs`:
- Around line 108-117: Extract the repeated "clear active session state" block
(the statements setting this.sessionsList, this.activeSession,
this.activeSessionMessages, resetSessionDetailPagination(),
resetSessionPreviewMessageRender(), this.activeSessionDetailClipped,
cancelSessionTimelineSync(), this.sessionTimelineActiveKey and
clearSessionTimelineRefs(this)) into a single helper function (e.g.,
clearActiveSessionState or resetActiveSessionState) and replace the three
duplicated blocks (around the branches at the locations noted) with a call to
that helper; implement it either as a private method on the same object/class or
a module-level function that accepts the instance (this) to preserve context,
and ensure any referenced helper calls like resetSessionDetailPagination,
resetSessionPreviewMessageRender, cancelSessionTimelineSync, and
clearSessionTimelineRefs are invoked from the same scope so behavior remains
identical.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a43ef944-4907-49a6-8d92-8dd602fcf99c
📒 Files selected for processing (10)
tests/unit/config-tabs-ui.test.mjstests/unit/session-tab-switch-performance.test.mjstests/unit/web-ui-behavior-parity.test.mjsweb-ui/app.jsweb-ui/index.htmlweb-ui/modules/config-mode.computed.mjsweb-ui/partials/index/layout-header.htmlweb-ui/partials/index/panel-sessions.htmlweb-ui/partials/index/panel-usage.htmlweb-ui/session-helpers.mjs
💤 Files with no reviewable changes (2)
- web-ui/app.js
- web-ui/partials/index/panel-sessions.html
📜 Review details
🧰 Additional context used
🪛 HTMLHint (1.9.2)
web-ui/partials/index/panel-usage.html
[error] 2-2: Doctype must be declared before any non-comment content.
(doctype-first)
🔇 Additional comments (4)
web-ui/partials/index/panel-usage.html (1)
70-75: Good defensive handling in top-path width normalization.Using
Math.max(..., 1)plus optional chaining here avoids both divide-by-zero and undefined access edge cases.tests/unit/web-ui-behavior-parity.test.mjs (1)
323-325: Allowlist update matches the removedsessionsViewModekey.This keeps parity assertions aligned with the current app data shape after the usage-tab split.
tests/unit/session-tab-switch-performance.test.mjs (1)
88-119: Nice coverage for the usage-tab load path.This test cleanly captures the intended behavior split: usage loads session data but skips heavy session render preparation.
tests/unit/config-tabs-ui.test.mjs (1)
190-195: Good test realignment after the usage-tab split.Moving usage assertions into
usagePaneland updatingenteringSessionDataTabexpectation keeps tests aligned with the new UI/data flow.Also applies to: 297-297
awsl233777
left a comment
awsl233777
left a comment
There was a problem hiding this comment.
Checks passed and current review blockers are resolved enough to merge.
2 participants
Summary by CodeRabbit
Release Notes