fix: improve channel test failure details UX - #4988
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 (1)
WalkthroughRefactors the channel test dialog to add failure-details state, components to show truncated summaries and raw details in a responsive sheet, improved accessibility/localization in the table, and new i18n keys across six locales. ChangesChannel Test Dialog Enhancement
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
web/default/src/features/channels/components/dialogs/channel-test-dialog.tsx (1)
679-687: ⚡ Quick winUse named props types and read from
propsin the new subcomponents.The new components all inline-destructure their props. Please switch these to named prop types/interfaces and access fields via
props.xxxto match the repo convention and keep the prop contracts reusable.As per coding guidelines, "Do not destructure component props; use
props.xxxdirectly instead for clarity" and "Use functional components with Hooks; ensure single responsibility; explicitly define component prop types with interfaces or type aliases".Also applies to: 727-735, 784-790, 858-866
🤖 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 679 - 687, Replace inline prop destructuring with named prop types and read from props.xxx: define an explicit props type (e.g., interface TestStatusCellProps { result?: TestResult; model: string; onOpenDetails: (details: FailureDetailsState) => void }) and change the TestStatusCell signature to function TestStatusCell(props: TestStatusCellProps) { ... } using props.result, props.model, props.onOpenDetails; do the same for the other new subcomponents in this file that currently destructure props so each has a named props interface/type and accesses fields via props.xxx to follow the repo convention and keep prop contracts reusable.
🤖 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/default/src/features/channels/components/dialogs/channel-test-dialog.tsx`:
- Around line 633-640: The empty-state messages are raw strings and must be
localized: import and call useTranslation() in the ChannelTestDialog component
(ensure const { t } = useTranslation()), then replace the two literal strings in
the TableCell (the ternary that checks models.length) with t() lookups (e.g.
t('channels.noModelsMatched') and t('channels.noConfiguredModels') or whatever
i18n keys you choose), updating any relevant translation JSONs; keep the
conditional logic and TableCell structure unchanged.
- Around line 575-578: The div in ChannelTestDialog (the element with
role='region' in channel-test-dialog.tsx) has no accessible name; add an
accessible name by adding aria-label={t('Channel models')} or aria-labelledby
pointing to a visible heading inside the region, or remove the role='region' if
the region landmark is unnecessary—update the JSX in the ChannelTestDialog
component accordingly.
In `@web/default/src/i18n/locales/ru.json`:
- Line 3855: Replace the incorrect count-sensitive string "Проверить {{count}}
выбранных" with a grammatically correct count-neutral phrase or use ICU
pluralization; either change the value to "Проверить выбранные ({{count}})" for
a simple fix, or implement ICU plural syntax like "{count, plural, one
{Проверить {count} выбранный} few {Проверить {count} выбранных} many {Проверить
{count} выбранных} other {Проверить {count} выбранных}}" in the same JSON key so
the UI shows correct Russian forms for 1, few, many, and other counts.
---
Nitpick comments:
In
`@web/default/src/features/channels/components/dialogs/channel-test-dialog.tsx`:
- Around line 679-687: Replace inline prop destructuring with named prop types
and read from props.xxx: define an explicit props type (e.g., interface
TestStatusCellProps { result?: TestResult; model: string; onOpenDetails:
(details: FailureDetailsState) => void }) and change the TestStatusCell
signature to function TestStatusCell(props: TestStatusCellProps) { ... } using
props.result, props.model, props.onOpenDetails; do the same for the other new
subcomponents in this file that currently destructure props so each has a named
props interface/type and accesses fields via props.xxx to follow the repo
convention and keep prop contracts reusable.
🪄 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: 37caabda-3af9-43ab-8683-32536839c3db
📒 Files selected for processing (7)
web/default/src/features/channels/components/dialogs/channel-test-dialog.tsxweb/default/src/i18n/locales/en.jsonweb/default/src/i18n/locales/fr.jsonweb/default/src/i18n/locales/ja.jsonweb/default/src/i18n/locales/ru.jsonweb/default/src/i18n/locales/vi.jsonweb/default/src/i18n/locales/zh.json
| <TableRow> | ||
| <TableCell | ||
| colSpan={table.getVisibleLeafColumns().length} | ||
| className='text-muted-foreground h-16 text-center text-sm' | ||
| > | ||
| {models.length | ||
| ? 'No models matched your search.' | ||
| : 'This channel has no configured models.'} |
There was a problem hiding this comment.
Localize the empty-state copy.
These fallback messages are rendered as raw strings, so they will stay English when the rest of the dialog switches locale.
Suggested fix
- {models.length
- ? 'No models matched your search.'
- : 'This channel has no configured models.'}
+ {models.length
+ ? t('No models matched your search.')
+ : t('This channel has no configured models.')}As per coding guidelines, "All user-facing text content must support i18n using the t() function from useTranslation() in React components".
📝 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.
| <TableRow> | |
| <TableCell | |
| colSpan={table.getVisibleLeafColumns().length} | |
| className='text-muted-foreground h-16 text-center text-sm' | |
| > | |
| {models.length | |
| ? 'No models matched your search.' | |
| : 'This channel has no configured models.'} | |
| <TableRow> | |
| <TableCell | |
| colSpan={table.getVisibleLeafColumns().length} | |
| className='text-muted-foreground h-16 text-center text-sm' | |
| > | |
| {models.length | |
| ? t('No models matched your search.') | |
| : t('This channel has no configured models.')} |
🤖 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 633 - 640, The empty-state messages are raw strings and must be
localized: import and call useTranslation() in the ChannelTestDialog component
(ensure const { t } = useTranslation()), then replace the two literal strings in
the TableCell (the ternary that checks models.length) with t() lookups (e.g.
t('channels.noModelsMatched') and t('channels.noConfiguredModels') or whatever
i18n keys you choose), updating any relevant translation JSONs; keep the
conditional logic and TableCell structure unchanged.
| "Tencent": "Tencent", | ||
| "Termination requested": "Запрошено завершение", | ||
| "Test": "Проверить", | ||
| "Test {{count}} selected": "Проверить {{count}} выбранных", |
There was a problem hiding this comment.
Fix Russian count phrasing on Line 3855 to avoid grammatical errors.
"Проверить {{count}} выбранных" is incorrect for values like 1. Use count-neutral wording (e.g., "Проверить выбранные ({{count}})") or ICU pluralization if supported.
Suggested change
- "Test {{count}} selected": "Проверить {{count}} выбранных",
+ "Test {{count}} selected": "Проверить выбранные ({{count}})",🤖 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/i18n/locales/ru.json` at line 3855, Replace the incorrect
count-sensitive string "Проверить {{count}} выбранных" with a grammatically
correct count-neutral phrase or use ICU pluralization; either change the value
to "Проверить выбранные ({{count}})" for a simple fix, or implement ICU plural
syntax like "{count, plural, one {Проверить {count} выбранный} few {Проверить
{count} выбранных} many {Проверить {count} выбранных} other {Проверить {count}
выбранных}}" in the same JSON key so the UI shows correct Russian forms for 1,
few, many, and other counts.
* fix: improve channel test failure details UX * fix: add accessible label to channel models region
* fix: improve channel test failure details UX * fix: add accessible label to channel models region
* fix: improve channel test failure details UX * fix: add accessible label to channel models region
* fix: improve channel test failure details UX * fix: add accessible label to channel models region
Important
📝 变更描述 / Description
(简述:做了什么?为什么这样改能生效?请基于你对代码逻辑的理解来写,避免粘贴未经整理的内容)
优化 default 前端测试渠道弹窗的失败状态展示,避免长错误信息撑宽表格或造成移动端多层滚动。失败行现在只展示简短摘要,完整错误通过独立 Sheet 查看,并保留复制能力;同时模型列保持单行展示,操作列固定在右侧,方便用户直接重新测试。
🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix,我已提交或关联对应 Issue,且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
(请在此粘贴截图、关键日志或测试报告,以证明变更生效)
web 端:
iShot_2026-05-20_15.42.07.mp4
移动端:
iShot_2026-05-20_15.43.25.mp4
Summary by CodeRabbit
New Features
Accessibility
Localization