fix(channels): refresh channel test dialog status - #5517
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughTwo independent areas are modified: the generic ChangesDataTable cellRenderColumns Memo Prop
Channel Test: Cache Patching, Streaming Control, and Response Time
Sequence Diagram(s)sequenceDiagram
participant User
participant ChannelTestDialogContent
participant testSingleModel
participant handleTestChannel
participant updateChannelTestCache
participant refreshChannelLists
participant QueryClient
User->>ChannelTestDialogContent: Start batch test
loop For each model (deferRefresh=true)
ChannelTestDialogContent->>testSingleModel: testSingleModel(model, {deferRefresh: true})
testSingleModel->>handleTestChannel: call with success/error callbacks
handleTestChannel-->>testSingleModel: onTestComplete(result, responseTime, completedAt)
testSingleModel->>updateChannelTestCache: patch local cache entry
updateChannelTestCache->>QueryClient: setQueryData(channelId, response_time, test_time)
end
ChannelTestDialogContent->>refreshChannelLists: call with latestPatch (from getLatestPatch)
refreshChannelLists->>QueryClient: invalidateQueries(channelsQueryKeys)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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)
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.
🧹 Nitpick comments (2)
web/default/src/features/channels/components/dialogs/channel-test-dialog.tsx (1)
284-288: 💤 Low valueProps are destructured in the function signature.
Per coding guidelines, component props should be accessed via
props.xxxdirectly rather than destructured. While this is an internal component, consistency with the codebase pattern would improve clarity.-function ChannelTestDialogContent({ - open, - onOpenChange, - currentRow, -}: ChannelTestDialogContentProps) { +function ChannelTestDialogContent( + props: ChannelTestDialogContentProps +) { const { t } = useTranslation() const queryClient = useQueryClient() - const currentChannelId = currentRow.id + const currentChannelId = props.currentRow.idThen use
props.open,props.onOpenChange, andprops.currentRowthroughout.🤖 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/default/src/features/channels/components/dialogs/channel-test-dialog.tsx` around lines 284 - 288, The ChannelTestDialogContent function is destructuring props in the function signature, but the codebase pattern prefers accessing props directly. Change the function parameter from destructured props (open, onOpenChange, currentRow) to accept a single props parameter of type ChannelTestDialogContentProps, and then update all references throughout the component to use props.open, props.onOpenChange, and props.currentRow instead of the destructured variable names.Source: Coding guidelines
web/default/src/features/channels/lib/channel-actions.ts (1)
55-72: 💤 Low valueConsider aligning the validation logic for both response time sources.
The first path accepts
responseTime >= 0(any finite number), while the fallback path requiresresponse.time > 0. If a response time of exactly0is valid fromdata.response_time, it should arguably also be valid fromtime(after conversion). This asymmetry could cause the fallback to silently discard a legitimate zero-millisecond result.If zero is never a valid response time in practice, consider adding
&& responseTime > 0to the first condition for consistency; otherwise, relax the second condition to>= 0.🤖 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/default/src/features/channels/lib/channel-actions.ts` around lines 55 - 72, The validation logic in the getChannelTestResponseTime function is inconsistent between the two response time sources: the first condition (for response.data?.response_time) accepts any finite number including 0, while the second condition (for response.time) explicitly requires response.time > 0. This asymmetry could cause a valid zero-millisecond response time to be silently rejected in the fallback path. Determine whether zero is a valid response time in your domain and align both conditions accordingly: either add && responseTime > 0 to the first condition if zero should never be valid, or change the second condition from response.time > 0 to response.time >= 0 if zero is legitimate.
🤖 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.
Nitpick comments:
In
`@web/default/src/features/channels/components/dialogs/channel-test-dialog.tsx`:
- Around line 284-288: The ChannelTestDialogContent function is destructuring
props in the function signature, but the codebase pattern prefers accessing
props directly. Change the function parameter from destructured props (open,
onOpenChange, currentRow) to accept a single props parameter of type
ChannelTestDialogContentProps, and then update all references throughout the
component to use props.open, props.onOpenChange, and props.currentRow instead of
the destructured variable names.
In `@web/default/src/features/channels/lib/channel-actions.ts`:
- Around line 55-72: The validation logic in the getChannelTestResponseTime
function is inconsistent between the two response time sources: the first
condition (for response.data?.response_time) accepts any finite number including
0, while the second condition (for response.time) explicitly requires
response.time > 0. This asymmetry could cause a valid zero-millisecond response
time to be silently rejected in the fallback path. Determine whether zero is a
valid response time in your domain and align both conditions accordingly: either
add && responseTime > 0 to the first condition if zero should never be valid, or
change the second condition from response.time > 0 to response.time >= 0 if zero
is legitimate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ae471fe7-2b21-4940-9c44-489670812639
📒 Files selected for processing (5)
web/default/src/components/data-table/core/data-table-row.tsxweb/default/src/components/data-table/core/data-table-view.tsxweb/default/src/features/channels/components/dialogs/channel-test-dialog.tsxweb/default/src/features/channels/lib/channel-actions.tsweb/default/src/features/channels/types.ts
Add a newline at the end of the file.
Upstream changes (50+ commits, v1.0.0-rc.11): - data-table perf: row selection memo, column pinning, badge display - Fixes: channel test dialog (QuantumNous#5517), CC Switch model selector (QuantumNous#5515), API key form options (QuantumNous#5512), cell overflow (QuantumNous#5510), kimi k2.6 temp (QuantumNous#5390), Anthropic-compatible GLM chunked encoding (QuantumNous#5307), streaming image relay (QuantumNous#4608) - Feat: audit auth method tracking (QuantumNous#5462), channel affinity clear toggle (QuantumNous#5306), relay idle timeout config (QuantumNous#5309), 6-decimal pricing precision (QuantumNous#5332) - Classic frontend: Rsbuild support, Semi React 19 adapter - Shared dialog wrapper, JSON code editor, debounce channel search Conflict resolved: web/bun.lock (accepted upstream, will regenerate) Co-Authored-By: Claude <noreply@anthropic.com>
Merge upstream v1.0.0-rc.11 (50+ commits): - data-table perf: row selection memo, column pinning, badge display - Fixes: channel test dialog (QuantumNous#5517), CC Switch (QuantumNous#5515), API key (QuantumNous#5512), kimi k2.6 temp (QuantumNous#5390), GLM chunked encoding (QuantumNous#5307), streaming image (QuantumNous#4608) - Feat: audit auth tracking (QuantumNous#5462), channel affinity toggle (QuantumNous#5306), relay idle timeout (QuantumNous#5309), 6-decimal pricing (QuantumNous#5332) - Shared dialog wrapper, JSON code editor, classic Rsbuild support SEO optimization: - robots.txt: 10 AI crawler blocks + 22 path disallows + crawl-delay - sitemap.xml: 7 public URLs with 6-language hreflang annotations - index.html: hreflang tags, og:locale:alternate, og:image, canonical, 5 structured data types (Organization, SoftwareApplication, FAQPage, WebSite, SearchAction), expanded keywords (gateway, agent router, aggregation, orchestration) - i18n/config.ts: sync <html lang> with active language for SEO Co-Authored-By: Claude <noreply@anthropic.com>
Merge pull request QuantumNous#5517 from yyhhyyyyyy/fix/channel-test-dialog-status-refresh
Merge pull request QuantumNous#5517 from yyhhyyyyyy/fix/channel-test-dialog-status-refresh
2 participants
Important
📝 变更描述 / Description
(简述：做了什么？为什么这样改能生效？请基于你对代码逻辑的理解来写，避免粘贴未经整理的内容)
time字段，让主列表的响应时间和最后测试时间能及时回填。弹窗内部状态重置也从useEffect同步 setState 改为 remount/事件驱动，消除 React hooks lint 报错。🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix，我已提交或关联对应 Issue，且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
(请在此粘贴截图、关键日志或测试报告，以证明变更生效)
未修改前：
修改后：
Summary by CodeRabbit
Release Notes
New Features
Improvements