Fix high-concurrency dashboard session stats#988
Merged
Conversation
📝 WalkthroughWalkthrough引入并传播系统设置开关 Changes
预估代码审查工作量🎯 3 (中等) | ⏱️ ~20 分钟 可能相关的 PR
🚥 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 |
github-actions
Bot
added
bug
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a 'High Concurrency Mode' to the dashboard, which optimizes the UI by hiding live session panels and switching metrics from concurrent sessions to Requests Per Minute (RPM) when enabled. The changes include updates to the Bento dashboard components, usage logs sections, and corresponding unit tests. Feedback suggests restoring the REFRESH_INTERVAL constant to avoid magic numbers and implementing request-level caching for system settings to prevent redundant database queries.
Contributor
🧪 测试结果
总体结果: ✅ 所有测试通过 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![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 the current code and only fix it if needed.
Inline comments:
In `@src/app/`[locale]/dashboard/logs/page.test.tsx:
- Around line 1-2: The test file page.test.tsx under
src/app/[locale]/dashboard/logs is placed outside the project testing
conventions; either move this unit test into the tests/unit/ directory (e.g.,
tests/unit/dashboard/logs/page.test.tsx) or convert it to a source-adjacent test
that matches the src/**/*.test.ts convention (rename/move to
src/app/[locale]/dashboard/logs/page.test.ts) so it follows the rule for
**/*.test.{ts,tsx}; update any import paths accordingly (references:
page.test.tsx, imports like Children/isValidElement/ReactNode and vitest
fixtures describe/expect/it/vi) to ensure test runner discovers it under the
chosen convention.
🪄 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: 3e05b930-7c0d-4a36-b35d-6b48f334808b
📒 Files selected for processing (6)
src/app/[locale]/dashboard/_components/bento/dashboard-bento.tsxsrc/app/[locale]/dashboard/_components/dashboard-bento-sections.tsxsrc/app/[locale]/dashboard/logs/_components/usage-logs-sections.tsxsrc/app/[locale]/dashboard/logs/page.test.tsxsrc/app/[locale]/dashboard/logs/page.tsxtests/unit/dashboard/dashboard-home-layout.test.tsx
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review Summary
No significant issues identified in this PR.
PR Size: M
- Lines changed: 201
- Files changed: 6
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Clean
- Type safety - Clean
- Documentation accuracy - Clean
- Test coverage - Adequate
- Code clarity - Good
Automated review by Codex AI
Contributor
There was a problem hiding this comment.
- Identified PR
#988(“Fix high-concurrency dashboard session stats”) - Calculated PR size as M (201 lines changed across 6 files) and applied label
size/M - Completed a 6‑perspective diff-scoped review and submitted the “No significant issues” review summary via
gh pr review
Contributor
🧪 测试结果
总体结果: ✅ 所有测试通过 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/unit/dashboard/logs/page.test.tsx (1)
37-84: 建议增加测试隔离性当前每个测试都重复动态导入页面模块，且没有在测试之间重置 mock 状态。虽然由于每个测试都显式设置了
mockResolvedValue，当前代码能正常工作，但建议添加beforeEach来重置 mock，提高测试隔离性和可维护性。此外，通过计算子元素数量来验证渲染结果的方式较为脆弱——如果页面结构变化（如添加包装元素），测试会失败。可考虑使用更稳定的断言方式。
建议的改进方案
+import { beforeEach } from "vitest"; + +let UsageLogsPage: typeof import("@/app/[locale]/dashboard/logs/page").default; + +beforeEach(async () => { + vi.resetAllMocks(); + vi.resetModules(); + const module = await import("@/app/[locale]/dashboard/logs/page"); + UsageLogsPage = module.default; +}); describe("UsageLogsPage", () => { it("only renders the logs data section in high concurrency mode", async () => { authMocks.getSession.mockResolvedValue({ user: { id: 7, role: "admin", }, }); systemConfigMocks.getSystemSettings.mockResolvedValue({ enableHighConcurrencyMode: true, }); - const { default: UsageLogsPage } = await import("@/app/[locale]/dashboard/logs/page"); const element = await UsageLogsPage({🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/dashboard/logs/page.test.tsx` around lines 37 - 84, Add test isolation and replace the brittle child-count assertion: add a beforeEach that calls jest.resetAllMocks() (or authMocks.getSession.mockReset() and systemConfigMocks.getSystemSettings.mockReset()) before each test, then keep the dynamic import of UsageLogsPage but change the final assertions to check for the presence or absence of specific child components instead of Children.toArray length — e.g., inspect element.props.children and assert that a child with type.name or displayName matching "ActiveSessions" exists when enableHighConcurrencyMode is false and is absent when true, and likewise assert the presence of the logs section component (e.g., type.name "LogsData") in both modes; use authMocks.getSession and systemConfigMocks.getSystemSettings to set up each scenario.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/unit/dashboard/logs/page.test.tsx`:
- Around line 37-84: Add test isolation and replace the brittle child-count
assertion: add a beforeEach that calls jest.resetAllMocks() (or
authMocks.getSession.mockReset() and
systemConfigMocks.getSystemSettings.mockReset()) before each test, then keep the
dynamic import of UsageLogsPage but change the final assertions to check for the
presence or absence of specific child components instead of Children.toArray
length — e.g., inspect element.props.children and assert that a child with
type.name or displayName matching "ActiveSessions" exists when
enableHighConcurrencyMode is false and is absent when true, and likewise assert
the presence of the logs section component (e.g., type.name "LogsData") in both
modes; use authMocks.getSession and systemConfigMocks.getSystemSettings to set
up each scenario.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5c3c59e3-4220-45c7-a53c-eb851ad98b3e
📒 Files selected for processing (1)
tests/unit/dashboard/logs/page.test.tsx
![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: 072aed3c2d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Contributor
🧪 测试结果
总体结果: ✅ 所有测试通过 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/unit/dashboard/logs/page.test.tsx (1)
38-47: 建议补上systemSettings透传断言，锁定本次 PR 的关键契约。当前只校验了“组件是否出现”，但还未验证
UsageLogsDataSection是否拿到systemSettings。建议同时断言getSystemSettings调用次数，增强对回归（例如重复读取或漏传 props）的防护。参考修改
+const usageLogsDataSectionSpy = vi.fn(); function MockUsageLogsDataSection() { + usageLogsDataSectionSpy(arguments[0]); return null; } MockUsageLogsDataSection.displayName = "UsageLogsDataSection"; beforeEach(() => { vi.clearAllMocks(); + usageLogsDataSectionSpy.mockClear(); }); @@ expect(getChildComponentNames(element)).toEqual(["UsageLogsDataSection"]); + expect(systemConfigMocks.getSystemSettings).toHaveBeenCalledTimes(1); + expect(usageLogsDataSectionSpy).toHaveBeenCalledWith( + expect.objectContaining({ + systemSettings: expect.objectContaining({ + enableHighConcurrencyMode: true, + }), + }), + ); }); @@ expect(getChildComponentNames(element)).toEqual([ "UsageLogsActiveSessionsSection", "UsageLogsDataSection", ]); + expect(systemConfigMocks.getSystemSettings).toHaveBeenCalledTimes(1); + expect(usageLogsDataSectionSpy).toHaveBeenCalledWith( + expect.objectContaining({ + systemSettings: expect.objectContaining({ + enableHighConcurrencyMode: false, + }), + }), + ); });Also applies to: 96-97, 120-123
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/dashboard/logs/page.test.tsx` around lines 38 - 47, Add assertions to verify systemSettings is forwarded: update the mocked UsageLogsDataSection (MockUsageLogsDataSection) and relevant tests to assert that getSystemSettings was called the expected number of times and that UsageLogsDataSection received the systemSettings prop (or that the mock was invoked with a props object containing systemSettings). Locate mocks for UsageLogsDataSection/UsageLogsActiveSessionsSection and the getSystemSettings spy in the test file and add assertions for call count and prop presence to lock the contract and prevent regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/unit/dashboard/logs/page.test.tsx`:
- Around line 38-47: Add assertions to verify systemSettings is forwarded:
update the mocked UsageLogsDataSection (MockUsageLogsDataSection) and relevant
tests to assert that getSystemSettings was called the expected number of times
and that UsageLogsDataSection received the systemSettings prop (or that the mock
was invoked with a props object containing systemSettings). Locate mocks for
UsageLogsDataSection/UsageLogsActiveSessionsSection and the getSystemSettings
spy in the test file and add assertions for call count and prop presence to lock
the contract and prevent regressions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1ab55b69-4987-45ca-a8b7-fe0e9eb7bf98
📒 Files selected for processing (1)
tests/unit/dashboard/logs/page.test.tsx
Contributor
🧪 测试结果
总体结果: ✅ 所有测试通过 |
github-project-automation
Bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
Merged
This was referenced Apr 24, 2026
Merged
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
In high-concurrency mode, replace the dashboard's top "Active Sessions" metric with RPM, hide the dashboard live-sessions panel, and hide the active-sessions section on the usage-logs page. Keep the normal-mode session UI unchanged by branching on
enableHighConcurrencyMode.Follow-up to #979 - adapts the dashboard UI to the high-concurrency mode introduced there, where expensive real-time session observability writes are intentionally reduced.
Problem
PR #979 added
enableHighConcurrencyMode, but the dashboard and usage-logs pages still rendered session-dependent UI:In high-concurrency mode those session figures are intentionally incomplete, so the existing UI became misleading and wasted work on queries that should not run.
Solution
Gate the affected UI on
enableHighConcurrencyMode:RPM(recentMinuteRequests) as the first dashboard cardLiveSessionsPanelChanges
src/app/[locale]/dashboard/_components/dashboard-bento-sections.tsxenableHighConcurrencyModefrom system settings intoDashboardBentosrc/app/[locale]/dashboard/_components/bento/dashboard-bento.tsxRPMand concurrent sessionsREFRESH_INTERVALconstant for session pollingsrc/app/[locale]/dashboard/logs/page.tsxUsageLogsActiveSessionsSectionin normal modecurrencyDisplaywhen rendering the logs active-sessions sectionsrc/app/[locale]/dashboard/logs/_components/usage-logs-sections.tsxUsageLogsDataSectionrequest-scoped settings reuseUsageLogsActiveSessionsSectionacceptcurrencyCodedirectly to avoid an extra settings fetch in normal modetests/unit/dashboard/dashboard-home-layout.test.tsxtests/unit/dashboard/logs/page.test.tsxLocal Verification
Targeted regression checks
bunx vitest run tests/unit/dashboard/dashboard-home-layout.test.tsx tests/unit/dashboard/logs/page.test.tsx src/app/[locale]/dashboard/logs/_components/usage-logs-sections.test.tsx3 passed,9 passedFull project checks
bun run lint0(10 warnings,1 info,0 errors)bun run typecheck0bun run build0bun run test420 passed | 1 skippedtest files,4199 passed | 3 skippedtests, exit0Local browser smoke
Using the repo dev stack at
http://127.0.0.1:23000:cch-dev-adminsystem_settings.enable_high_concurrency_mode = true/en/dashboardshowsRPMas the first card and no live-sessions panel/en/dashboard/logsdoes not render the active-sessions sectionGreptile Summary
This PR adapts the dashboard and usage-logs UI to the high-concurrency mode introduced in #979, ensuring that session-dependent UI elements (which are intentionally degraded in that mode) are hidden and replaced with more meaningful metrics.
Key changes:
dashboard-bento.tsx: WhenenableHighConcurrencyModeis on, the first metric card now shows RPM (recentMinuteRequests) instead of concurrent sessions; the active-sessions React Query is disabled (enabled: isAdmin && !enableHighConcurrencyMode); andLiveSessionsPanelis not rendered. The grid drops from a 4-column to a 3-column layout to fill the space cleanly.dashboard-bento-sections.tsx: PassesenableHighConcurrencyModefrom the server-fetchedsystemSettingsintoDashboardBento.usage-logs-sections.tsx: ConvertsUsageLogsActiveSessionsSectionfrom anasynccomponent (which fetched its owngetSystemSettings()) into a simple synchronous component that receivescurrencyCodeas a prop.UsageLogsDataSectionnow accepts an optionalsystemSettingsprop as a bypass for the internal cached fetch.logs/page.tsx: FetchessystemSettingsonce, passescurrencyDisplaydirectly toUsageLogsActiveSessionsSection, conditionally renders it only in normal mode, and forwards the full settings object toUsageLogsDataSection— resolving the duplicate DB-query concern from the previous review thread.Confidence Score: 5/5
getSystemSettingscall raised in the previous review thread is resolved by this PR:UsageLogsActiveSessionsSectionnow receivescurrencyCodeas a prop, andUsageLogsDataSectionreceives the full settings object, so only one DB round-trip is made per request in both modes.Important Files Changed
enableHighConcurrencyModeprop to conditionally show RPM vs concurrent-sessions metric, gate the active-sessions query, and hideLiveSessionsPanel; grid switches from 4- to 3-column in HCM.UsageLogsActiveSessionsSectionfrom async (fetching its own settings) to a pure sync component that acceptscurrencyCodeas a prop;UsageLogsDataSectionnow accepts an optionalsystemSettingsprop to avoid a redundant DB call.systemSettingsonce, conditionally rendersUsageLogsActiveSessionsSectiononly in normal mode, and passes the settings object down toUsageLogsDataSection, eliminating the duplicate DB query flagged in the previous thread.UsageLogsPage, using a JSX-tree helper to assert which section components are included in the output.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[DashboardBentoSection / UsageLogsPage\nfetch systemSettings once] --> B{enableHighConcurrencyMode?} B -- Yes --> C[DashboardBento\nFirst card: RPM\nSessions query: disabled\nLiveSessionsPanel: hidden\nGrid: 3-column] B -- No --> D[DashboardBento\nFirst card: Concurrent Sessions\nSessions query: enabled\nLiveSessionsPanel: shown\nGrid: 4-column] A --> E{enableHighConcurrencyMode?} E -- Yes --> F[UsageLogsPage\nSkip UsageLogsActiveSessionsSection] E -- No --> G[UsageLogsPage\nRender UsageLogsActiveSessionsSection\ncurrencyCode passed as prop] F --> H[UsageLogsDataSection\nsystemSettings passed as prop] G --> HReviews (4): Last reviewed commit: "refactor: polish high-concurrency dashbo..." | Re-trigger Greptile