feat(providers): expose vendor endpoint pools in settings UI#719
feat(providers): expose vendor endpoint pools in settings UI#719
Conversation
📝 WalkthroughWalkthrough新增提供商端点管理的本地化字符串与错误提示，引入端点状态模型与 UI 组件（悬停列表、端点表格、端点池表单集成），对提供商视图与表单进行重构并添加大量单元测试与少量配置调整。 Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 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 |
Summary of ChangesHello @ding113, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly upgrades the user interface for managing provider endpoints within the settings. It introduces dedicated components for displaying endpoint health, providing detailed endpoint summaries on hover, and centralizing all endpoint creation, modification, and deletion functionalities. These changes aim to provide a more intuitive and consistent experience for users interacting with provider endpoint configurations. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
github-actions
bot
added
size/XL
![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 is a great addition, exposing vendor endpoint pools in the settings UI. The changes are well-structured, with new components for endpoint status display, a hover-over for endpoint details, and a reusable CRUD table for endpoints. The refactoring in provider-vendor-view.tsx to use the new reusable ProviderEndpointsSection component is a significant improvement in code organization and maintainability. The addition of comprehensive unit tests for the new functionality is also highly commendable. I have one suggestion to improve UI consistency.
| onClick={() => { | ||
| if (confirm(t("confirmDeleteEndpoint"))) { | ||
| deleteMutation.mutate(); | ||
| } | ||
| }} |
There was a problem hiding this comment.
The delete confirmation uses window.confirm(), which is a blocking browser dialog. This is inconsistent with other parts of the application (e.g., deleting a vendor uses a non-blocking AlertDialog). Using an AlertDialog here would provide a more consistent and modern user experience, improving the overall feel of the interface.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| <div | ||
| className="flex items-center gap-1.5 cursor-help opacity-80 hover:opacity-100 transition-opacity focus:outline-none focus:ring-2 focus:ring-ring focus:ring-offset-2 rounded px-1" | ||
| tabIndex={0} | ||
| role="button" | ||
| aria-label={t("endpointStatus.viewDetails", { count })} | ||
| data-testid="endpoint-hover-trigger" | ||
| > |
There was a problem hiding this comment.
[P1] i18n placeholder mismatch: t("endpointStatus.viewDetails", { count }) passes count, but messages/*/settings/providers/strings.json defines endpointStatus.viewDetails without a {count} placeholder. This will either be ignored or can surface as an intl formatting error depending on next-intl config, and it also means screen readers won’t get the count you intended.
Consider either removing the { count } argument or updating the translation string to include {count} (e.g. "View details ({count})").
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/[locale]/settings/providers/_components/provider-endpoint-hover.tsx
Line: 62:68
Comment:
[P1] i18n placeholder mismatch: `t("endpointStatus.viewDetails", { count })` passes `count`, but `messages/*/settings/providers/strings.json` defines `endpointStatus.viewDetails` without a `{count}` placeholder. This will either be ignored or can surface as an intl formatting error depending on `next-intl` config, and it also means screen readers won’t get the count you intended.
Consider either removing the `{ count }` argument or updating the translation string to include `{count}` (e.g. "View details ({count})").
How can I resolve this? If you propose a fix, please make it concise.| {endpoint.lastProbeLatencyMs && ( | ||
| <span className="text-[10px] text-muted-foreground tabular-nums shrink-0"> | ||
| {endpoint.lastProbeLatencyMs}ms | ||
| </span> | ||
| )} |
There was a problem hiding this comment.
[P2] Latency display drops valid 0 values: endpoint.lastProbeLatencyMs && (...) treats 0 as falsy, so an endpoint that reports 0ms (or is mocked/tests with 0) won’t show latency. If 0 is a valid value, prefer a null/undefined check (e.g. endpoint.lastProbeLatencyMs != null).
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/[locale]/settings/providers/_components/provider-endpoint-hover.tsx
Line: 127:131
Comment:
[P2] Latency display drops valid `0` values: `endpoint.lastProbeLatencyMs && (...)` treats `0` as falsy, so an endpoint that reports 0ms (or is mocked/tests with 0) won’t show latency. If `0` is a valid value, prefer a null/undefined check (e.g. `endpoint.lastProbeLatencyMs != null`).
How can I resolve this? If you propose a fix, please make it concise.| // Build query key based on whether we filter by type | ||
| const queryKey = providerType | ||
| ? ["provider-endpoints", vendorId, providerType, queryKeySuffix].filter(Boolean) | ||
| : ["provider-endpoints", vendorId, queryKeySuffix].filter(Boolean); | ||
|
|
There was a problem hiding this comment.
[P2] queryKey uses .filter(Boolean), which will drop valid falsy values (e.g. 0, ""). If queryKeySuffix is intentionally allowed to be ""/0 for isolation, this will collapse distinct caches and lead to stale/mixed data. Prefer filtering only null/undefined (e.g. filter((v) => v != null)).
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/[locale]/settings/providers/_components/provider-endpoints-table.tsx
Line: 97:101
Comment:
[P2] `queryKey` uses `.filter(Boolean)`, which will drop valid falsy values (e.g. `0`, `""`). If `queryKeySuffix` is intentionally allowed to be `""`/`0` for isolation, this will collapse distinct caches and lead to stale/mixed data. Prefer filtering only `null`/`undefined` (e.g. `filter((v) => v != null)`).
How can I resolve this? If you propose a fix, please make it concise.![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| }); | ||
| } | ||
| } else { | ||
| toast.error(res.error || t("endpointAddFailed")); |
There was a problem hiding this comment.
[HIGH] [STANDARD-VIOLATION] UI surfaces non-i18n backend res.error strings on endpoint create
Why this is a problem: CLAUDE.md states: 3. **i18n Required** - All user-facing strings must use i18n (5 languages supported). Never hardcode display text. Here the UI renders res.error directly:
toast.error(res.error || t("endpointAddFailed"));The backing action returns non-localized error text alongside errorCode (e.g. Chinese messages in src/actions/provider-endpoints.ts), so users can see the wrong language and we bypass the existing getErrorMessage mapping.
Suggested fix:
import { getErrorMessage } from "@/lib/utils/error-messages";
// inside AddEndpointButton
const tErrors = useTranslations("errors");
// ...
} else {
toast.error(res.errorCode ? getErrorMessage(tErrors, res.errorCode) : t("endpointAddFailed"));
}
| setOpen(false); | ||
| queryClient.invalidateQueries({ queryKey: ["provider-endpoints"] }); | ||
| } else { | ||
| toast.error(res.error || t("endpointUpdateFailed")); |
There was a problem hiding this comment.
[HIGH] [STANDARD-VIOLATION] UI surfaces non-i18n backend res.error strings on endpoint edit
Why this is a problem: CLAUDE.md states: 3. **i18n Required** - All user-facing strings must use i18n (5 languages supported). Never hardcode display text. Here the UI renders res.error directly:
toast.error(res.error || t("endpointUpdateFailed"));The action returns errorCode, so we should translate via getErrorMessage(...) (consistent with provider-vendor-view.tsx) instead of showing raw backend text.
Suggested fix:
import { getErrorMessage } from "@/lib/utils/error-messages";
// inside EditEndpointDialog
const tErrors = useTranslations("errors");
// ...
} else {
toast.error(
res.errorCode ? getErrorMessage(tErrors, res.errorCode) : t("endpointUpdateFailed")
);
}
| statusModel.color | ||
| )} | ||
| > | ||
| {circuitState} |
There was a problem hiding this comment.
[MEDIUM] [STANDARD-VIOLATION] Circuit state badge renders raw token (open/half-open) instead of i18n
Why this is a problem: CLAUDE.md states: 3. **i18n Required** - All user-facing strings must use i18n (5 languages supported). Never hardcode display text. This tooltip currently shows:
{circuitState}Those raw tokens are not localized.
Suggested fix:
{(circuitState === "open" || circuitState === "half-open") && (
<Badge
variant="outline"
className={cn(
"h-4 px-1 text-[9px] uppercase tracking-wider border-current opacity-80",
statusModel.color
)}
>
{t(circuitState === "open" ? "endpointStatus.circuitOpen" : "endpointStatus.circuitHalfOpen")}
</Badge>
)}
| className="flex items-center gap-1.5 cursor-help opacity-80 hover:opacity-100 transition-opacity focus:outline-none focus:ring-2 focus:ring-ring focus:ring-offset-2 rounded px-1" | ||
| tabIndex={0} | ||
| role="button" | ||
| aria-label={t("endpointStatus.viewDetails", { count })} |
There was a problem hiding this comment.
[MEDIUM] [COMMENT-INACCURATE] i18n placeholder mismatch - viewDetails translation doesn't include {count} parameter
Why this is a problem: Line 66 passes { count } to the translation key, but the translation strings across all languages don't include the {count} placeholder. This may cause runtime errors depending on the i18n library's strictness setting, or silently drop the count from the accessibility label.
Evidence:
- src/app/[locale]/settings/providers/_components/provider-endpoint-hover.tsx:66 -
aria-label={t("endpointStatus.viewDetails", { count })} - messages/en/settings/providers/strings.json:108 -
"viewDetails": "View Details"(no{count}placeholder)
Suggested fix:
"viewDetails": "View Details ({count} endpoints)"Then update all 5 language files (en, ja, ru, zh-CN, zh-TW) with the translated equivalents.
There was a problem hiding this comment.
Code Review Summary
This PR adds a new provider endpoint pool UI (hover status + reusable endpoint CRUD table) and refactors the vendor view to use it. The remaining issues are i18n compliance problems where UI surfaces raw backend error strings / raw circuit-state tokens that are not localized.
PR Size: XL
- Lines changed: 2736
- Files changed: 14
- Split suggestions: Consider splitting into (1) endpoint status model + hover UI + i18n keys, (2) endpoint CRUD table extraction + vendor view integration, (3) unit tests.
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 2 | 1 | 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 | 0 |
Critical Issues (Must Fix)
None.
High Priority Issues (Should Fix)
- [HIGH] [STANDARD-VIOLATION]
src/app/[locale]/settings/providers/_components/provider-endpoints-table.tsx:430surfacesres.errordirectly on endpoint create; useerrorCode+getErrorMessageinstead. (Confidence: 90) - [HIGH] [STANDARD-VIOLATION]
src/app/[locale]/settings/providers/_components/provider-endpoints-table.tsx:588surfacesres.errordirectly on endpoint edit; useerrorCode+getErrorMessageinstead. (Confidence: 90) - [MEDIUM] [STANDARD-VIOLATION]
src/app/[locale]/settings/providers/_components/provider-endpoint-hover.tsx:147renders raw circuit state tokens (open/half-open) in UI; localize via i18n. (Confidence: 80)
Review Coverage
- Logic and correctness
- Security (OWASP Top 10)
- Error handling
- Type safety
- Documentation accuracy
- Test coverage
- Code clarity
Automated review by Codex AI
There was a problem hiding this comment.
Applied PR size label size/XL to PR #719 (feat(providers): expose vendor endpoint pools in settings UI), posted 3 inline review comments, and submitted the review summary via gh pr review.
Issues raised (all on new/changed lines):
src/app/[locale]/settings/providers/_components/provider-endpoints-table.tsx:430[HIGH] [STANDARD-VIOLATION]toast.error(res.error || ...)surfaces raw backendres.error(can be non-localized); useerrorCode+getErrorMessage(...)instead.src/app/[locale]/settings/providers/_components/provider-endpoints-table.tsx:588[HIGH] [STANDARD-VIOLATION] same issue on endpoint edit failure path.src/app/[locale]/settings/providers/_components/provider-endpoint-hover.tsx:147[MEDIUM] [STANDARD-VIOLATION] renders rawcircuitStatetoken (open/half-open) in UI; localize via i18n.
XL split suggestion included in the summary: (1) endpoint status model + hover UI + i18n keys, (2) endpoint CRUD table extraction + vendor view integration, (3) unit tests.
There was a problem hiding this comment.
Code Review Summary
This PR successfully exposes vendor endpoint pools in the Settings UI with comprehensive visual indicators and a reusable CRUD table component. The implementation is well-architected with strong test coverage (1,151 lines of tests across 4 new test files).
PR Size: XL
- Lines changed: 1,682 (2,209 additions + 527 deletions)
- Files changed: 14
Note: Despite the XL size, the PR has a net code reduction of 527 lines due to effective refactoring (extracted provider-endpoints-table from vendor view).
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 | 1 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Medium Priority Issues (Should Fix)
- i18n Placeholder Mismatch (provider-endpoint-hover.tsx:66)
- The
viewDetailstranslation key receives a{count}parameter but the translation strings don't include this placeholder - May cause runtime errors or silently drop the count from the accessibility label
- Action Required: Update all 5 language translation files to include
{count}placeholder
- The
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Comprehensive (all mutations have error handlers with user feedback)
- Type safety - Strong (good use of TypeScript types, no
anyusage) - Documentation accuracy - Good (inline comments match implementation)
- Test coverage - Excellent (80%+ coverage with 1,151 lines of tests)
- Code clarity - Very good (well-structured, clear naming, good separation of concerns)
Strengths
- Excellent Test Coverage: 4 comprehensive test files covering all new components
- Complete i18n Support: All 5 languages properly supported (except the one placeholder issue)
- Good Refactoring: Successfully extracted reusable components, reducing code by 527 lines
- Type Safety: Strong TypeScript usage with no
anytypes - Error Handling: All mutations have proper error handling with toast notifications
- Accessibility: Good use of ARIA labels and semantic HTML
Minor Observations (No Action Required)
-
Query Key Construction: Lines 99-100 in provider-endpoints-table.tsx use
.filter(Boolean)which could theoretically collapse distinct cache keys if falsy suffixes are intentionally used. However, this appears to be intentional design sincequeryKeySuffixis optional. -
Latency Display: Line 127 in provider-endpoint-hover.tsx only displays latency when truthy, which means 0ms latency won't be shown. This is likely intentional UX design.
Overall Assessment
This is a high-quality PR that successfully builds on the vendor-endpoint architecture from #608. The single i18n issue is straightforward to fix. The code is well-tested, properly typed, and follows project conventions. The refactoring demonstrates good engineering practices by reducing duplication while adding features.
Recommendation: Approve after fixing the i18n placeholder mismatch.
Automated review by Claude AI
🧪 测试结果
总体结果: ✅ 所有测试通过 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@src/app/`[locale]/settings/providers/_components/forms/provider-form/sections/basic-info-section.tsx:
- Around line 74-96: The SectionCard and the inner SmartInputWrapper both use
the same label t("websiteUrl.label"), causing duplicate visible labels; remove
the redundant label by deleting the SectionCard's title prop (SectionCard(...
title={t("websiteUrl.label")} ...)) so the input keeps its label via
SmartInputWrapper, or conversely remove the SmartInputWrapper label and keep
SectionCard's title—adjust only one of SectionCard or SmartInputWrapper
surrounding the website URL input to eliminate the duplicate.
🧹 Nitpick comments (3)
tests/unit/settings/providers/provider-form-total-limit-ui.test.tsx (1)
18-23: 建议简化hasOwn辅助函数当前实现使用了不必要的类型转换。可以直接使用
Object.hasOwn(ES2022+) 或Object.prototype.hasOwnProperty.call。♻️ 建议的简化方案
-function hasOwn(obj: object, prop: PropertyKey): boolean { - return (Object as unknown as { hasOwn: (obj: object, prop: PropertyKey) => boolean }).hasOwn( - obj, - prop - ); -} +function hasOwn(obj: object, prop: PropertyKey): boolean { + return Object.prototype.hasOwnProperty.call(obj, prop); +}src/app/[locale]/settings/providers/_components/forms/provider-form/index.tsx (1)
145-149: 建议澄清端点 URL 回退逻辑
endpointPoolPreferredUrl会回退到禁用端点的 URL，但由于endpointPoolHideLegacyUrlInput在无启用端点时为false，这个回退值实际不会被用于提交。逻辑上是安全的，但可以考虑简化以提高可读性。♻️ 可选的简化方案
- const endpointPoolPreferredUrl = - (enabledEndpointPoolEndpoints[0] ?? endpointPoolEndpoints[0])?.url ?? null; + const endpointPoolPreferredUrl = enabledEndpointPoolEndpoints[0]?.url ?? null;由于仅在
endpointPoolHasEnabledEndpoints为true时才隐藏旧版 URL 输入，回退到禁用端点的逻辑是冗余的。tests/unit/settings/providers/provider-form-endpoint-pool.test.tsx (1)
104-112:setNativeValue辅助函数重复此辅助函数与
provider-form-total-limit-ui.test.tsx中的实现完全相同。建议提取到共享的测试工具模块中以避免代码重复。♻️ 建议提取共享工具函数
创建
tests/unit/utils/test-helpers.ts：export function setNativeValue(element: HTMLInputElement, value: string) { const prototype = Object.getPrototypeOf(element) as unknown as { value?: unknown }; const descriptor = Object.getOwnPropertyDescriptor(prototype, "value"); if (descriptor?.set) { descriptor.set.call(element, value); return; } element.value = value; } export async function flushTicks(times = 3) { for (let i = 0; i < times; i++) { await act(async () => { await new Promise((r) => setTimeout(r, 0)); }); } }然后在两个测试文件中导入使用。
| {/* Website URL */} | ||
| {!hideWebsiteUrl && ( | ||
| <SectionCard | ||
| title={t("websiteUrl.label")} | ||
| description={t("websiteUrl.desc")} | ||
| icon={ExternalLink} | ||
| > | ||
| <SmartInputWrapper label={t("websiteUrl.label")}> | ||
| <div className="relative"> | ||
| <Input | ||
| id={isEdit ? "edit-website-url" : "website-url"} | ||
| type="url" | ||
| value={state.basic.websiteUrl} | ||
| onChange={(e) => dispatch({ type: "SET_WEBSITE_URL", payload: e.target.value })} | ||
| placeholder={t("websiteUrl.placeholder")} | ||
| disabled={state.ui.isPending} | ||
| className="pr-10" | ||
| /> | ||
| <ExternalLink className="absolute right-3 top-1/2 -translate-y-1/2 h-4 w-4 text-muted-foreground pointer-events-none" /> | ||
| </div> | ||
| </SmartInputWrapper> | ||
| </SectionCard> | ||
| )} |
There was a problem hiding this comment.
Website URL 区块存在重复的标签
SectionCard 的 title 和内部 SmartInputWrapper 的 label 都设置为 t("websiteUrl.label")，导致标签文本重复显示。
🐛 建议移除重复的标签
<SectionCard
title={t("websiteUrl.label")}
description={t("websiteUrl.desc")}
icon={ExternalLink}
>
- <SmartInputWrapper label={t("websiteUrl.label")}>
+ <SmartInputWrapper label="">
<div className="relative">或者移除 SectionCard 的 title，只保留 SmartInputWrapper 的 label。
📝 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.
| {/* Website URL */} | |
| {!hideWebsiteUrl && ( | |
| <SectionCard | |
| title={t("websiteUrl.label")} | |
| description={t("websiteUrl.desc")} | |
| icon={ExternalLink} | |
| > | |
| <SmartInputWrapper label={t("websiteUrl.label")}> | |
| <div className="relative"> | |
| <Input | |
| id={isEdit ? "edit-website-url" : "website-url"} | |
| type="url" | |
| value={state.basic.websiteUrl} | |
| onChange={(e) => dispatch({ type: "SET_WEBSITE_URL", payload: e.target.value })} | |
| placeholder={t("websiteUrl.placeholder")} | |
| disabled={state.ui.isPending} | |
| className="pr-10" | |
| /> | |
| <ExternalLink className="absolute right-3 top-1/2 -translate-y-1/2 h-4 w-4 text-muted-foreground pointer-events-none" /> | |
| </div> | |
| </SmartInputWrapper> | |
| </SectionCard> | |
| )} | |
| {/* Website URL */} | |
| {!hideWebsiteUrl && ( | |
| <SectionCard | |
| title={t("websiteUrl.label")} | |
| description={t("websiteUrl.desc")} | |
| icon={ExternalLink} | |
| > | |
| <SmartInputWrapper label=""> | |
| <div className="relative"> | |
| <Input | |
| id={isEdit ? "edit-website-url" : "website-url"} | |
| type="url" | |
| value={state.basic.websiteUrl} | |
| onChange={(e) => dispatch({ type: "SET_WEBSITE_URL", payload: e.target.value })} | |
| placeholder={t("websiteUrl.placeholder")} | |
| disabled={state.ui.isPending} | |
| className="pr-10" | |
| /> | |
| <ExternalLink className="absolute right-3 top-1/2 -translate-y-1/2 h-4 w-4 text-muted-foreground pointer-events-none" /> | |
| </div> | |
| </SmartInputWrapper> | |
| </SectionCard> | |
| )} |
🤖 Prompt for AI Agents
In
`@src/app/`[locale]/settings/providers/_components/forms/provider-form/sections/basic-info-section.tsx
around lines 74 - 96, The SectionCard and the inner SmartInputWrapper both use
the same label t("websiteUrl.label"), causing duplicate visible labels; remove
the redundant label by deleting the SectionCard's title prop (SectionCard(...
title={t("websiteUrl.label")} ...)) so the input keeps its label via
SmartInputWrapper, or conversely remove the SmartInputWrapper label and keep
SectionCard's title—adjust only one of SectionCard or SmartInputWrapper
surrounding the website URL input to eliminate the duplicate.
- Add bg-popover/text-popover-foreground to tooltip for correct dark mode colors - Show endpoint URL directly instead of label in hover tooltip - Use structured errorCode with getErrorMessage for add/edit endpoint errors - Add i18n circuit state labels (circuitOpen/circuitHalfOpen) - Add endpoint count to viewDetails i18n string across all 5 languages - Replace .filter(Boolean) with .filter(value => value != null) per linter Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| onSuccess: (data) => { | ||
| queryClient.invalidateQueries({ queryKey: ["provider-endpoints"] }); | ||
| if (data?.result.ok) { |
There was a problem hiding this comment.
Over-broad cache invalidation
invalidateQueries({ queryKey: ["provider-endpoints"] }) will refetch every endpoints query (all vendors/types/suffixes) whenever you probe/delete/toggle/edit a single endpoint. Since ProviderEndpointHover and the form also use provider-endpoints keys, this can cause unnecessary network churn and UI jank as the settings page grows. Prefer invalidating the specific queryKey used by the table (e.g. include vendorId/providerType/queryKeySuffix) or use invalidateQueries({ queryKey, exact: true }) where possible.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/[locale]/settings/providers/_components/provider-endpoints-table.tsx
Line: 205:207
Comment:
**Over-broad cache invalidation**
`invalidateQueries({ queryKey: ["provider-endpoints"] })` will refetch *every* endpoints query (all vendors/types/suffixes) whenever you probe/delete/toggle/edit a single endpoint. Since `ProviderEndpointHover` and the form also use `provider-endpoints` keys, this can cause unnecessary network churn and UI jank as the settings page grows. Prefer invalidating the specific `queryKey` used by the table (e.g. include `vendorId`/`providerType`/`queryKeySuffix`) or use `invalidateQueries({ queryKey, exact: true })` where possible.
How can I resolve this? If you propose a fix, please make it concise.| { | ||
| "$schema": "https://biomejs.dev/schemas/2.3.11/schema.json", | ||
| "$schema": "https://biomejs.dev/schemas/2.3.10/schema.json", | ||
| "vcs": { | ||
| "enabled": true, |
There was a problem hiding this comment.
Schema version regression
This changes Biome’s $schema from 2.3.11 to 2.3.10. If the repo/tooling expects the newer schema (e.g., editors using the schema URL for validation/autocomplete), this is a downgrade that can break config validation or hide config errors. Unless you have a concrete reason to pin to the older schema, revert this change and keep the schema consistent with the rest of the repo/tooling.
Prompt To Fix With AI
This is a comment left during a code review.
Path: biome.json
Line: 1:4
Comment:
**Schema version regression**
This changes Biome’s `$schema` from `2.3.11` to `2.3.10`. If the repo/tooling expects the newer schema (e.g., editors using the schema URL for validation/autocomplete), this is a downgrade that can break config validation or hide config errors. Unless you have a concrete reason to pin to the older schema, revert this change and keep the schema consistent with the rest of the repo/tooling.
How can I resolve this? If you propose a fix, please make it concise.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@src/app/`[locale]/settings/providers/_components/provider-endpoints-table.tsx:
- Around line 291-298: The Switch's aria-label is static and doesn't reflect the
current enabled state; update the Switch (in the JSX around Switch, using
endpoint.isEnabled and t(...)) to provide a dynamic accessible label (or
aria-checked) that changes with endpoint.isEnabled so screen readers know
whether the endpoint is enabled or disabled; ensure you continue to respect
readOnly and isToggling and use the same toggleMutation.mutate handler.
🧹 Nitpick comments (8)
messages/en/settings/providers/strings.json (1)
71-71: 顶层noEndpoints与endpointStatus.noEndpoints键名重复，注意区分上下文。顶层
noEndpoints（Line 71）用于表格空状态，endpointStatus.noEndpoints（Line 110）用于悬浮提示。语义不同但键名相同，后续维护时容易混淆。当前不阻塞，仅提醒注意。Also applies to: 110-110
src/app/[locale]/settings/providers/_components/provider-endpoints-table.tsx (7)
116-128: 变量t遮蔽了外层的翻译函数t。Line 118 中
typeOrder.map((t, i) => [t, i])的回调参数t遮蔽了 Line 95 的useTranslations返回值t。虽然在map回调内没有引用翻译函数，不会造成运行时错误，但会降低可读性，建议重命名为type或pt。建议修改
const endpoints = useMemo(() => { const typeOrder = getAllProviderTypes(); - const typeIndexMap = new Map(typeOrder.map((t, i) => [t, i])); + const typeIndexMap = new Map(typeOrder.map((type, i) => [type, i]));
197-220:isProbing/isToggling手动状态管理与 mutation 内置isPending冗余。
useMutation已经提供probeMutation.isPending和toggleMutation.isPending，无需额外用useState+onMutate/onSettled手动追踪。简化后可减少状态同步风险。示例：移除手动状态，改用 mutation.isPending
- const [isProbing, setIsProbing] = useState(false); - const [isToggling, setIsToggling] = useState(false); const probeMutation = useMutation({ mutationFn: async () => { const res = await probeProviderEndpoint({ endpointId: endpoint.id }); if (!res.ok) throw new Error(res.error); return res.data; }, - onMutate: () => setIsProbing(true), - onSettled: () => setIsProbing(false), onSuccess: (data) => { /* ... */ }, onError: () => { /* ... */ }, }); // In JSX, replace isProbing with probeMutation.isPending // and isToggling with toggleMutation.isPendingAlso applies to: 238-256
389-391: 可选类型的硬编码排除列表不够灵活。Line 390 硬编码了
["claude-auth", "gemini-cli"]作为排除项。如果未来新增不可选类型，需要手动更新此列表。建议在PROVIDER_TYPE_CONFIG中增加如selectable: boolean属性，由配置驱动过滤。
497-506: URL 输入框混用了受控与非受控模式。
urlInput 没有value属性（非受控），但通过onChange同步到 state 以供UrlPreview使用。而labelInput（Line 514）却是完全受控的（有value={label}）。同一表单内两种模式混用易导致混淆。建议统一为受控模式：建议修改
<Input id="url" name="url" placeholder={t("endpointUrlPlaceholder")} required + value={url} onChange={(e) => setUrl(e.target.value)} />如果改为受控，
handleSubmit中也可以直接使用urlstate 而非formData.get("url")，使数据流更清晰。
424-432: 重复的缓存失效调用。Line 425 的
invalidateQueries({ queryKey: ["provider-endpoints", vendorId] })已经会匹配所有以["provider-endpoints", vendorId]开头的查询键（包括带providerType和queryKeySuffix的），因此 Lines 426-431 的第二次失效调用是冗余的。建议简化
if (res.ok) { toast.success(t("endpointAddSuccess")); setOpen(false); - // Invalidate both specific and general queries queryClient.invalidateQueries({ queryKey: ["provider-endpoints", vendorId] }); - if (fixedProviderType) { - queryClient.invalidateQueries({ - queryKey: ["provider-endpoints", vendorId, fixedProviderType, queryKeySuffix].filter( - (value) => value != null - ), - }); - } } else {
560-666: 编辑对话框缺少UrlPreview，与新增对话框不一致。
AddEndpointButton在 Line 539 包含了<UrlPreview baseUrl={url} providerType={providerType} />，但EditEndpointDialog没有。用户编辑 URL 时同样可能受益于实时预览，确认代理路径是否正确。如果是有意省略请忽略。
339-346: 删除操作使用浏览器原生confirm()而非自定义确认对话框。Line 342 使用
confirm(...)弹出浏览器原生确认框，与项目其他地方（如deleteVendorConfirmTitle/deleteVendorDoubleConfirmTitle等 i18n 键暗示的自定义确认对话框）风格不一致，且无法被主题样式覆盖。建议后续统一为自定义AlertDialog组件。
| {!readOnly && ( | ||
| <Switch | ||
| checked={endpoint.isEnabled} | ||
| onCheckedChange={(checked) => toggleMutation.mutate(checked)} | ||
| disabled={isToggling} | ||
| aria-label={t("enabledStatus")} | ||
| /> | ||
| )} |
There was a problem hiding this comment.
Switch 的 aria-label 是静态的，无法反映当前启用/禁用状态。
Line 296 的 aria-label={t("enabledStatus")} 始终为 "enabled"，不随开关状态变化。屏幕阅读器用户无法区分当前状态。建议根据 endpoint.isEnabled 动态切换。
建议修改
<Switch
checked={endpoint.isEnabled}
onCheckedChange={(checked) => toggleMutation.mutate(checked)}
disabled={isToggling}
- aria-label={t("enabledStatus")}
+ aria-label={endpoint.isEnabled ? t("enabledStatus") : t("disabledStatus")}
/>📝 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.
| {!readOnly && ( | |
| <Switch | |
| checked={endpoint.isEnabled} | |
| onCheckedChange={(checked) => toggleMutation.mutate(checked)} | |
| disabled={isToggling} | |
| aria-label={t("enabledStatus")} | |
| /> | |
| )} | |
| {!readOnly && ( | |
| <Switch | |
| checked={endpoint.isEnabled} | |
| onCheckedChange={(checked) => toggleMutation.mutate(checked)} | |
| disabled={isToggling} | |
| aria-label={endpoint.isEnabled ? t("enabledStatus") : t("disabledStatus")} | |
| /> | |
| )} |
🤖 Prompt for AI Agents
In `@src/app/`[locale]/settings/providers/_components/provider-endpoints-table.tsx
around lines 291 - 298, The Switch's aria-label is static and doesn't reflect
the current enabled state; update the Switch (in the JSX around Switch, using
endpoint.isEnabled and t(...)) to provide a dynamic accessible label (or
aria-checked) that changes with endpoint.isEnabled so screen readers know
whether the endpoint is enabled or disabled; ensure you continue to respect
readOnly and isToggling and use the same toggleMutation.mutate handler.
github-project-automation
bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
* fix(proxy): add 'cannot be modified' error detection to thinking signature rectifier Extend the thinking signature rectifier to detect and handle the Anthropic API error when thinking/redacted_thinking blocks have been modified from their original response. This error occurs when clients inadvertently modify these blocks in multi-turn conversations. The rectifier will now remove these blocks and retry the request, similar to how it handles other thinking-related signature errors. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * chore(deps): bump jspdf in the npm_and_yarn group across 1 directory Bumps the npm_and_yarn group with 1 update in the / directory: [jspdf](https://github.com/parallax/jsPDF). Updates `jspdf` from 3.0.4 to 4.1.0 - [Release notes](https://github.com/parallax/jsPDF/releases) - [Changelog](https://github.com/parallax/jsPDF/blob/master/RELEASE.md) - [Commits](parallax/jsPDF@v3.0.4...v4.1.0) --- updated-dependencies: - dependency-name: jspdf dependency-version: 4.1.0 dependency-type: direct:production dependency-group: npm_and_yarn ... Signed-off-by: dependabot[bot] <support@github.com> * fix: Hot-reload cache invalidation for Request Filters and Sensitive Words (#710) * fix: hot-reload request filters via globalThis singleton pattern EventEmitter and RequestFilterEngine now use globalThis caching to ensure the same instance is shared across different Next.js worker contexts. This fixes the issue where filter changes required Docker restart. Added diagnostic logging for event subscription and propagation. * fix(redis): wait for subscriber connection ready before subscribe - ensureSubscriber now returns Promise<Redis>, waits for 'ready' event - subscribeCacheInvalidation returns null on failure instead of noop - RequestFilterEngine checks cleanup !== null before logging success - Fixes false "Subscribed" log when Redis connection fails * feat(sensitive-words): add hot-reload via Redis pub/sub Enable real-time cache invalidation for sensitive words detector, matching the pattern used by request-filter-engine and error-rule-detector. * fix(redis): harden cache invalidation subscriptions Ensure sensitive-words CRUD emits update events so hot-reload propagates across workers. Roll back failed pub/sub subscriptions, add retry/timeout coverage, and avoid sticky provider-cache subscription state. * fix(codex): bump default User-Agent fallback Update the hardcoded Codex UA used when requests lack an effective user-agent (e.g. filtered out). Keep unit tests in sync with the new default. * fix(redis): resubscribe cache invalidation after reconnect Clear cached subscription state on disconnect and resubscribe on ready so cross-worker cache invalidation survives transient Redis reconnects. Add unit coverage, avoid misleading publish logs, track detector cleanup handlers, and translate leftover Russian comments to English. * fix(sensitive-words): use globalThis singleton detector Align SensitiveWordDetector with existing __CCH_* singleton pattern to avoid duplicate instances across module reloads. Extend singleton unit tests to cover the detector. * chore: format code (req-fix-dda97fd) * fix: address PR review comments - pubsub.ts: use `once` instead of `on` for ready event to prevent duplicate resubscription handlers on reconnect - forwarder.ts: extract DEFAULT_CODEX_USER_AGENT constant - provider-cache.ts: wrap subscribeCacheInvalidation in try/catch - tests: use exported constant instead of hardcoded UA string * fix(redis): resubscribe across repeated reconnects Ensure pub/sub resubscribe runs on every reconnect, extend unit coverage, and keep emitRequestFiltersUpdated resilient when logger import fails. --------- Co-authored-by: John Doe <johndoe@example.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * feat(logs): make cost column toggleable with improved type safety (#715) close #713 * fix(proxy): add OpenAI chat completion format support in usage extraction (#705) (#716) The `extractUsageMetrics` function was missing support for OpenAI chat completion format fields (`prompt_tokens`/`completion_tokens`), causing token statistics to not be recorded for OpenAI-compatible providers. Changes: - Add `prompt_tokens` -> `input_tokens` mapping - Add `completion_tokens` -> `output_tokens` mapping - Preserve priority: Claude > Gemini > OpenAI format - Add 5 unit tests for OpenAI format handling Closes #705 Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com> * fix(currency): respect system currencyDisplay setting in UI (#717) Fixes #678 - Currency display unit configuration was not applied. Root cause: - `users-page-client.tsx` hardcoded `currencyCode="USD"` - `UserLimitBadge` and `LimitStatusIndicator` had hardcoded `unit="$"` default - `big-screen/page.tsx` used hardcoded "$" in multiple places Changes: - Add `getCurrencySymbol()` helper function to currency.ts - Fetch system settings in `users-page-client.tsx` and pass to table - Pass `currencySymbol` from `user-key-table-row.tsx` to limit badges - Remove hardcoded "$" defaults from badge components - Update big-screen page to fetch settings and use dynamic symbol - Add unit tests for `getCurrencySymbol` Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com> * feat(gemini): add Google Search web access preference for Gemini providers (#721) * feat(gemini): add Google Search web access preference for Gemini providers Add provider-level preference for Gemini API type providers to control Google Search (web access) tool injection: - inherit: Follow client request (default) - enabled: Force inject googleSearch tool into request - disabled: Force remove googleSearch tool from request Changes: - Add geminiGoogleSearchPreference field to provider schema - Add GeminiGoogleSearchPreference type and validation - Implement applyGeminiGoogleSearchOverride with audit trail - Add UI controls in provider form (Gemini Overrides section) - Add i18n translations for 5 languages (en, zh-CN, zh-TW, ja, ru) - Integrate override logic in proxy forwarder for Gemini requests - Add 22 unit tests for the override logic Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(gemini): address code review feedback - Use explicit else-if for disabled preference check (gemini-code-assist) - Use i18n for SelectValue placeholder instead of hardcoded string (coderabbitai) - Sync overridden body back to session.request.message for log consistency (coderabbitai) - Persist Gemini special settings immediately, matching Anthropic pattern (coderabbitai) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(gemini): use strict types for provider config and audit - Narrow preference type to "enabled" | "disabled" (exclude unreachable "inherit") - Use ProviderType and GeminiGoogleSearchPreference types instead of string Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com> * fix(api): 透传 /api/actions 认证会话以避免 getUsers 返回空数据 (#720) * fix(api): 透传 /api/actions 认证会话以避免 getUsers 返回空数据 * fix(auth): 让 scoped session 继承 allowReadOnlyAccess 语义并支持内部降权校验 * chore: format code (dev-93585fa) * fix: bound SessionTracker active_sessions zsets by env TTL (#718) * fix(session): bound active_sessions zsets by env ttl Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai> * fix(rate-limit): pass session ttl to lua cleanup Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai> * fix(session): validate SESSION_TTL env and prevent ZSET leak on invalid values - Add input validation for SESSION_TTL (reject NaN, 0, negative; default 300) - Guard against invalid TTL in Lua script to prevent clearing all sessions - Use dynamic EXPIRE based on SESSION_TTL instead of hardcoded 3600s - Add unit tests for TTL validation and dynamic expiry behavior --------- Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai> * fix(provider): stop standard-path fallback to legacy provider url * feat(providers): expose vendor endpoint pools in settings UI (#719) * feat(providers): add endpoint status mapping * feat(providers): add endpoint pool hover * feat(providers): show vendor endpoints in list rows * feat(providers): extract vendor endpoint CRUD table * chore(i18n): add provider endpoint UI strings * fix(providers): integrate endpoint pool into provider form * fix(provider): wrap endpoint sync in transactions to prevent race conditions (#730) * fix(provider): wrap provider create/update endpoint sync in transactions Provider create and update operations now run vendor resolution and endpoint sync inside database transactions to prevent race conditions that could leave orphaned or inconsistent endpoint rows. Key changes: - createProvider: wrap vendor + insert + endpoint seed in a single tx - updateProvider: wrap vendor + update + endpoint sync in a single tx - Add syncProviderEndpointOnProviderEdit for atomic URL/type/vendor migration with in-place update, soft-delete, and conflict handling - Vendor cleanup failures degrade to warnings instead of propagating - Add comprehensive unit and integration tests for sync edge cases Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(provider): defer endpoint circuit reset until transaction commit Avoid running endpoint circuit reset side effects inside DB transactions to prevent rollback inconsistency. Run resets only after commit and add regression tests for deferred reset behavior in helper and provider update flows. * fix(provider): distinguish noop from created-next in endpoint sync action label When ensureNextEndpointActive() returns "noop" (concurrent transaction already created an active next endpoint), the action was incorrectly labelled "kept-previous-and-created-next". Add a new "kept-previous-and-kept-next" action to ProviderEndpointSyncAction and use a three-way branch so callers and logs reflect the true outcome. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> * fix: address review comments from PR #731 - fix(auth): prevent scoped session access widening via ?? -> && guard - fix(i18n): standardize zh-CN provider terminology to "服务商" - fix(i18n): use consistent Russian translations for circuit status - fix(i18n): replace raw formatDistanceToNow with locale-aware RelativeTime - fix(gemini): log warning for unknown google search preference values - fix(error-rules): check subscribeCacheInvalidation return value - fix(test): correct endpoint hover sort test to assert URLs not labels Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: export auth session storage and fix test mock types - Export authSessionStorage from auth-session-storage.node.ts to prevent undefined on named imports; remove duplicate declare global block - Fix mockEndpoints in provider-endpoint-hover test: remove nonexistent lastOk/lastLatencyMs fields, add missing lastProbe* fields, use Date objects for createdAt/updatedAt Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: miraserver <20286838+miraserver@users.noreply.github.com> Co-authored-by: John Doe <johndoe@example.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: 泠音 <im@ling.plus> Co-authored-by: Longlone <41830147+WAY29@users.noreply.github.com> Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
1 participant
Summary
Expose vendor endpoint pools in the Providers settings UI with visual status indicators, hover tooltips, and a reusable CRUD table component. This PR builds on the vendor-endpoint architecture introduced in #608 by making endpoint management accessible to users through the settings interface.
Problem
After #608 introduced the vendor-endpoint architecture with health probing and circuit breakers, the UI lacked visibility into:
Related Issues:
Solution
Add comprehensive UI components to expose endpoint pool information:
endpoint-status.ts) - Consistent probe/circuit state renderingprovider-endpoint-hover.tsx) - Shows endpoint count + per-endpoint status in provider list rowsprovider-rich-list-item.tsx) - Display vendor name + endpoint summary when vendor is assignedprovider-endpoints-table.tsx) - Extracted from vendor view for reusabilityChanges
Core Changes
src/app/[locale]/settings/providers/_components/endpoint-status.ts(+107) - Status mapping helpersrc/app/[locale]/settings/providers/_components/provider-endpoint-hover.tsx(+154) - Endpoint pool hover tooltipsrc/app/[locale]/settings/providers/_components/provider-endpoints-table.tsx(+704) - Reusable endpoint CRUD tablesrc/app/[locale]/settings/providers/_components/provider-rich-list-item.tsx(+24/-3) - Show vendor + endpoint summarySupporting Changes
src/app/[locale]/settings/providers/_components/provider-vendor-view.tsx(+9/-519) - Refactored to use extracted table componentmessages/*/settings/providers/strings.json(+60) - i18n strings for endpoint status UITesting
tests/unit/settings/providers/endpoint-status.test.ts(+101) - Status mapping teststests/unit/settings/providers/provider-endpoint-hover.test.tsx(+244) - Hover component teststests/unit/settings/providers/provider-endpoints-table.test.tsx(+567) - CRUD table teststests/unit/settings/providers/provider-rich-list-item-endpoints.test.tsx(+239) - List item integration testsTesting
Automated Tests
bun run test)bun run typecheck)bun run lint)bun run build)Manual Testing
Notes
Checklist
Description enhanced by Claude AI
Greptile Overview
Greptile Summary
Confidence Score: 3/5
Important Files Changed
Sequence Diagram