Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📜 Recent review details🔇 Additional comments (6)
📝 WalkthroughWalkthrough为客户端路由列表、流式请求 Hook 和提供商行组件引入性能与行为变更：添加 ClientTypeRoutesContent 的可选 searchQuery、稳定化 providerStats 与索引映射、useStreamingRequests 的可配置节流（throttleMs），以及 provider-row 组件的 React.memo 优化和按需细化渲染；并微调 users 页面样式。 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as ClientTypeRoutesContent (UI)
participant Hook as useStreamingRequests (Hook)
participant Stream as StreamingSource
participant Timer as ThrottleTimer
UI->>Hook: 调用 useStreamingRequests({ throttleMs })
Hook->>Stream: 订阅流更新
Stream-->>Hook: 发送请求/状态更新
Hook->>Hook: 更新 activeRequestsRef（缓冲）
alt throttleMs > 0
Hook->>Timer: scheduleFlush()
Timer-->>Hook: 触发 applyState()
Hook-->>UI: 推送已节流的状态
else throttleMs == 0
Hook-->>UI: 立即 applyState() 并推送状态
end
UI->>UI: 使用 stableProviderStats 与索引映射 更新渲染与拖拽行为
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 分钟 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 unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 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 `@web/src/hooks/use-streaming.ts`:
- Around line 136-149: When throttleMs toggles to 0 the existing cleanup path
clears flushTimerRef.current before the effect that should flush runs, so the
immediate flush is skipped; update the throttleMs effect to both clear the timer
and explicitly perform the flush (e.g., call setActiveRequests(new
Map(activeRequestsRef.current)) or invoke handleRequestUpdate) after clearing
flushTimerRef.current so the pending state is applied immediately; reference
throttleMs, flushTimerRef, setActiveRequests, activeRequestsRef and
handleRequestUpdate when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fc6fc216-68a2-4034-92d8-f293d8e8ab7d
📒 Files selected for processing (4)
web/src/components/routes/ClientTypeRoutesContent.tsxweb/src/hooks/use-streaming.tsweb/src/pages/client-routes/components/provider-row.tsxweb/src/pages/users/index.tsx
📜 Review details
🔇 Additional comments (6)
web/src/pages/users/index.tsx (1)
147-147:Card外边距调整与需求一致这是纯展示层改动，满足 users 页间距优化目标，且不影响现有数据流与交互行为。
web/src/hooks/use-streaming.ts (1)
24-71: 节流状态与渲染状态分离实现合理
activeRequestsRef与setActiveRequests分离，能把高频事件更新和 UI 提交解耦，整体优化方向正确。Also applies to: 89-110
web/src/pages/client-routes/components/provider-row.tsx (2)
53-65: 行级组件 memo 化改造有效
SortableProviderRow和ProviderRowContent的自定义比较 +memo封装清晰，能直接减少列表重渲染压力。Also applies to: 146-148, 233-249, 637-638
126-141: 详情弹窗按需挂载有助于渲染开销控制仅在
showDetailsDialog为true时渲染ProviderDetailsDialog，能避免大列表场景下无效子树渲染。web/src/components/routes/ClientTypeRoutesContent.tsx (2)
61-97: Provider 统计稳定化实现到位
useStableProviderStats与对stableProviderStats的消费路径打通后，能有效降低统计对象引用抖动带来的子组件重渲染。Also applies to: 125-126, 396-397, 414-415
157-174: 索引与查找结构优化提升了拖拽健壮性
itemIds/itemsById/itemIndexById的引入以及undefined边界判断，让拖拽索引解析更稳定，避免了潜在越界与空值路径。Also applies to: 257-275, 321-326, 383-384
There was a problem hiding this comment.
🧹 Nitpick comments (2)
web/src/hooks/use-streaming.ts (2)
201-223: 设计提示：派生 hooks 不使用节流选项这些派生 hooks 调用
useStreamingRequests()时未传递options，因此不会使用新的节流功能。如果组件同时使用useStreamingRequests({ throttleMs: 100 })和这些派生 hooks，会创建独立的状态实例。这是现有设计，如果未来需要统一节流行为，可考虑使用 React Context 共享单一的 streaming 状态实例。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/hooks/use-streaming.ts` around lines 201 - 223, Derived hooks useClientStreamingCount, useProviderStreamingCount, and useProviderClientStreamingCount call useStreamingRequests() without forwarding options, so they create independent state when a caller uses useStreamingRequests({ throttleMs: ... }). Update these derived hooks to accept an optional options parameter (or accept and forward an existing options object) and pass it through to useStreamingRequests(options); alternatively, if you intend a single shared instance, refactor useStreamingRequests to expose a React Context and have these hooks read the shared context instead of calling useStreamingRequests directly—choose one approach and apply it consistently across useClientStreamingCount, useProviderStreamingCount, and useProviderClientStreamingCount.
143-154: 已修复之前的 review 问题，逻辑正确。当
throttleMs切换到 0 时，现在会正确地清除定时器并立即刷出缓冲状态，避免丢失更新。关于
void handleRequestUpdate;这一行：这是为了满足 exhaustive-deps 规则而使用的模式。由于handleRequestUpdate的依赖链最终只依赖于throttleMs，实际上这个依赖是冗余的。建议考虑使用// eslint-disable-next-line react-hooks/exhaustive-deps注释来明确说明意图，或移除这个依赖。可选：简化依赖数组
useEffect(() => { if (throttleMs <= 0) { if (flushTimerRef.current) { clearTimeout(flushTimerRef.current); flushTimerRef.current = null; } // 关闭节流时立即刷出缓冲状态，避免清理定时器后丢失一次更新。 setActiveRequests(new Map(activeRequestsRef.current)); } - // 让 effect 与最新的 handleRequestUpdate 逻辑保持一致（依赖变更触发重新刷出）。 - void handleRequestUpdate; - }, [throttleMs, handleRequestUpdate]); + }, [throttleMs]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/hooks/use-streaming.ts` around lines 143 - 154, The current useEffect includes a no-op "void handleRequestUpdate;" to satisfy exhaustive-deps; instead, explicitly document the intent by either removing handleRequestUpdate from the dependency list and adding a comment to suppress the linter or keep it and add a eslint disable line: update the effect so that when throttleMs becomes 0 it still clears flushTimerRef.current and calls setActiveRequests(new Map(activeRequestsRef.current)), and then replace the "void handleRequestUpdate;" trick with a clear directive like "// eslint-disable-next-line react-hooks/exhaustive-deps" or remove handleRequestUpdate from the deps array to avoid the redundant dependency while preserving behavior of throttleMs, flushTimerRef, setActiveRequests, activeRequestsRef, and handleRequestUpdate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@web/src/hooks/use-streaming.ts`:
- Around line 201-223: Derived hooks useClientStreamingCount,
useProviderStreamingCount, and useProviderClientStreamingCount call
useStreamingRequests() without forwarding options, so they create independent
state when a caller uses useStreamingRequests({ throttleMs: ... }). Update these
derived hooks to accept an optional options parameter (or accept and forward an
existing options object) and pass it through to useStreamingRequests(options);
alternatively, if you intend a single shared instance, refactor
useStreamingRequests to expose a React Context and have these hooks read the
shared context instead of calling useStreamingRequests directly—choose one
approach and apply it consistently across useClientStreamingCount,
useProviderStreamingCount, and useProviderClientStreamingCount.
- Around line 143-154: The current useEffect includes a no-op "void
handleRequestUpdate;" to satisfy exhaustive-deps; instead, explicitly document
the intent by either removing handleRequestUpdate from the dependency list and
adding a comment to suppress the linter or keep it and add a eslint disable
line: update the effect so that when throttleMs becomes 0 it still clears
flushTimerRef.current and calls setActiveRequests(new
Map(activeRequestsRef.current)), and then replace the "void
handleRequestUpdate;" trick with a clear directive like "//
eslint-disable-next-line react-hooks/exhaustive-deps" or remove
handleRequestUpdate from the deps array to avoid the redundant dependency while
preserving behavior of throttleMs, flushTimerRef, setActiveRequests,
activeRequestsRef, and handleRequestUpdate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4768fc7c-1e2c-4a93-9aab-773064cd7fd1
📒 Files selected for processing (1)
web/src/hooks/use-streaming.ts
📜 Review details
🔇 Additional comments (3)
web/src/hooks/use-streaming.ts (3)
24-27: LGTM!接口定义清晰，
throttleMs默认值为 0（不节流）是合理的默认行为。
48-71: LGTM!节流逻辑实现正确：
scheduleFlush使用早返回避免重复创建定时器applyState正确地通过activeRequestsRef避免闭包陈旧问题，并根据throttleMs路由到立即更新或延迟刷新
96-110: LGTM!
handleRequestUpdate正确使用activeRequestsRef.current避免闭包陈旧问题，并通过new Map()保证状态不可变性。
|
@coderabbitai re-review |
|
✅ Actions performedFull review triggered. |
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 `@web/src/hooks/use-streaming.ts`:
- Around line 133-153: When throttleMs changes we must flush any buffered state
even when the new value is still >0 to avoid dropping pending updates; update
the useEffect that depends on throttleMs to check for an existing
flushTimerRef.current (and clearTimeout it and null it) and then immediately
call setActiveRequests(new Map(activeRequestsRef.current)) so the buffered
activeRequests are committed on any throttleMs change; reference flushTimerRef,
throttleMs, setActiveRequests and activeRequestsRef to locate and modify the
effect and ensure cleanup still clears the timer.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3289267f-5ebb-437f-ae95-b44e4ad912b0
📒 Files selected for processing (1)
web/src/hooks/use-streaming.ts
|
@coderabbitai re-review |
|
✅ Actions performedFull review triggered. |
* perf: 降低路由页重渲染 * style: users 卡片外边距 * fix: 关闭节流立即刷新 * fix: 透传 streaming 参数 * fix: 节流切换立即刷新
2 participants
Summary
Testing
Summary by CodeRabbit
发布说明
性能优化
功能改进
UI 改进