feat: fake200 增加 tooltip 提示#1046
Conversation
github-actions
Bot
added
enhancement
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a Fake200RetryTooltip component to explain why server-side retries are unavailable for certain streaming failures, integrating it into the SummaryTab and ProviderChainPopover components. It includes localized strings for multiple languages and updates the corresponding test files. Feedback points out a nested tooltip issue in the provider chain popover that hinders interactivity and suggests wrapping the new component in a TooltipProvider to ensure it functions correctly across various contexts.
| </div> | ||
| )} | ||
| <div>{t("logs.details.fake200ForwardedNotice")}</div> | ||
| <Fake200RetryTooltip className="text-amber-600 dark:text-amber-300" /> |
There was a problem hiding this comment.
此处存在 Tooltip 嵌套问题。外层的 Tooltip（第 177 行）默认是非交互式的（non-interactive）。当用户尝试将鼠标移入 Tooltip 内容区域去点击或悬停在 Fake200RetryTooltip 上时，外层 Tooltip 会因为鼠标离开触发器区域而立即关闭，导致用户实际上无法触发内层的 Tooltip。建议在此处直接展示解释文字，或者将外层的 Tooltip 替换为 Popover 以支持交互式内容。
|
|
||
| import { InfoIcon } from "lucide-react"; | ||
| import { useTranslations } from "next-intl"; | ||
| import { Tooltip, TooltipContent, TooltipTrigger } from "@/components/ui/tooltip"; |
There was a problem hiding this comment.
建议导入 TooltipProvider。Tooltip 组件通常需要 TooltipProvider 上下文才能正常工作。为了使 Fake200RetryTooltip 成为一个自包含且健壮的组件，建议在内部包裹它，或者确保所有调用方都提供了该上下文。
| import { Tooltip, TooltipContent, TooltipTrigger } from "@/components/ui/tooltip"; | |
| import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from "@/components/ui/tooltip"; |
| return ( | ||
| <Tooltip> | ||
| <TooltipTrigger asChild> | ||
| <button | ||
| type="button" | ||
| className={cn( | ||
| "inline-flex items-center gap-1 text-[10px] font-medium underline decoration-dotted underline-offset-2 hover:no-underline focus-visible:outline-none", | ||
| className | ||
| )} | ||
| aria-label={t("fake200RetryTooltipLabel")} | ||
| > | ||
| <span>{t("fake200RetryTooltipLabel")}</span> | ||
| <InfoIcon className="h-3 w-3 shrink-0" aria-hidden="true" /> | ||
| </button> | ||
| </TooltipTrigger> | ||
| <TooltipContent side={side} align={align} className="max-w-[320px] space-y-2"> | ||
| <div className="font-medium">{t("fake200RetryTooltipTitle")}</div> | ||
| <p>{t("fake200RetryTooltipServerRetry")}</p> | ||
| <p>{t("fake200RetryTooltipSessionFallback")}</p> | ||
| </TooltipContent> | ||
| </Tooltip> | ||
| ); |
There was a problem hiding this comment.
建议在此处包裹 TooltipProvider。目前在 SummaryTab.tsx 和 ProviderChainPopover.tsx 的底部区域使用该组件时，外部并没有提供 TooltipProvider（参考 ProviderChainPopover.tsx 第 176 行的用法），这会导致 Tooltip 无法正常显示。在组件内部包裹 Provider 可以提高其复用性。
return (
<TooltipProvider>
<Tooltip>
<TooltipTrigger asChild>
<button
type="button"
className={cn(
"inline-flex items-center gap-1 text-[10px] font-medium underline decoration-dotted underline-offset-2 hover:no-underline focus-visible:outline-none",
className
)}
aria-label={t("fake200RetryTooltipLabel")}
>
<span>{t("fake200RetryTooltipLabel")}</span>
<InfoIcon className="h-3 w-3 shrink-0" aria-hidden="true" />
</button>
</TooltipTrigger>
<TooltipContent side={side} align={align} className="max-w-[320px] space-y-2">
<div className="font-medium">{t("fake200RetryTooltipTitle")}</div>
<p>{t("fake200RetryTooltipServerRetry")}</p>
<p>{t("fake200RetryTooltipSessionFallback")}</p>
</TooltipContent>
</Tooltip>
</TooltipProvider>
);
🧪 测试结果
总体结果: ✅ 所有测试通过 |
📝 WalkthroughWalkthrough本 PR 在五种语言的仪表板本地化文件中新增 4 个与 “fake200” 重试相关的翻译键，新增 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 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 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| </div> | ||
| )} | ||
| <div>{t("logs.details.fake200ForwardedNotice")}</div> | ||
| <Fake200RetryTooltip className="text-amber-600 dark:text-amber-300" /> |
There was a problem hiding this comment.
Tooltip inside TooltipContent
Fake200RetryTooltip renders its own <Tooltip>/<TooltipTrigger>/<TooltipContent> stack, but here it's placed inside the outer TooltipContent. Radix UI tooltips persist while the cursor stays inside content, so the outer tooltip will remain open – but clicking the inner trigger button (which fires onPointerDown) can cause the outer tooltip to dismiss before the inner tooltip content appears, making the inner tooltip effectively unreachable for pointer users. Consider replacing the inner tooltip with a Popover or moving the retry explanation text inline.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/[locale]/dashboard/logs/_components/provider-chain-popover.tsx
Line: 238
Comment:
**Nested `Tooltip` inside `TooltipContent`**
`Fake200RetryTooltip` renders its own `<Tooltip>/<TooltipTrigger>/<TooltipContent>` stack, but here it's placed inside the outer `TooltipContent`. Radix UI tooltips persist while the cursor stays inside content, so the outer tooltip will remain open – but clicking the inner trigger button (which fires `onPointerDown`) can cause the outer tooltip to dismiss before the inner tooltip content appears, making the inner tooltip effectively unreachable for pointer users. Consider replacing the inner tooltip with a `Popover` or moving the retry explanation text inline.
How can I resolve this? If you propose a fix, please make it concise.| <button | ||
| type="button" | ||
| className={cn( | ||
| "inline-flex items-center gap-1 text-[10px] font-medium underline decoration-dotted underline-offset-2 hover:no-underline focus-visible:outline-none", | ||
| className | ||
| )} | ||
| aria-label={t("fake200RetryTooltipLabel")} | ||
| > | ||
| <span>{t("fake200RetryTooltipLabel")}</span> | ||
| <InfoIcon className="h-3 w-3 shrink-0" aria-hidden="true" /> | ||
| </button> |
There was a problem hiding this comment.
aria-label duplicates visible text
The aria-label is set to the same value as the <span> text rendered inside the button. When a button already has visible text content, aria-label overrides (rather than supplements) the accessible name, which is the same string in this case — making it purely redundant. Removing the aria-label and relying on the visible text is simpler and avoids any future divergence if the label key is changed independently.
| <button | |
| type="button" | |
| className={cn( | |
| "inline-flex items-center gap-1 text-[10px] font-medium underline decoration-dotted underline-offset-2 hover:no-underline focus-visible:outline-none", | |
| className | |
| )} | |
| aria-label={t("fake200RetryTooltipLabel")} | |
| > | |
| <span>{t("fake200RetryTooltipLabel")}</span> | |
| <InfoIcon className="h-3 w-3 shrink-0" aria-hidden="true" /> | |
| </button> | |
| className={cn( | |
| "inline-flex items-center gap-1 text-[10px] font-medium underline decoration-dotted underline-offset-2 hover:no-underline focus-visible:outline-none", | |
| className | |
| )} |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/[locale]/dashboard/logs/_components/fake200-retry-tooltip.tsx
Line: 24-34
Comment:
**Redundant `aria-label` duplicates visible text**
The `aria-label` is set to the same value as the `<span>` text rendered inside the button. When a button already has visible text content, `aria-label` overrides (rather than supplements) the accessible name, which is the same string in this case — making it purely redundant. Removing the `aria-label` and relying on the visible text is simpler and avoids any future divergence if the label key is changed independently.
```suggestion
className={cn(
"inline-flex items-center gap-1 text-[10px] font-medium underline decoration-dotted underline-offset-2 hover:no-underline focus-visible:outline-none",
className
)}
```
How can I resolve this? If you propose a fix, please make it concise.![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/_components/fake200-retry-tooltip.tsx:
- Around line 24-34: The button in fake200-retry-tooltip.tsx removes the native
focus outline (class includes "focus-visible:outline-none") leaving keyboard
users with no visible focus; update the button's className to preserve an
accessible focus indicator (e.g. replace "focus-visible:outline-none" with a
visible focus style such as "focus-visible:ring-2 focus-visible:ring-offset-2
focus-visible:ring-primary" or an equivalent focus-visible outline class used
across the app) so the element (the <button> that renders
t("fake200RetryTooltipLabel") and InfoIcon) shows a clear focus ring when
focused via keyboard.
In `@src/app/`[locale]/dashboard/logs/_components/provider-chain-popover.tsx:
- Line 238: The inner interactive tooltip component Fake200RetryTooltip is
nested inside the outer TooltipContent, which creates nested/focusable tooltip
triggers and breaks keyboard/touch UX; fix by removing the interactive component
from inside TooltipContent and either (a) inline its explanatory text directly
into TooltipContent (render plain text/markup instead of the button trigger) or
(b) replace the outer Tooltip with a Popover so the inner Fake200RetryTooltip
can remain interactive; update usages around TooltipContent and
Fake200RetryTooltip (or move Fake200RetryTooltip’s trigger outside the outer
Tooltip) to ensure only one interactive tooltip trigger is active at a time.
🪄 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: 425639ec-a5bb-4d28-a3c2-c3e6893cdbf2
📒 Files selected for processing (10)
messages/en/dashboard.jsonmessages/ja/dashboard.jsonmessages/ru/dashboard.jsonmessages/zh-CN/dashboard.jsonmessages/zh-TW/dashboard.jsonsrc/app/[locale]/dashboard/logs/_components/error-details-dialog.test.tsxsrc/app/[locale]/dashboard/logs/_components/error-details-dialog/components/SummaryTab.tsxsrc/app/[locale]/dashboard/logs/_components/fake200-retry-tooltip.tsxsrc/app/[locale]/dashboard/logs/_components/provider-chain-popover.test.tsxsrc/app/[locale]/dashboard/logs/_components/provider-chain-popover.tsx
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Summary
This PR adds a helpful tooltip explaining why fake200 errors cannot be retried server-side. The implementation is clean and well-tested, with one minor accessibility improvement suggestion.
PR Size: S
- Lines changed: 128 (126 additions, 2 deletions)
- Files changed: 10
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 1 |
Low Priority Issues (Author's Discretion)
src/app/[locale]/dashboard/logs/_components/fake200-retry-tooltip.tsx:25
[LOW] [SIMPLIFY-READABILITY] Redundant aria-label duplicates visible text
The button has an aria-label set to the same value as the visible text inside the <span>. When a button contains visible text content, screen readers will use that text as the accessible name, making the aria-label redundant.
Suggested fix:
<button
type=button
className={cn(
"inline-flex items-center gap-1 text-[10px] font-medium underline decoration-dotted underline-offset-2 hover:no-underline focus-visible:outline-none",
className
)}
>
<span>{t("fake200RetryTooltipLabel")}</span>
<InfoIcon className="h-3 w-3 shrink-0" aria-hidden="true" />
</button>Review Coverage
- Logic and correctness - Clean, no logic changes
- Security (OWASP Top 10) - Clean, no security-sensitive code
- Error handling - Clean, UI-only change
- Type safety - Clean, proper TypeScript interfaces
- Documentation accuracy - Comments explain the fake200 context well
- Test coverage - Tests verify tooltip content is rendered
- Code clarity - Component is well-structured and reusable
Notes
- All 5 locale files have complete i18n translations
- The tooltip is properly integrated in both SummaryTab and ProviderChainPopover
- Test files mock the tooltip components and verify content strings are rendered
- The nested tooltip pattern in ProviderChainPopover is acceptable given Radix UI's implementation
Automated review by Claude AI
There was a problem hiding this comment.
Code Review Summary
No significant issues identified in this PR.
PR Size: M
- Lines changed: 128
- Files changed: 10
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
There was a problem hiding this comment.
- Reviewed PR
#1046(feat: fake200 增加 tooltip 提示) across the 6 required perspectives, scoped to diff-only lines. - PR size computed as M (
128lines changed,10files changed) and applied labelsize/M. - No issues met the ≥80 confidence threshold after validation, so no inline comments were posted.
- Submitted the required summary review comment via
gh pr review --comment.
🧪 测试结果
总体结果: ✅ 所有测试通过 |
github-project-automation
Bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
1 participant
Summary
Problem
Users encountering FAKE_200 errors (e.g.,
FAKE_200_JSON_ERROR_MESSAGE_NON_EMPTY) were confused why CCH did not retry or switch providers automatically. This is particularly frustrating when the upstream provider returns HTTP 200 with an error message buried in the SSE stream—by the time the error is detected, the response has already been committed to the client.Related Issues:
Related PRs:
Solution
Added a reusable
Fake200RetryTooltipcomponent that appears alongside fake200 error notices in:SummaryTab.tsx) - Shows when viewing failed request detailsprovider-chain-popover.tsx) - Shows in the provider chain visualizationThe tooltip explains:
Changes
Core Changes
src/app/[locale]/dashboard/logs/_components/fake200-retry-tooltip.tsx- New shared tooltip component with i18n supportsrc/app/[locale]/dashboard/logs/_components/error-details-dialog/components/SummaryTab.tsx- Integrated tooltip into fake200 error noticesrc/app/[locale]/dashboard/logs/_components/provider-chain-popover.tsx- Integrated tooltip into provider chain fake200 noticesSupporting Changes
messages/en/dashboard.json- Added 4 i18n keys for tooltip contentmessages/ja/dashboard.json- Japanese translationsmessages/ru/dashboard.json- Russian translationsmessages/zh-CN/dashboard.json- Simplified Chinese translationsmessages/zh-TW/dashboard.json- Traditional Chinese translationssrc/app/[locale]/dashboard/logs/_components/error-details-dialog.test.tsx- Added tooltip assertionssrc/app/[locale]/dashboard/logs/_components/provider-chain-popover.test.tsx- Added tooltip assertionsCircuit Breaker Check
recordFailure(...)recordEndpointFailure(...)404still stays out of circuit-breaker accountingVerification
bun run buildbun run lintbun run lint:fixbun run typecheckbun run testbunx vitest run 'src/app/[locale]/dashboard/logs/_components/provider-chain-popover.test.tsx'bunx vitest run 'src/app/[locale]/dashboard/logs/_components/error-details-dialog.test.tsx'bunx vitest run 'tests/unit/proxy/response-handler-endpoint-circuit-isolation.test.ts'bunx vitest run 'tests/unit/proxy/proxy-forwarder-fake-200-html.test.ts'Screenshot
!fake200 tooltip verification
Description enhanced by Claude AI
Greptile Summary
This PR adds a
Fake200RetryTooltipcomponent that explains why CCH cannot server-retry FAKE_200 streaming errors, integrating it into the Error Details dialog and the Provider Chain popover across all 5 supported locales.Previous review feedback (nested
TooltipinsideTooltipContent, redundantaria-label) has been fully addressed: the single-request tooltip path now inlines the explanation text directly rather than embedding the component, and thearia-labelwas removed.Confidence Score: 5/5
Safe to merge; all prior P1 concerns are resolved and the sole remaining finding is a P2 copy-divergence note
All findings are P2 or lower. The nested-tooltip and aria-label issues from the previous round have been addressed. The one remaining note (inline copy of tooltip text in the single-request path) is a deliberate trade-off to avoid the nested-tooltip interaction bug and carries no correctness risk.
provider-chain-popover.tsx — contains a manual inline copy of the tooltip text that will need to stay in sync with Fake200RetryTooltip
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Log entry with FAKE_200 error] --> B{Display context} B -->|Error Details Dialog SummaryTab| C[Amber callout box] C --> D[Fake200RetryTooltip component] D --> E[Tooltip: why no server retry?] B -->|Provider Chain Popover multi-request path| F[PopoverContent footer] F --> G[Fake200RetryTooltip component] G --> E B -->|Provider Chain Popover single-request path| H[TooltipContent inline block] H --> I[Inline copy of tooltip text - no nested Tooltip] E --> J[fake200RetryTooltipTitle / fake200RetryTooltipServerRetry / fake200RetryTooltipSessionFallback] I --> JPrompt To Fix All With AI
Reviews (2): Last reviewed commit: "fix: address fake200 tooltip review feed..." | Re-trigger Greptile