Skip to content

feat(providers): add sub-navigation, Options tab, and proxy indicator#876

Merged
ding113 merged 20 commits intoding113:devfrom
miraserver:feat/providers-list-3
Mar 8, 2026
Merged

feat(providers): add sub-navigation, Options tab, and proxy indicator#876
ding113 merged 20 commits intoding113:devfrom
miraserver:feat/providers-list-3

Conversation

@miraserver
Copy link
Contributor

@miraserver miraserver commented Mar 7, 2026

Summary

Adds hierarchical sub-navigation to the provider form sidebar, extracts Options into a dedicated tab, and introduces a proxy status indicator across the provider list.

What changed

  • Sub-navigation system: Tabs now support nested sub-items (Routing > Scheduling, Options > Active Time, Limits > Circuit Breaker, Network > Timeout) with scroll-synced highlighting across all breakpoints (desktop sidebar, tablet horizontal, mobile bottom nav)
  • Options tab: Extracted from the 960-line RoutingSection into a standalone OptionsSection (572 lines) — covers advanced settings, Codex/Anthropic/Gemini overrides, and Active Time scheduling
  • Proxy indicator: Green ShieldCheck icon on provider list items when proxyUrl is configured, with tooltip; works for both vendor-resolved and legacy URL providers
  • Batch edit dialog: Options tab now available in batch edit mode alongside existing tabs
  • Tab status indicators: options tab shows "configured" dot when any override deviates from defaults
  • i18n: All new labels translated across 5 languages (en, zh-CN, zh-TW, ja, ru)
  • Tests: 24 unit tests for FormTabNav covering rendering, interaction, status indicators, sub-tabs, excludeTabs, and responsive layouts

Files changed (27 files, +1671 / -1068)

Area Key files
Navigation components/form-tab-nav.tsx (sub-tab config, NAV_CONFIG, PARENT_MAP, responsive layouts)
Form core index.tsx (scroll sync, SET_ACTIVE_NAV, section refs, tab status)
State provider-form-context.tsx, provider-form-types.ts (new action/types)
Sections options-section.tsx (new), routing-section.tsx (reduced), limits-section.tsx, network-section.tsx (sub-section refs)
List provider-rich-list-item.tsx (ShieldCheck proxy badge), provider-list.tsx (layout tweaks)
Batch provider-batch-dialog.tsx (Options tab integration)
i18n 15 message files across 5 locales
Tests form-tab-nav.test.tsx (24 tests)

Review notes

  • 4 rounds of automated codex review conducted (Quality, Security, Performance, Architecture)
  • Security audit: Grade A — zero vulnerabilities
  • All pre-existing performance patterns (context rerenders, lazy init) left unchanged — not in scope
  • allowedProviderTypes discard (_allowedProviderTypes) is pre-existing in dev, not introduced here

Test plan

  • bun run build — production build passes
  • bun run typecheck — zero type errors
  • bun run test — 3478 tests passed, 3 pre-existing Redis/DB failures unrelated to this PR
  • Biome lint clean on all changed files
  • Manual: verify sub-nav scroll sync on desktop/tablet/mobile breakpoints
  • Manual: verify proxy ShieldCheck renders for providers with and without vendor resolution
  • Manual: verify Options tab in batch edit dialog

Greptile Summary

This PR adds hierarchical sub-navigation to the provider form, extracts a new OptionsSection from the 960-line RoutingSection, and introduces a proxy status indicator (ShieldCheck) on provider list items. The changes are well-scoped with coherent architecture — NAV_CONFIG/PARENT_MAP/TAB_ORDER provide a single source of truth for the navigation hierarchy, scroll-sync is correctly throttled via requestAnimationFrame, and the scrollend-based unlock pattern is sound.

Key finding:

  • Inconsistent i18n key for Anthropic Overrides: The Anthropic parameter section uses maxTokens.label ("Max Tokens Override") as the section title, while Codex and Gemini sections both use dedicated title keys. A dedicated anthropicOverrides.title key should be added to match the established pattern.

The proxy badge correctly handles both vendor-resolved and legacy URL providers. Test coverage (24 nav tests + 534-line options section tests) is thorough for the new code.

Confidence Score: 4/5

  • Safe to merge; the architecture is sound, tests are comprehensive, and the only remaining concern is a minor i18n consistency issue in the Anthropic Overrides section title.
  • The PR implements hierarchical sub-navigation, Options section extraction, and proxy status indicators with well-structured code and thorough test coverage (24 nav tests + 534-line section tests). The scroll-sync logic is correctly implemented using requestAnimationFrame throttling and proper cleanup. The only remaining finding is a style inconsistency where the Anthropic Overrides section uses a misaligned i18n key pattern compared to Codex and Gemini—this is straightforward to fix but non-critical to functionality.
  • src/app/[locale]/settings/providers/_components/forms/provider-form/sections/options-section.tsx (Anthropic Overrides i18n key pattern) — though the fix is optional as it does not affect functionality.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[User clicks nav item] --> B{Parent tab or sub-tab?}
    B -- "Parent tab (handleTabChange)" --> C["dispatch SET_ACTIVE_TAB\n(resets activeSubTab to null)"]
    B -- "Sub-tab (handleSubTabChange)" --> D["Look up parentTab\nvia PARENT_MAP"]
    D --> E["dispatch SET_ACTIVE_NAV\n{tab: parentTab, subTab}"]
    C --> F[scrollToSection parent]
    E --> F2[scrollToSection sub-tab]

    F --> G[contentRef.scrollTo smooth]
    F2 --> G

    G --> H[addEventListener scrollend once]
    H --> I{scrollend fires?}
    I -- Yes --> J["isScrollingToSection = false\nclear 1000ms timer"]
    I -- No after 1000ms --> K["fallback: unlock\nremove scrollend listener"]

    L[User scrolls manually] --> M[handleScroll via rAF throttle]
    M --> N{isScrollingToSection?}
    N -- false --> O[Scan NAV_ORDER sections\nfind closest to viewport top]
    O --> P{Hit sub-tab?}
    P -- Yes --> Q["dispatch SET_ACTIVE_NAV\n{tab: parent, subTab}"]
    P -- No --> R["dispatch SET_ACTIVE_NAV\n{tab: section, subTab: null}"]
    N -- true --> S[Ignore scroll event]
Loading

Comments Outside Diff (17)

  1. src/app/[locale]/settings/providers/_components/forms/provider-form/provider-form-types.ts, line 918-919 (link)

    Unused SET_ACTIVE_SUB_TAB action type

    SET_ACTIVE_SUB_TAB is defined in the union type and handled in the reducer (provider-form-context.tsx), but it is never dispatched anywhere in this PR. All callers use SET_ACTIVE_NAV instead. This dead code path adds unnecessary surface area to the state management API and could confuse future contributors.

    Consider removing the action type and its reducer case, or alternatively documenting why it is kept for future use.

    The same SET_ACTIVE_SUB_TAB case in provider-form-context.tsx (line ~878) should also be removed.

  2. src/app/[locale]/settings/providers/_components/provider-rich-list-item.tsx, line 3093 (link)

    Redundant delayDuration prop on Tooltip

    The parent TooltipProvider in provider-list.tsx already sets delayDuration={200}. Specifying the same value on the individual Tooltip is redundant, since child Tooltip components inherit the provider's delay when their own delayDuration is not set.

  3. src/app/[locale]/settings/providers/_components/forms/provider-form/sections/options-section.tsx, line 1437-1442 (link)

    subSectionRefs.options property is never provided by callers

    OptionsSectionProps declares subSectionRefs?.options as a ref callback, but index.tsx only passes activeTime when mounting OptionsSection. The scroll-sync for the options parent tab is handled by the outer wrapper div ref in index.tsx, so this interface entry serves no purpose. The dead property in the interface makes the props contract misleading.

    Consider removing it to keep the API surface minimal:

  4. src/app/[locale]/settings/providers/_components/forms/provider-form/sections/options-section.tsx, line 30-54 (link)

    The options property is defined in OptionsSectionProps (line 32) and bound to a <div ref={subSectionRefs?.options}> (line 54), but when OptionsSection is called from index.tsx (lines 661–667), the activeTime key is passed but not the options key. Additionally, the outer wrapper in index.tsx already registers sectionRefs.current.options = el directly (line 658), making the inner ref redundant.

    This dead code adds no functional value and may confuse future maintainers. Remove the unused options property from the interface and its corresponding ref binding:

  5. src/app/[locale]/settings/providers/_components/forms/provider-form/sections/options-section.tsx, line 1440-1465 (link)

    Dead options prop in subSectionRefs

    The options key in the OptionsSectionProps.subSectionRefs interface is declared and wired to a <div ref={subSectionRefs?.options}>, but it is never passed by any caller. In index.tsx, the parent <div ref={(el) => { sectionRefs.current.options = el; }}> already registers the options section in the scroll-sync map — there is no code path that provides subSectionRefs.options.

    This leaves the inner <div ref={subSectionRefs?.options}> always receiving undefined as its ref, making the wrapper div and the interface property dead code. This could mislead future maintainers into thinking the options sub-section has its own dedicated tracking slot in sectionRefs.

    Consider removing the options key from the subSectionRefs interface and the associated wrapper <div>, or — if it was intended for future use — leaving a clear comment explaining why it is present.

    And in the JSX, remove the wrapping <div ref={subSectionRefs?.options}> and keep the space-y-6 class directly on the <motion.div> child.

  6. src/app/[locale]/settings/providers/_components/forms/provider-form/components/form-tab-nav.tsx, line 516-549 (link)

    Tablet and mobile sub-nav use unfiltered NAV_BY_ID

    The tablet sub-nav IIFE (and the identical mobile one a few lines below) reads children via:

    const activeItem = NAV_BY_ID[activeTab];

    NAV_BY_ID is built directly from the raw NAV_CONFIG and is not filtered by excludeTabs. The desktop sidebar correctly iterates filteredNav, which respects excluded tabs. While the current callers (batch dialog uses its own tab system, not FormTabNav's sub-nav) don't expose a broken path today, this inconsistency could surface sub-items for a parent tab that has been excluded once excludeTabs is used more broadly.

    For defensiveness and consistency, derive activeItem from filteredNav instead:

    const activeItem = filteredNav.find((item) => item.id === activeTab);

    This applies identically to the mobile IIFE below (line ~556).

  7. src/app/[locale]/settings/providers/_components/forms/provider-form/index.tsx, line 208-230 (link)

    Stale scrollend listener can prematurely unlock scroll guard on rapid tab clicks

    When scrollToSection is called twice in quick succession (e.g., rapid tab clicks), the scrollend listener registered by the first call is never removed before the second call begins. Both listeners remain simultaneously active on the scroll container.

    If the browser fires scrollend for the canceled/interrupted first scroll (per CSSWG spec, scrollend is dispatched when a scroll completes or is interrupted), onScrollEnd1 runs and:

    1. Calls clearTimeout(scrollLockTimerRef.current!) — which now points to timer2 (the second call's fallback), canceling it.
    2. Calls unlock() — setting isScrollingToSection.current = false while the second smooth scroll is still animating.

    With isScrollingToSection.current = false during the ongoing programmatic scroll, the rAF-throttled handleScroll will begin updating the active tab/subtab from the intermediate scroll position, causing nav-highlight jitter until the second animation finishes.

    The fix is to keep a mutable ref to the current onScrollEnd callback so the previous listener can be explicitly removed before registering a new one:

    const scrollEndListenerRef = useRef<(() => void) | null>(null);
    
    const scrollToSection = useCallback((tab: NavTargetId) => {
      const section = sectionRefs.current[tab];
      if (section && contentRef.current) {
        // Remove any existing scrollend listener before starting a new scroll
        if (scrollEndListenerRef.current) {
          contentRef.current.removeEventListener("scrollend", scrollEndListenerRef.current);
          scrollEndListenerRef.current = null;
        }
        if (scrollLockTimerRef.current) clearTimeout(scrollLockTimerRef.current);
    
        isScrollingToSection.current = true;
        const containerTop = contentRef.current.getBoundingClientRect().top;
        const sectionTop = section.getBoundingClientRect().top;
        const offset = sectionTop - containerTop + contentRef.current.scrollTop;
        contentRef.current.scrollTo({ top: offset, behavior: "smooth" });
    
        const unlock = () => { isScrollingToSection.current = false; };
        const onScrollEnd = () => {
          clearTimeout(scrollLockTimerRef.current!);
          scrollEndListenerRef.current = null;
          unlock();
        };
        scrollEndListenerRef.current = onScrollEnd;
        contentRef.current.addEventListener("scrollend", onScrollEnd, { once: true });
        scrollLockTimerRef.current = setTimeout(() => {
          contentRef.current?.removeEventListener("scrollend", onScrollEnd);
          scrollEndListenerRef.current = null;
          unlock();
        }, 1000);
      }
    }, []);

    Also update the useEffect cleanup to remove the pending listener on unmount:

    return () => {
      if (rafRef.current !== null) cancelAnimationFrame(rafRef.current);
      if (scrollLockTimerRef.current) clearTimeout(scrollLockTimerRef.current);
      if (scrollEndListenerRef.current && contentRef.current) {
        contentRef.current.removeEventListener("scrollend", scrollEndListenerRef.current);
      }
    };
  8. src/app/[locale]/settings/providers/_components/forms/provider-form/sections/options-section.tsx, line 30-35 (link)

    Dead options property in subSectionRefs

    The options callback is declared in the interface and wired to the wrapper <div> at line 54 (ref={subSectionRefs?.options}), but index.tsx never passes options when calling <OptionsSection>. The scroll anchor for the "options" tab is already set by the outer <div> wrapper in index.tsx:

    // index.tsx
    <div ref={(el) => { sectionRefs.current.options = el; }}>
      <OptionsSection subSectionRefs={{ activeTime: ... }} />  // ← no `options` key
    </div>

    Because subSectionRefs?.options is always undefined at runtime, the ref on line 54 is never populated. The options property in the interface (and its corresponding ref={} usage) is dead code and can be removed to avoid confusion.

  9. src/app/[locale]/settings/providers/_components/forms/provider-form/index.tsx, line 228-240 (link)

    scrollend listener cleanup races with the fallback timer

    When scrollend fires, onScrollEnd calls clearTimeout(scrollLockTimerRef.current!) with a non-null assertion. While this is safe in practice (the timer is set synchronously on the line immediately after the listener is registered), the null assertion could hide a future regression if the cleanup order ever changes. Consider guarding it:

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

  10. src/app/[locale]/settings/providers/_components/forms/provider-form/components/form-tab-nav.tsx, line 324-326 (link)

    Tablet sub-nav uses NAV_BY_ID which ignores excludeTabs

    The tablet and mobile sub-nav rows use NAV_BY_ID[activeTab] to find the active item's children, but NAV_BY_ID is built from the full NAV_CONFIG — it is unaffected by excludeTabs. This means that if a parent tab is excluded via excludeTabs but somehow still becomes activeTab, its children would still appear in the tablet/mobile sub-row. While no current caller triggers this (the batch dialog doesn't use excludeTabs), the inconsistency with filteredNav/filteredTabs is worth noting. Consider deriving sub-items from filteredNav instead:

    const activeNavItem = filteredNav.find((item) => item.id === activeTab);

    Then use activeNavItem?.children in both the tablet and mobile sub-rows.

  11. src/app/[locale]/settings/providers/_components/provider-rich-list-item.tsx, line 771 (link)

    delayDuration={200} is already configured on the parent TooltipProvider in provider-list.tsx (line 85), so the per-instance override here is redundant. Remove it to keep tooltip delay controlled centrally.

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

  12. src/app/[locale]/settings/providers/_components/forms/provider-form/sections/options-section.tsx, line 337-339 (link)

    Anthropic SectionCard uses wrong i18n keys for title and description

    The SectionCard for Anthropic overrides is reusing the field-level maxTokens.label ("Max Tokens Override") as the card's section heading, and maxTokens.help ("Override max_tokens in request body. Range: 1–64000...") as the card description. This is misleading because the section contains three distinct controls — max tokens, thinking budget, and adaptive thinking — not just max tokens.

    Compare the parallel Codex and Gemini sections which correctly use dedicated section-level keys (sections.routing.codexOverrides.title, sections.routing.geminiOverrides.title). The anthropicOverrides namespace in the i18n files has no title / desc keys, so both the component and the message files need updating.

    You will also need to add the missing keys to every locale's sections.json, e.g. in messages/en/settings/providers/form/sections.json:

    "anthropicOverrides": {
      "title": "Anthropic Parameter Overrides",
      "desc": "Override Anthropic API request parameters at the provider level",
      ...
    }
  13. src/app/[locale]/settings/providers/_components/forms/provider-form/sections/options-section.tsx, line 38 (link)

    useTranslations for batch namespace called unconditionally

    tBatch is only consumed inside {isBatch && ...} guards, but useTranslations("settings.providers.batchEdit") runs on every render regardless of mode. This causes next-intl to resolve the batch message namespace even for regular edit/create modes. Consider moving this call inside a component that is only rendered in batch mode, or guard it with a conditional to avoid loading the namespace in non-batch contexts.

    Note: React rules prevent calling hooks conditionally, so the cleanest fix is to split OptionsSection into a thin wrapper that passes the translated batch strings as props, or accept a batchLabels prop from the parent (which already knows the mode).

  14. tests/unit/settings/providers/form-tab-nav.test.tsx, line 112-118 (link)

    Sub-tab selected by hardcoded positional index

    desktopButtons[2] assumes scheduling is the third button in the desktop nav. If a new tab or sub-tab is ever inserted before routing in NAV_CONFIG, this test will silently assert the wrong button. Prefer a text-based or role-based query:

  15. src/app/[locale]/settings/providers/_components/forms/provider-form/components/form-tab-nav.tsx, line 56-62 (link)

    PARENT_MAP type assertion could hide future mismatches

    Object.fromEntries(...) as Record<SubTabId, TabId> casts away the Record<string, unknown> return type of Object.fromEntries. If a new sub-tab ID is added to NAV_CONFIG but not added to the SubTabId union type, the cast silently succeeds while lookups at that key would return undefined at runtime. Using a typed helper or satisfies would surface the discrepancy at compile time:

    Or, for stricter safety, verify exhaustiveness with satisfies:

    export const PARENT_MAP = Object.fromEntries(
      NAV_CONFIG.flatMap((item) => (item.children ?? []).map((child) => [child.id, item.id]))
    ) satisfies Record<SubTabId, TabId>;
  16. src/components/ui/chart.tsx, line 170-172 (link)

    Inconsistent indentation in .map() callback body

    The refactor removed the explicit type annotation but left the callback body indented 4 extra spaces relative to the arrow, which is inconsistent with the rest of the file and will likely be flagged by Biome. The const key = ... line and the return below it should be at one indentation level inside the arrow function body rather than 4 extra spaces deeper.

  17. src/app/[locale]/settings/providers/_components/forms/provider-form/sections/options-section.tsx, line 337-339 (link)

    The Anthropic Overrides section card uses t("sections.routing.anthropicOverrides.maxTokens.label") as its title, which resolves to "Max Tokens Override". However, this card actually covers three distinct features:

    • Max Tokens Override
    • Thinking Budget Override
    • Adaptive Thinking

    The Codex and Gemini sections both use dedicated title keys (codexOverrides.title, geminiOverrides.title), but no equivalent anthropicOverrides.title key exists in the i18n files. This results in a misleading section title that only names one feature.

    Consider adding a dedicated title key in all i18n files (e.g., "anthropicOverrides.title": "Anthropic Parameter Overrides") and use it here:

    This would also require adding corresponding anthropicOverrides.desc entries to match the pattern used for Codex and Gemini sections.

Last reviewed commit: f7af6bc

John Doe and others added 7 commits March 7, 2026 12:55
…sidebar

Add Scheduling, Circuit Breaker, Timeout sub-items under their parent tabs
in the desktop sidebar for quick scroll access. Promote Options to a
top-level tab. Includes scroll tracking, i18n (5 langs), and 13 tests.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add activeTime sub-navigation for quick scroll access to Scheduled Active
Time section. Also add variant="highlight" to Options SectionCard for
consistent visual styling with other main sections.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Separate icon with tooltip next to endpoint count on provider list.
Visible only when proxyUrl is set. i18n for 5 languages.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ture

- Derive TAB_CONFIG, TAB_ORDER, NAV_ORDER, PARENT_MAP from NAV_CONFIG (DRY)
- Add sub-nav to tablet and mobile breakpoints
- Move activeSubTab from useState into form reducer
- Compute Options tab status from routing state instead of hardcoding
- Lift TooltipProvider from per-item to list container level
- Fix RU i18n: singular form for timeout label
- Add 8 new tests covering derived constants and responsive sub-nav

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… fix

- Extract OptionsSection from RoutingSection (963 -> 432 lines)
- Throttle scroll handler via requestAnimationFrame
- Merge double dispatch into single SET_ACTIVE_NAV action
- Derive sectionRefs from NAV_ORDER instead of manual record
- Add NAV_BY_ID lookup map for O(1) tablet/mobile nav access
- Add excludeTabs prop to FormTabNav, hide Options in batch dialog
- Clean up setTimeout/rAF refs on unmount

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Move ShieldCheck proxy indicator out of the vendor-specific branch
so it renders for all providers with a configured proxyUrl.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link

coderabbitai bot commented Mar 7, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

新增“选项”主标签与子标签(scheduling、activeTime、circuitBreaker、timeout),引入 OptionsSection 组件、分层导航常量(NAV_CONFIG/NAV_ORDER/PARENT_MAP/TAB_ORDER)、子节 refs 与类型、表单上下文的 activeSubTab 支持,及 provider 列表的代理指示与若干 i18n 文案与 tagInput 文本项。

Changes

Cohort / File(s) Summary
国际化 — tabs 翻译
messages/en/.../form/common.json, messages/ja/.../form/common.json, messages/ru/.../form/common.json, messages/zh-CN/.../form/common.json, messages/zh-TW/.../form/common.json
新增 tabs 翻译键:scheduling, options, activeTime, circuitBreaker, timeout
国际化 — sections 翻译
messages/en/.../form/sections.json, messages/ja/.../form/sections.json, messages/ru/.../form/sections.json, messages/zh-CN/.../form/sections.json, messages/zh-TW/.../form/sections.json
在 preserveClientIp / routing.preserveClientIp 中新增 options 对象(含 titledesc)。
国际化 — 列表 & UI 文案
messages/*/settings/providers/list.json, messages/*/ui.json
新增 proxyEnabled 本地化文案;在 tagInput 中新增 unknownError 文案(多语言)。
导航与类型
src/app/[locale]/settings/providers/.../components/form-tab-nav.tsx, .../provider-form-types.ts
引入分层 NAV_CONFIG,新增 SubTabIdNavTargetIdoptions TabId,并导出 TAB_ORDERNAV_ORDERPARENT_MAP;FormTabNav 支持子标签渲染、排除过滤与交互。
表单容器与上下文
src/app/[locale]/settings/providers/.../provider-form/index.tsx, .../provider-form-context.tsx
将导航/滚动改为基于 NavTargetId,增加 activeSubTab 状态与 SET_ACTIVE_NAV action,添加 handleSubTabChange 并将 sub-tab refs 接入表单。
新增 OptionsSection
src/app/[locale]/settings/providers/.../sections/options-section.tsx
新增大型 OptionsSection 组件(高级设置、覆盖、活跃时间等多个子节),并在表单与批量编辑中集成。
各 Section 子节 refs 与签名变化
.../sections/routing-section.tsx, .../sections/limits-section.tsx, .../sections/network-section.tsx
这些 Section 接受可选 subSectionRefs prop,并在相应子节(scheduling、circuitBreaker、timeout 等)处包裹 ref 容器;Limits/Network 调整布局与部分控件。
批量编辑与列表 UI 调整
src/app/[locale]/settings/providers/_components/batch-edit/provider-batch-dialog.tsx, .../provider-list.tsx, .../provider-rich-list-item.tsx
在批量编辑对话框中集成 OptionsSection;为 provider 列表项添加代理(proxy)指示 UI 与 Tooltip 上下文。
测试
tests/unit/settings/providers/form-tab-nav.test.tsx, tests/unit/settings/providers/options-section.test.tsx
扩展/新增单元测试:覆盖新导出的 NAV_ORDERPARENT_MAPTAB_ORDER,并为 OptionsSection 添加全面行为测试。

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 15.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed PR标题清晰准确地总结了本次变更的核心内容:添加子导航、Options标签页和代理指示器,对应了文件摘要中的主要改动。
Description check ✅ Passed PR描述与变更集直接相关,清晰地说明了子导航系统、Options选项卡、代理指示器等核心功能的实现。

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello, 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 enhances the user interface for provider configuration by introducing a hierarchical sub-navigation system, making complex settings more organized and accessible. It also extracts advanced provider options into a dedicated tab and adds a visual indicator for proxy usage in the provider list, improving clarity and usability for managing various provider settings.

Highlights

  • Sub-navigation System: Introduced hierarchical sub-navigation to the provider form sidebar, allowing for nested items like 'Routing > Scheduling', 'Options > Active Time', 'Limits > Circuit Breaker', and 'Network > Timeout', with scroll-synced highlighting across all responsive breakpoints.
  • Dedicated Options Tab: Extracted advanced provider settings, including Codex/Anthropic/Gemini overrides and Active Time scheduling, from the 'RoutingSection' into a new, dedicated 'OptionsSection' for better organization and clarity.
  • Proxy Status Indicator: Added a visual 'ShieldCheck' icon to provider list items, which appears green with a tooltip when a 'proxyUrl' is configured, enhancing visibility of proxy usage.
  • Batch Edit Dialog Integration: The newly created 'Options' tab is now available within the batch edit dialog, allowing for bulk configuration of these advanced settings.
  • Tab Status Indicators: The 'Options' tab now displays a 'configured' dot indicator when any of its override settings deviate from their default values.
  • Internationalization (i18n): All new labels and descriptions introduced by these features have been translated across five languages: English, Simplified Chinese, Traditional Chinese, Japanese, and Russian.
  • Comprehensive Unit Tests: Added 24 new unit tests for the 'FormTabNav' component, covering rendering, interaction, status indicators, sub-tabs, exclusion logic, and responsive layouts.
Changelog
  • messages/en/settings/providers/form/common.json
    • Added new translation keys for sub-navigation items like "scheduling", "options", "activeTime", "circuitBreaker", and "timeout".
  • messages/en/settings/providers/form/sections.json
    • Added new translation keys for the "Options" section title and description.
  • messages/en/settings/providers/list.json
    • Added a new translation key for "Proxy enabled" for the proxy indicator.
  • messages/ja/settings/providers/form/common.json
    • Added Japanese translation keys for new sub-navigation items.
  • messages/ja/settings/providers/form/sections.json
    • Added Japanese translation keys for the "Options" section.
  • messages/ja/settings/providers/list.json
    • Added Japanese translation key for "Proxy enabled".
  • messages/ru/settings/providers/form/common.json
    • Added Russian translation keys for new sub-navigation items.
  • messages/ru/settings/providers/form/sections.json
    • Added Russian translation keys for the "Options" section.
  • messages/ru/settings/providers/list.json
    • Added Russian translation key for "Proxy enabled".
  • messages/zh-CN/settings/providers/form/common.json
    • Added Simplified Chinese translation keys for new sub-navigation items.
  • messages/zh-CN/settings/providers/form/sections.json
    • Added Simplified Chinese translation keys for the "Options" section.
  • messages/zh-CN/settings/providers/list.json
    • Added Simplified Chinese translation key for "Proxy enabled".
  • messages/zh-TW/settings/providers/form/common.json
    • Added Traditional Chinese translation keys for new sub-navigation items.
  • messages/zh-TW/settings/providers/form/sections.json
    • Added Traditional Chinese translation keys for the "Options" section.
  • messages/zh-TW/settings/providers/list.json
    • Added Traditional Chinese translation key for "Proxy enabled".
  • src/app/[locale]/settings/providers/_components/batch-edit/provider-batch-dialog.tsx
    • Imported the new OptionsSection component and integrated it into the batch edit dialog, allowing it to be displayed when the "options" tab is active.
  • src/app/[locale]/settings/providers/_components/forms/provider-form/components/form-tab-nav.tsx
    • Refactored the tab navigation component to support hierarchical sub-navigation, introduced new icon imports, defined NAV_CONFIG, TAB_ORDER, NAV_ORDER, and PARENT_MAP constants, and updated rendering logic to display sub-tabs and handle their active states across different layouts (desktop, tablet, mobile).
  • src/app/[locale]/settings/providers/_components/forms/provider-form/index.tsx
    • Updated imports to include new navigation constants (NAV_ORDER, PARENT_MAP, TAB_ORDER) and the OptionsSection component. Modified the sectionRefs to track sub-tab elements, adjusted scrollToSection and handleScroll to correctly identify and navigate to active tabs/sub-tabs, and integrated the OptionsSection into the form's main content area.
  • src/app/[locale]/settings/providers/_components/forms/provider-form/provider-form-context.tsx
    • Updated the initial UI state to include activeSubTab and added new reducer actions (SET_ACTIVE_SUB_TAB, SET_ACTIVE_NAV) to manage the active sub-tab and combined navigation state.
  • src/app/[locale]/settings/providers/_components/forms/provider-form/provider-form-types.ts
    • Extended type definitions to include TabId with "options", introduced new types SubTabId for specific sub-sections ("scheduling", "activeTime", "circuitBreaker", "timeout"), and NavTargetId to represent any navigation target (tab or sub-tab).
  • src/app/[locale]/settings/providers/_components/forms/provider-form/sections/limits-section.tsx
    • Modified the LimitsSection component to accept subSectionRefs as a prop and wrapped the "Circuit Breaker Settings" in a div to allow it to be referenced for scroll-to-section functionality.
  • src/app/[locale]/settings/providers/_components/forms/provider-form/sections/network-section.tsx
    • Modified the NetworkSection component to accept subSectionRefs as a prop and wrapped the "Timeout Configuration" in a div to allow it to be referenced for scroll-to-section functionality.
  • src/app/[locale]/settings/providers/_components/forms/provider-form/sections/options-section.tsx
    • Added a new OptionsSection component, which encapsulates advanced settings, Codex/Anthropic/Gemini overrides, and active time scheduling, previously part of the RoutingSection.
  • src/app/[locale]/settings/providers/_components/forms/provider-form/sections/routing-section.tsx
    • Refactored the RoutingSection component by removing the advanced settings, Codex/Anthropic/Gemini overrides, and active time scheduling logic, which were moved to the new OptionsSection. It now accepts subSectionRefs and wraps the "Scheduling Parameters" in a div for scroll-to-section functionality.
  • src/app/[locale]/settings/providers/_components/provider-list.tsx
    • Wrapped the ProviderList component with TooltipProvider to enable tooltips for its children, specifically for the new proxy indicator.
  • src/app/[locale]/settings/providers/_components/provider-rich-list-item.tsx
    • Added the ShieldCheck icon import and implemented logic to display a green ShieldCheck icon with a "Proxy enabled" tooltip next to the provider URL if proxyUrl is configured.
  • tests/unit/settings/providers/form-tab-nav.test.tsx
    • Updated the unit tests for FormTabNav to reflect the new sub-navigation structure, including new icon stubs, adjusted button counts for different layouts, and added tests for sub-tab rendering, interaction, and active state highlighting.
Activity
  • Four rounds of automated codex review were conducted (Quality, Security, Performance, Architecture).
  • A Security audit resulted in an "Grade A" with zero vulnerabilities.
  • Existing performance patterns were intentionally left unchanged as they were not within the scope of this PR.
  • The allowedProviderTypes discard (_allowedProviderTypes) is a pre-existing feature and not introduced in this PR.
  • The following tests passed: bun run build, bun run typecheck, and bun run test (3478 tests passed, with 3 pre-existing Redis/DB failures unrelated to this PR).
  • Biome lint was clean on all changed files.
  • Manual verification is pending for sub-nav scroll sync, proxy ShieldCheck rendering, and Options tab in batch edit dialog.
Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This is an excellent pull request that significantly improves the provider form's user experience. The introduction of sub-navigation with scroll-syncing is a great enhancement, and the refactoring of the 'Options' section into its own tab makes the form much more organized. The addition of the proxy indicator and comprehensive unit tests are also valuable contributions. I have a few minor suggestions to improve maintainability, but overall, this is a very solid piece of work.

@github-actions github-actions bot added the size/M Medium PR (< 500 lines) label Mar 7, 2026
…ing comments

Address Gemini Code Assist review findings:
- Remove unused SET_ACTIVE_SUB_TAB action type and reducer case (superseded by SET_ACTIVE_NAV)
- Add grouping comments to options tab status conditional for readability

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🧹 Nitpick comments (6)
src/app/[locale]/settings/providers/_components/provider-rich-list-item.tsx (1)

771-782: 代理指示器仅在桌面视图中显示。

代理指示器的实现正确,使用了 i18n 翻译和合适的图标样式。但当前实现仅在桌面视图(md:block 区域,第 695 行开始)中渲染,移动端视图(第 502-552 行的徽章区域)没有对应的代理指示器。

如果这是有意为之以简化移动端 UI,可以忽略此建议。否则,建议在移动端徽章区域也添加代理指示器以保持一致性。

♻️ 可选:在移动端视图添加代理指示器

在第 551 行(Schedule badge 之后)添加:

{/* Proxy indicator */}
{provider.proxyUrl && (
  <Tooltip delayDuration={200}>
    <TooltipTrigger asChild>
      <Badge variant="outline" className="flex items-center gap-1">
        <ShieldCheck className="h-3 w-3 text-emerald-500" />
        {tList("proxyEnabled")}
      </Badge>
    </TooltipTrigger>
  </Tooltip>
)}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/app/`[locale]/settings/providers/_components/provider-rich-list-item.tsx
around lines 771 - 782, The mobile badge area is missing the proxy indicator
present in the desktop section; add the same conditional rendering used around
provider.proxyUrl (the Tooltip + TooltipTrigger + TooltipContent that uses
tList("proxyEnabled") and the ShieldCheck icon) into the mobile badge block
where badges like Schedule are rendered (the compact/mobile badge area that uses
Badge/variant="outline"), ensuring the icon size and layout match mobile styles
(e.g., use smaller icon classes and a Badge wrapper) so the proxy indicator
appears on mobile just as on desktop.
src/app/[locale]/settings/providers/_components/batch-edit/provider-batch-dialog.tsx (1)

46-46: 新加的 OptionsSection import 请改用 @/ 别名。

这行是新引入的 import,按仓库规则应改用 @/ 别名;否则会继续扩大同一文件里相对路径和别名混用的范围。

建议修改
-import { OptionsSection } from "../forms/provider-form/sections/options-section";
+import { OptionsSection } from "@/app/[locale]/settings/providers/_components/forms/provider-form/sections/options-section";
As per coding guidelines, Use path alias `@/` to reference files in `./src/` directory.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/app/`[locale]/settings/providers/_components/batch-edit/provider-batch-dialog.tsx
at line 46, The new import for OptionsSection should use the project path alias
instead of a relative path; update the import statement that brings in
OptionsSection so it references the module via the "@/..." alias (e.g., replace
the current relative import for OptionsSection with the corresponding "@/..."
path), ensuring the symbol OptionsSection is still imported and used unchanged
in provider-batch-dialog.tsx.
src/app/[locale]/settings/providers/_components/forms/provider-form/provider-form-types.ts (1)

23-29: 使用判别联合类型编码合法的父子导航关系。

虽然当前 reducer 逻辑通过 SET_ACTIVE_TAB 清空 activeSubTab 以及 SET_ACTIVE_NAV 仅通过 PARENT_MAP 查询来防止无效状态,但类型层面仍然允许不可能的组合(如 { tab: "basic", subTab: "timeout" })。此外,SET_ACTIVE_SUB_TAB 虽然定义但从未被调用,容易成为未来的隐患。建议将合法的父子组合编码进类型,以消除这些结构性漏洞。

建议的改进方式
+export type NavState =
+  | { tab: "basic"; subTab: null }
+  | { tab: "routing"; subTab: "scheduling" | null }
+  | { tab: "options"; subTab: "activeTime" | null }
+  | { tab: "limits"; subTab: "circuitBreaker" | null }
+  | { tab: "network"; subTab: "timeout" | null }
+  | { tab: "testing"; subTab: null };
+
 export interface UIState {
   activeTab: TabId;
   activeSubTab: SubTabId | null;
   ...
 }

- | { type: "SET_ACTIVE_TAB"; payload: TabId }
- | { type: "SET_ACTIVE_SUB_TAB"; payload: SubTabId | null }
- | { type: "SET_ACTIVE_NAV"; payload: { tab: TabId; subTab: SubTabId | null } }
+ | { type: "SET_ACTIVE_NAV"; payload: NavState }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/app/`[locale]/settings/providers/_components/forms/provider-form/provider-form-types.ts
around lines 23 - 29, The current TabId/SubTabId/NavTargetId types allow
impossible parent-child combos (e.g. tab "basic" with subTab "timeout"); update
the type model to encode valid parent→child relationships as a discriminated
union (e.g. a NavTarget union where each case names a specific TabId and its
allowed SubTabId set) so invalid combinations are rejected at compile time;
adjust code that uses NavTargetId (and references PARENT_MAP) to consume the new
NavTarget shape and either remove or wire up SET_ACTIVE_SUB_TAB consistently (or
restrict reducers to only accept the new discriminated NavTarget), referencing
the existing TabId, SubTabId, NavTargetId, PARENT_MAP, SET_ACTIVE_TAB,
SET_ACTIVE_NAV and SET_ACTIVE_SUB_TAB symbols to locate where changes are
needed.
src/app/[locale]/settings/providers/_components/forms/provider-form/sections/options-section.tsx (1)

437-449: 建议合并多次 dispatch 调用

onConfigChange 回调中连续调用了三次 dispatch,这可能导致三次状态更新和重渲染。考虑到已有 SET_ACTIVE_NAV 这样的组合 action 模式,建议为 adaptive thinking 配置添加类似的批量更新 action。

可选:添加批量更新 action

provider-form-types.ts 中添加:

| { type: "SET_ADAPTIVE_THINKING_CONFIG"; payload: AdaptiveThinkingConfig }

然后在 reducer 中处理为单次状态更新。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/app/`[locale]/settings/providers/_components/forms/provider-form/sections/options-section.tsx
around lines 437 - 449, The handler onConfigChange currently calls dispatch
three times (SET_ADAPTIVE_THINKING_EFFORT,
SET_ADAPTIVE_THINKING_MODEL_MATCH_MODE, SET_ADAPTIVE_THINKING_MODELS) causing
multiple updates; add a single batched action (e.g.,
SET_ADAPTIVE_THINKING_CONFIG) that carries the whole AdaptiveThinkingConfig
payload, update provider-form-types to include the new action variant, and
handle it in the reducer to update effort, modelMatchMode, and models in one
state update, then replace the three dispatch calls in onConfigChange with a
single dispatch of the new action.
src/app/[locale]/settings/providers/_components/forms/provider-form/index.tsx (1)

36-42: 新增导入请统一改成 @/ 别名。

这里引入了多处相对路径导入,后续重构目录时更容易断,也不符合当前仓库约定。 As per coding guidelines "Use path alias @/ to reference files in ./src/ directory"

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/app/`[locale]/settings/providers/_components/forms/provider-form/index.tsx
around lines 36 - 42, The imports in provider-form index.tsx use relative paths
but should use the project alias; update all imports (e.g. FormTabNav,
NAV_ORDER, PARENT_MAP, TAB_ORDER from "./components/form-tab-nav",
ProviderFormProvider and useProviderForm from "./provider-form-context",
NavTargetId/SubTabId/TabId from "./provider-form-types", and the section
components BasicInfoSection, LimitsSection, NetworkSection, OptionsSection) to
use the "@/..." alias pointing into src so they follow the repository convention
and won't break on refactors.
src/app/[locale]/settings/providers/_components/forms/provider-form/components/form-tab-nav.tsx (1)

18-18: 请把这里的类型导入改成 @/ 别名。

新增相对路径会让目录调整时更脆弱,也和仓库规范不一致。 As per coding guidelines "Use path alias @/ to reference files in ./src/ directory"

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/app/`[locale]/settings/providers/_components/forms/provider-form/components/form-tab-nav.tsx
at line 18, Replace the relative import of the types with the project path-alias
import: change the import of NavTargetId, SubTabId, TabId from
"../provider-form-types" to use the "@/..." alias that points into src (i.e.
import { NavTargetId, SubTabId, TabId } from "@/.../provider-form-types");
update the import statement in form-tab-nav.tsx so it references
provider-form-types via the `@/` alias instead of a relative path to comply with
repo guidelines.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@messages/zh-CN/settings/providers/form/sections.json`:
- Around line 43-46: The translation for "options.desc" is inconsistent with
other entries: it uses “提供者 / 覆盖设置” while the rest of the file uses “供应商 / 覆写”;
update the string value for "options.desc" (and ensure "options.title" if
needed) to use the consistent terminology “供应商 / 覆写” so the Options section
matches other form labels and copy (look for the JSON keys "options.title" and
"options.desc" in this file and replace the terms accordingly).

In `@messages/zh-TW/settings/providers/form/sections.json`:
- Around line 347-350: Update the localized strings under the "options" entry so
they use the project's existing terminology: change "提供者" to "供應商" and "覆蓋設定" to
"覆寫" in the "title" and "desc" values; specifically edit the JSON object with
key "options" (its "title" and "desc" properties) to reflect "選項" / "附加供應商選項和覆寫"
(or equivalent phrasing that matches other strings in the same file).

In
`@src/app/`[locale]/settings/providers/_components/forms/provider-form/components/form-tab-nav.tsx:
- Around line 330-339: 当前用于子导航的 button(使用 onSubTabChange、isChildActive、disabled
和 cn 组合 class)移除了默认 outline 但没有替代的可视焦点态,导致键盘无障碍回归;在 form-tab-nav.tsx 中把这两个
button 的 className 补上可见的 focus-visible 样式(例如 focus-visible:ring-2
focus-visible:ring-primary 或类似的 ring + ring-offset 组合,同时保留
focus-visible:outline-none)以在键盘聚焦时展示清晰的焦点环;确保对文件中另一处相同按钮(也使用
onSubTabChange/isChildActive/disabled,位于评论提到的第二处)做同样修改。

In
`@src/app/`[locale]/settings/providers/_components/forms/provider-form/index.tsx:
- Around line 545-562: The status-options logic in the provider form only treats
activeTimeStart as a configured field but misses the case where only
activeTimeEnd is set; update the condition in the block that assigns
status.options = "configured" to also check state.routing.activeTimeEnd !== null
(or replace the single activeTimeStart check with a check that either
state.routing.activeTimeStart !== null || state.routing.activeTimeEnd !== null)
so the status shows "configured" when the end-only time is provided; look for
the condition block referencing state.routing.* and status.options in the
provider-form (around the if (...) { status.options = "configured"; }) and add
the activeTimeEnd check.

In
`@src/app/`[locale]/settings/providers/_components/forms/provider-form/sections/routing-section.tsx:
- Around line 155-164: The onInvalidTag handler currently shows the raw
validation key to users via messages[reason] || reason; replace that fallback
with a translated generic error string (e.g. call tUI with a new key like
"unknownTagError" or "genericTagError") so all user-facing text is i18n'd;
update the same pattern in the other occurrence (around lines 289-298) and
ensure toast.error always receives a localized string (referencing onInvalidTag,
the messages object, toast.error, and GROUP_TAG_MAX_TOTAL_LENGTH to locate the
logic).

---

Nitpick comments:
In
`@src/app/`[locale]/settings/providers/_components/batch-edit/provider-batch-dialog.tsx:
- Line 46: The new import for OptionsSection should use the project path alias
instead of a relative path; update the import statement that brings in
OptionsSection so it references the module via the "@/..." alias (e.g., replace
the current relative import for OptionsSection with the corresponding "@/..."
path), ensuring the symbol OptionsSection is still imported and used unchanged
in provider-batch-dialog.tsx.

In
`@src/app/`[locale]/settings/providers/_components/forms/provider-form/components/form-tab-nav.tsx:
- Line 18: Replace the relative import of the types with the project path-alias
import: change the import of NavTargetId, SubTabId, TabId from
"../provider-form-types" to use the "@/..." alias that points into src (i.e.
import { NavTargetId, SubTabId, TabId } from "@/.../provider-form-types");
update the import statement in form-tab-nav.tsx so it references
provider-form-types via the `@/` alias instead of a relative path to comply with
repo guidelines.

In
`@src/app/`[locale]/settings/providers/_components/forms/provider-form/index.tsx:
- Around line 36-42: The imports in provider-form index.tsx use relative paths
but should use the project alias; update all imports (e.g. FormTabNav,
NAV_ORDER, PARENT_MAP, TAB_ORDER from "./components/form-tab-nav",
ProviderFormProvider and useProviderForm from "./provider-form-context",
NavTargetId/SubTabId/TabId from "./provider-form-types", and the section
components BasicInfoSection, LimitsSection, NetworkSection, OptionsSection) to
use the "@/..." alias pointing into src so they follow the repository convention
and won't break on refactors.

In
`@src/app/`[locale]/settings/providers/_components/forms/provider-form/provider-form-types.ts:
- Around line 23-29: The current TabId/SubTabId/NavTargetId types allow
impossible parent-child combos (e.g. tab "basic" with subTab "timeout"); update
the type model to encode valid parent→child relationships as a discriminated
union (e.g. a NavTarget union where each case names a specific TabId and its
allowed SubTabId set) so invalid combinations are rejected at compile time;
adjust code that uses NavTargetId (and references PARENT_MAP) to consume the new
NavTarget shape and either remove or wire up SET_ACTIVE_SUB_TAB consistently (or
restrict reducers to only accept the new discriminated NavTarget), referencing
the existing TabId, SubTabId, NavTargetId, PARENT_MAP, SET_ACTIVE_TAB,
SET_ACTIVE_NAV and SET_ACTIVE_SUB_TAB symbols to locate where changes are
needed.

In
`@src/app/`[locale]/settings/providers/_components/forms/provider-form/sections/options-section.tsx:
- Around line 437-449: The handler onConfigChange currently calls dispatch three
times (SET_ADAPTIVE_THINKING_EFFORT, SET_ADAPTIVE_THINKING_MODEL_MATCH_MODE,
SET_ADAPTIVE_THINKING_MODELS) causing multiple updates; add a single batched
action (e.g., SET_ADAPTIVE_THINKING_CONFIG) that carries the whole
AdaptiveThinkingConfig payload, update provider-form-types to include the new
action variant, and handle it in the reducer to update effort, modelMatchMode,
and models in one state update, then replace the three dispatch calls in
onConfigChange with a single dispatch of the new action.

In `@src/app/`[locale]/settings/providers/_components/provider-rich-list-item.tsx:
- Around line 771-782: The mobile badge area is missing the proxy indicator
present in the desktop section; add the same conditional rendering used around
provider.proxyUrl (the Tooltip + TooltipTrigger + TooltipContent that uses
tList("proxyEnabled") and the ShieldCheck icon) into the mobile badge block
where badges like Schedule are rendered (the compact/mobile badge area that uses
Badge/variant="outline"), ensuring the icon size and layout match mobile styles
(e.g., use smaller icon classes and a Badge wrapper) so the proxy indicator
appears on mobile just as on desktop.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a8c417ea-68c9-455a-852c-4b1ea5a1afa8

📥 Commits

Reviewing files that changed from the base of the PR and between 2b60af9 and d336953.

📒 Files selected for processing (27)
  • messages/en/settings/providers/form/common.json
  • messages/en/settings/providers/form/sections.json
  • messages/en/settings/providers/list.json
  • messages/ja/settings/providers/form/common.json
  • messages/ja/settings/providers/form/sections.json
  • messages/ja/settings/providers/list.json
  • messages/ru/settings/providers/form/common.json
  • messages/ru/settings/providers/form/sections.json
  • messages/ru/settings/providers/list.json
  • messages/zh-CN/settings/providers/form/common.json
  • messages/zh-CN/settings/providers/form/sections.json
  • messages/zh-CN/settings/providers/list.json
  • messages/zh-TW/settings/providers/form/common.json
  • messages/zh-TW/settings/providers/form/sections.json
  • messages/zh-TW/settings/providers/list.json
  • src/app/[locale]/settings/providers/_components/batch-edit/provider-batch-dialog.tsx
  • src/app/[locale]/settings/providers/_components/forms/provider-form/components/form-tab-nav.tsx
  • src/app/[locale]/settings/providers/_components/forms/provider-form/index.tsx
  • src/app/[locale]/settings/providers/_components/forms/provider-form/provider-form-context.tsx
  • src/app/[locale]/settings/providers/_components/forms/provider-form/provider-form-types.ts
  • src/app/[locale]/settings/providers/_components/forms/provider-form/sections/limits-section.tsx
  • src/app/[locale]/settings/providers/_components/forms/provider-form/sections/network-section.tsx
  • src/app/[locale]/settings/providers/_components/forms/provider-form/sections/options-section.tsx
  • src/app/[locale]/settings/providers/_components/forms/provider-form/sections/routing-section.tsx
  • src/app/[locale]/settings/providers/_components/provider-list.tsx
  • src/app/[locale]/settings/providers/_components/provider-rich-list-item.tsx
  • tests/unit/settings/providers/form-tab-nav.test.tsx

@miraserver
Copy link
Contributor Author

CI Failures Analysis

All CI failures are pre-existing and not introduced by this PR:

Check Failing files In our diff?
TypeScript (tsgo) latency-chart.tsx, chart.tsx — Recharts type compat No (0 lines changed)
Unit: i18n guard Missing prices.badges.multi in zh-CN No (prices namespace)
Unit: cost-calc (x2) Tiered pricing math in cost-calculation-breakdown.test.ts No
Unit: lease-decrement (x5) response-handler-lease-decrement.test.ts No

Verified: files identical between dev and this branch. Claude CI Auto-Fix on main also failed with the same errors.


Also addressed Gemini Code Assist findings in latest push:

  • Removed dead SET_ACTIVE_SUB_TAB action type and reducer case
  • Added grouping comments to options status conditional

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

This PR adds hierarchical sub-navigation to the provider form sidebar, extracts Options into a dedicated tab, and introduces a proxy status indicator. The implementation is generally well-structured with proper i18n coverage across 5 languages.

PR Size: M

  • Lines changed: 1671 additions / 1068 deletions
  • Files changed: 27

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 1 0 0 0
Simplification 0 0 0 0

Critical Issue (Must Fix)

[TEST-MISSING-CRITICAL] OptionsSection component (572 lines, new file) has zero test coverage.

Why this is a problem: Per CLAUDE.md guidelines, all new features must have unit test coverage of at least 80%. The OptionsSection is a significant new component that handles complex form state for provider overrides, but has no corresponding tests.

Suggested fix: Add unit tests in tests/unit/settings/providers/options-section.test.tsx covering:

  • Rendering of all provider type-specific override fields
  • State management through the form context
  • Conditional rendering based on provider type (claude/codex/gemini)

Review Coverage

  • Logic and correctness - Clean
  • Security (OWASP Top 10) - Clean
  • Error handling - Clean
  • Type safety - Clean
  • Documentation accuracy - Clean
  • Test coverage - Needs improvement
  • Code clarity - Good

Positive Notes

  • Excellent i18n coverage across 5 languages
  • Clean hierarchical navigation implementation
  • Proper TypeScript type safety maintained
  • Good separation of concerns in the new OptionsSection

Automated review by Claude AI

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
src/app/[locale]/settings/providers/_components/forms/provider-form/index.tsx (1)

545-567: ⚠️ Potential issue | 🟡 Minor

Options 状态点仍然漏掉了“只设置结束时间”的场景。

这里还是只检查了 activeTimeStart !== null。如果用户只填了 activeTimeEndstatus.options 不会显示为 configured,和新加的 Active Time 子区块不一致。应覆盖开始/结束任一一侧被设置的情况。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/app/`[locale]/settings/providers/_components/forms/provider-form/index.tsx
around lines 545 - 567, The options configured check currently sets
status.options = "configured" only when state.routing.activeTimeStart !== null,
missing cases where only state.routing.activeTimeEnd is set; update the
conditional in the provider form logic that evaluates advanced/options (the
block inspecting state.routing.* and setting status.options) to treat active
time as configured if either state.routing.activeTimeStart !== null OR
state.routing.activeTimeEnd !== null so that setting just the end time flips
status.options to "configured".
🧹 Nitpick comments (1)
src/app/[locale]/settings/providers/_components/forms/provider-form/index.tsx (1)

36-42: 把新增的本地导入改成 @/ 别名。

这几处新改动仍然在用相对路径引用 src/ 内文件,和仓库约定不一致。建议在这一轮改动里一起切到 @/,避免继续扩散。

As per coding guidelines "Use path alias @/ to reference files in ./src/ directory".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/app/`[locale]/settings/providers/_components/forms/provider-form/index.tsx
around lines 36 - 42, Replace the relative src imports with the repository alias
by updating the import paths for FormTabNav (and NAV_ORDER, PARENT_MAP,
TAB_ORDER), ProviderFormProvider and useProviderForm, the type imports
NavTargetId/SubTabId/TabId, and the section components BasicInfoSection,
LimitsSection, NetworkSection, OptionsSection to use "@/..." alias instead of
relative "./..." paths; ensure the aliased paths map to the same modules and
update any surrounding imports in this module to match the project's path-alias
convention.
🤖 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]/settings/providers/_components/forms/provider-form/index.tsx:
- Around line 208-219: 不要用固定 500ms 解锁滚动同步;在 scrollToSection 中保留设置
isScrollingToSection/current scrollLockTimerRef 的意图,但改为在实际到达目标或滚动真正停止后再释放锁:计算目标
offset(你已有的 offset 计算)后触发平滑滚动,然后启动一个短期的“等待滚动结束”检测(可用
contentRef.current.scrollTop 与 offset 的接近度检测循环或使用 IntersectionObserver 监听
sectionRefs.current[tab] 在 contentRef 中变为可见),当检测到到达目标或连续若干帧 scrollTop 不再变化时才把
isScrollingToSection.current = false 并清理 scrollLockTimerRef;更新相关清理逻辑以防内存泄漏,并确保
handleScroll 依赖 isScrollingToSection 来忽略程序触发的滚动事件。

---

Duplicate comments:
In
`@src/app/`[locale]/settings/providers/_components/forms/provider-form/index.tsx:
- Around line 545-567: The options configured check currently sets
status.options = "configured" only when state.routing.activeTimeStart !== null,
missing cases where only state.routing.activeTimeEnd is set; update the
conditional in the provider form logic that evaluates advanced/options (the
block inspecting state.routing.* and setting status.options) to treat active
time as configured if either state.routing.activeTimeStart !== null OR
state.routing.activeTimeEnd !== null so that setting just the end time flips
status.options to "configured".

---

Nitpick comments:
In
`@src/app/`[locale]/settings/providers/_components/forms/provider-form/index.tsx:
- Around line 36-42: Replace the relative src imports with the repository alias
by updating the import paths for FormTabNav (and NAV_ORDER, PARENT_MAP,
TAB_ORDER), ProviderFormProvider and useProviderForm, the type imports
NavTargetId/SubTabId/TabId, and the section components BasicInfoSection,
LimitsSection, NetworkSection, OptionsSection to use "@/..." alias instead of
relative "./..." paths; ensure the aliased paths map to the same modules and
update any surrounding imports in this module to match the project's path-alias
convention.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b06dedcb-6322-406a-9fea-05caedb72589

📥 Commits

Reviewing files that changed from the base of the PR and between d336953 and 54c1602.

📒 Files selected for processing (3)
  • src/app/[locale]/settings/providers/_components/forms/provider-form/index.tsx
  • src/app/[locale]/settings/providers/_components/forms/provider-form/provider-form-context.tsx
  • src/app/[locale]/settings/providers/_components/forms/provider-form/provider-form-types.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/app/[locale]/settings/providers/_components/forms/provider-form/provider-form-types.ts

- Fix zh-CN/zh-TW i18n terminology consistency (供应商/供應商 instead of 提供者)
- Add activeTimeEnd check to options tab status indicator
- Add focus-visible ring to tablet/mobile sub-nav buttons for accessibility

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
src/app/[locale]/settings/providers/_components/forms/provider-form/index.tsx (1)

208-219: ⚠️ Potential issue | 🟡 Minor

不要用固定 500ms 释放滚动锁。

平滑滚动的实际耗时并不固定;目标 section 较远时,这里会提前解锁,handleScroll 会在自动滚动途中把高亮切到经过的 section / sub-section,造成导航跳闪或停在错误项。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/app/`[locale]/settings/providers/_components/forms/provider-form/index.tsx
around lines 208 - 219, Don't rely on a fixed 500ms timer to clear the scroll
lock in scrollToSection; instead compute the target offset (already calculated
as offset) and keep isScrollingToSection.current true until the container
actually reaches that offset (within a small threshold). Implement one of:
attach a temporary 'scroll' listener on contentRef.current that checks
Math.abs(contentRef.current.scrollTop - offset) < threshold and then clears
isScrollingToSection.current and removes the listener (and any existing timer),
or poll with requestAnimationFrame until the scrollTop is within threshold and
then clear. Update scrollToSection, scrollLockTimerRef, isScrollingToSection,
contentRef, sectionRefs and ensure handleScroll respects isScrollingToSection to
avoid mid-scroll highlight changes.
🧹 Nitpick comments (2)
src/app/[locale]/settings/providers/_components/forms/provider-form/index.tsx (1)

36-43: 改成 @/ 别名导入。

这些导入都在 src/ 内部,继续用相对路径不符合仓库规范,也会放大后续重构时的路径维护成本。As per coding guidelines, Use path alias "@/..." to reference files in "./src/" directory.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/app/`[locale]/settings/providers/_components/forms/provider-form/index.tsx
around lines 36 - 43, Replace the relative imports in this file with the project
path-alias "@/..." style: update the imports for FormTabNav, NAV_ORDER,
PARENT_MAP, TAB_ORDER (from "./components/form-tab-nav"), ProviderFormProvider
and useProviderForm (from "./provider-form-context"), NavTargetId/SubTabId/TabId
types (from "./provider-form-types"), and each section import (BasicInfoSection,
LimitsSection, NetworkSection, OptionsSection, RoutingSection) so their module
specifiers begin with "@/..." pointing to the same modules under src; ensure the
new specifiers match your tsconfig/webpack path aliases and keep the imported
symbol names unchanged (e.g., FormTabNav, ProviderFormProvider, useProviderForm,
BasicInfoSection, etc.).
src/app/[locale]/settings/providers/_components/forms/provider-form/components/form-tab-nav.tsx (1)

18-18: 改成 @/ 别名导入。

这里仍然用了指向 src/ 内部模块的相对路径,和仓库约定不一致,后续目录调整时也更脆弱。As per coding guidelines, Use path alias "@/..." to reference files in "./src/" directory.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/app/`[locale]/settings/providers/_components/forms/provider-form/components/form-tab-nav.tsx
at line 18, The import of NavTargetId, SubTabId, and TabId currently uses a
relative path ("../provider-form-types"); change this to use the project path
alias (@"...") instead so it references the provider-form-types module via the
"@/..." alias entry point used across the repo. Update the import statement that
brings in NavTargetId, SubTabId, TabId to the corresponding "@/..." alias path
for provider-form-types, preserving the same named exports and ensuring imports
resolve via the alias configuration.
🤖 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]/settings/providers/_components/forms/provider-form/index.tsx:
- Around line 254-257: handleTabChange currently only dispatches SET_ACTIVE_TAB
which leaves any previous activeSubTab highlighted; update handleTabChange to
also clear the sub-tab state (e.g., dispatch SET_ACTIVE_SUBTAB with null/empty
id) before/after dispatching SET_ACTIVE_TAB and then call scrollToSection(tab),
and ensure the reducer handling SET_ACTIVE_SUBTAB accepts and clears the
activeSubTab state so the UI reflects the new top-level section.

---

Duplicate comments:
In
`@src/app/`[locale]/settings/providers/_components/forms/provider-form/index.tsx:
- Around line 208-219: Don't rely on a fixed 500ms timer to clear the scroll
lock in scrollToSection; instead compute the target offset (already calculated
as offset) and keep isScrollingToSection.current true until the container
actually reaches that offset (within a small threshold). Implement one of:
attach a temporary 'scroll' listener on contentRef.current that checks
Math.abs(contentRef.current.scrollTop - offset) < threshold and then clears
isScrollingToSection.current and removes the listener (and any existing timer),
or poll with requestAnimationFrame until the scrollTop is within threshold and
then clear. Update scrollToSection, scrollLockTimerRef, isScrollingToSection,
contentRef, sectionRefs and ensure handleScroll respects isScrollingToSection to
avoid mid-scroll highlight changes.

---

Nitpick comments:
In
`@src/app/`[locale]/settings/providers/_components/forms/provider-form/components/form-tab-nav.tsx:
- Line 18: The import of NavTargetId, SubTabId, and TabId currently uses a
relative path ("../provider-form-types"); change this to use the project path
alias (@"...") instead so it references the provider-form-types module via the
"@/..." alias entry point used across the repo. Update the import statement that
brings in NavTargetId, SubTabId, TabId to the corresponding "@/..." alias path
for provider-form-types, preserving the same named exports and ensuring imports
resolve via the alias configuration.

In
`@src/app/`[locale]/settings/providers/_components/forms/provider-form/index.tsx:
- Around line 36-43: Replace the relative imports in this file with the project
path-alias "@/..." style: update the imports for FormTabNav, NAV_ORDER,
PARENT_MAP, TAB_ORDER (from "./components/form-tab-nav"), ProviderFormProvider
and useProviderForm (from "./provider-form-context"), NavTargetId/SubTabId/TabId
types (from "./provider-form-types"), and each section import (BasicInfoSection,
LimitsSection, NetworkSection, OptionsSection, RoutingSection) so their module
specifiers begin with "@/..." pointing to the same modules under src; ensure the
new specifiers match your tsconfig/webpack path aliases and keep the imported
symbol names unchanged (e.g., FormTabNav, ProviderFormProvider, useProviderForm,
BasicInfoSection, etc.).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d55bcff4-8368-4259-80e1-f8b4e96b6e46

📥 Commits

Reviewing files that changed from the base of the PR and between 54c1602 and 48863eb.

📒 Files selected for processing (4)
  • messages/zh-CN/settings/providers/form/sections.json
  • messages/zh-TW/settings/providers/form/sections.json
  • src/app/[locale]/settings/providers/_components/forms/provider-form/components/form-tab-nav.tsx
  • src/app/[locale]/settings/providers/_components/forms/provider-form/index.tsx

…d event for scroll lock

- Replace raw reason string fallback with tUI("unknownError") i18n key
  in tag validation callbacks (routing-section.tsx)
- Add "unknownError" key to tagInput namespace in all 5 locales
- Use scrollend event with { once: true } + 1000ms fallback timer
  instead of fixed 500ms setTimeout for scroll lock release (index.tsx)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@coderabbitai coderabbitai bot requested a review from ding113 March 7, 2026 15:06
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
src/app/[locale]/settings/providers/_components/forms/provider-form/index.tsx (1)

216-228: ⚠️ Potential issue | 🟡 Minor

固定 1000ms 的兜底仍会让滚动锁过早释放。

这里每次都会启动定时器。表单内容较长时,平滑滚动可能还没结束就把 isScrollingToSection 置回 falsehandleScroll 会重新接管高亮,导致父/子导航在途中跳闪。建议把兜底条件改成“滚动位置接近目标或滚动已稳定”,不要继续依赖固定时长。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/app/`[locale]/settings/providers/_components/forms/provider-form/index.tsx
around lines 216 - 228, The current scroll unlock always falls back to a fixed
1000ms timeout which can clear isScrollingToSection too early; update the logic
around scrollLockTimerRef/onScrollEnd to instead detect when the scroll either
reaches (or is within a small threshold of) the target section or the scroll has
become stable (no position change for a short debounce window) before calling
unlock(); use contentRef.current.scrollTop (or bounding rect) and a short
repeated check (rAF or small interval) to compare last positions, cancel the
fallback only when position is near the intended target or stable, and keep
references to isScrollingToSection, onScrollEnd, contentRef and handleScroll to
implement this conditional unlock instead of relying on a fixed 1000ms timeout.
🧹 Nitpick comments (1)
src/app/[locale]/settings/providers/_components/forms/provider-form/index.tsx (1)

36-38: 新增仓库内模块导入请统一改为 @/ 别名。

这几处新增导入仍然使用相对路径,和项目约定不一致;后续目录调整时也更容易引入 ../ 回溯错误。建议至少把这次新增的几处先统一到 @/
As per coding guidelines "Use path alias @/ to reference files in ./src/ directory".

Also applies to: 42-42

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@src/app/`[locale]/settings/providers/_components/forms/provider-form/index.tsx
around lines 36 - 38, The three relative imports (FormTabNav, NAV_ORDER,
PARENT_MAP, TAB_ORDER from "./components/form-tab-nav"; ProviderFormProvider and
useProviderForm from "./provider-form-context"; and the types NavTargetId,
SubTabId, TabId from "./provider-form-types") should be replaced to use the
project alias (start with "@/") instead of relative paths; update the import
specifiers to point to the alias-based module locations for those symbols and
ensure the imports still resolve to the same files (and that your
tsconfig/webpack alias config supports the chosen alias).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In
`@src/app/`[locale]/settings/providers/_components/forms/provider-form/index.tsx:
- Around line 216-228: The current scroll unlock always falls back to a fixed
1000ms timeout which can clear isScrollingToSection too early; update the logic
around scrollLockTimerRef/onScrollEnd to instead detect when the scroll either
reaches (or is within a small threshold of) the target section or the scroll has
become stable (no position change for a short debounce window) before calling
unlock(); use contentRef.current.scrollTop (or bounding rect) and a short
repeated check (rAF or small interval) to compare last positions, cancel the
fallback only when position is near the intended target or stable, and keep
references to isScrollingToSection, onScrollEnd, contentRef and handleScroll to
implement this conditional unlock instead of relying on a fixed 1000ms timeout.

---

Nitpick comments:
In
`@src/app/`[locale]/settings/providers/_components/forms/provider-form/index.tsx:
- Around line 36-38: The three relative imports (FormTabNav, NAV_ORDER,
PARENT_MAP, TAB_ORDER from "./components/form-tab-nav"; ProviderFormProvider and
useProviderForm from "./provider-form-context"; and the types NavTargetId,
SubTabId, TabId from "./provider-form-types") should be replaced to use the
project alias (start with "@/") instead of relative paths; update the import
specifiers to point to the alias-based module locations for those symbols and
ensure the imports still resolve to the same files (and that your
tsconfig/webpack alias config supports the chosen alias).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 454a1dc4-fb1c-4053-af37-f56eac1c1f20

📥 Commits

Reviewing files that changed from the base of the PR and between 48863eb and 5a3ef4f.

📒 Files selected for processing (7)
  • messages/en/ui.json
  • messages/ja/ui.json
  • messages/ru/ui.json
  • messages/zh-CN/ui.json
  • messages/zh-TW/ui.json
  • src/app/[locale]/settings/providers/_components/forms/provider-form/index.tsx
  • src/app/[locale]/settings/providers/_components/forms/provider-form/sections/routing-section.tsx

Covers rendering, conditional display by provider type (claude/codex/gemini),
batch mode, dispatch actions, active time UI, disabled state, edit mode IDs,
and batch-only badges.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (3)
tests/unit/settings/providers/options-section.test.tsx (3)

499-506: Switch 数量断言与 DOM 顺序耦合

测试假设容器中恰好有 3 个 switch 元素,这与组件实现紧密耦合。如果后续添加新的 switch 控件,此测试将失败。

可以考虑改为仅验证所有 switch 都被禁用,而不断言具体数量,或在注释中说明预期的 3 个 switch 分别对应哪些功能。

♻️ 可选:添加注释说明或改用更灵活的断言
       const switches = Array.from(
         container.querySelectorAll('[data-testid="switch"]')
       ) as HTMLButtonElement[];
 
-      expect(switches).toHaveLength(3);
+      // Expect 3 switches: preserveClientIp, swapCacheTtlBilling, activeTime
+      expect(switches.length).toBeGreaterThanOrEqual(1);
       for (const toggle of switches) {
         expect(toggle.hasAttribute("disabled")).toBe(true);
       }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/unit/settings/providers/options-section.test.tsx` around lines 499 -
506, The test is tightly coupled to there being exactly 3 switch elements;
instead of asserting a fixed count, change the assertion to verify that all
elements matched by container.querySelectorAll('[data-testid="switch"]') (the
variable switches) are disabled, or narrow the selector to specific switches by
role/label if you need to assert presence of particular controls; remove or
comment out expect(switches).toHaveLength(3) and keep the loop that checks
toggle.hasAttribute("disabled") (or replace with a single every() check) so the
test no longer fails when additional switch controls are added.

197-199: 脆弱的位置选择器:依赖 DOM 索引 [2] 查找开关元素

getActiveTimeToggle 通过硬编码索引 [2] 获取第三个 switch 元素。如果组件中 switch 的顺序发生变化,测试将静默失败或产生误报。

建议使用更具语义化的选择方式,例如为 active time toggle 添加专用的 data-testid 或通过 id 属性查找。

♻️ 建议使用更健壮的选择器

如果 OptionsSection 组件中的 active time toggle 有对应的 id(类似于 preserve-client-ip),可以直接使用:

 function getActiveTimeToggle(container: HTMLDivElement) {
-  return container.querySelectorAll('[data-testid="switch"]')[2] as HTMLButtonElement | null;
+  return document.getElementById("active-time-toggle") as HTMLButtonElement | null;
 }

或者在组件中为该 toggle 添加专用的 data-testid

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/unit/settings/providers/options-section.test.tsx` around lines 197 -
199, The test uses a fragile DOM index in getActiveTimeToggle (it calls
container.querySelectorAll('[data-testid="switch"]')[2]) which will break if
switch order changes; update the selector to target a stable identifier (e.g.,
add a dedicated data-testid like data-testid="active-time-switch" or an id on
the active time toggle in the OptionsSection component) and change
getActiveTimeToggle to query for that specific attribute instead of relying on
the [2] index.

522-533: 补充其他 batch mode badge 的测试覆盖

测试仅包含 batchNotes.codexOnly 的验证。根据 options-section.tsx 的实现,batch 模式下存在三个 provider-specific badges:batchNotes.codexOnlybatchNotes.claudeOnlybatchNotes.geminiOnly。建议补充测试覆盖缺失的两个 badge。

♻️ 建议补充其他 badge 的测试
it("shows claude-only badge in batch mode", () => {
  const { unmount } = renderSection({
    mode: "batch",
    state: createMockState({ routing: { providerType: "claude" } }),
  });

  expect(getBodyText()).toContain("batchNotes.claudeOnly");

  unmount();
});

it("shows gemini-only badge in batch mode", () => {
  const { unmount } = renderSection({
    mode: "batch",
    state: createMockState({ routing: { providerType: "gemini" } }),
  });

  expect(getBodyText()).toContain("batchNotes.geminiOnly");

  unmount();
});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/unit/settings/providers/options-section.test.tsx` around lines 522 -
533, Add missing batch-mode badge tests for the Claude and Gemini providers:
copy the pattern used by the existing "shows codex-only badge in batch mode"
test and add two new it blocks that call renderSection({ mode: "batch", state:
createMockState({ routing: { providerType: "claude" } }) }) and renderSection({
mode: "batch", state: createMockState({ routing: { providerType: "gemini" } })
}), then assert getBodyText() contains "batchNotes.claudeOnly" and
"batchNotes.geminiOnly" respectively, and unmount each render; use the same
helpers renderSection, createMockState, and getBodyText as in the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@tests/unit/settings/providers/options-section.test.tsx`:
- Around line 499-506: The test is tightly coupled to there being exactly 3
switch elements; instead of asserting a fixed count, change the assertion to
verify that all elements matched by
container.querySelectorAll('[data-testid="switch"]') (the variable switches) are
disabled, or narrow the selector to specific switches by role/label if you need
to assert presence of particular controls; remove or comment out
expect(switches).toHaveLength(3) and keep the loop that checks
toggle.hasAttribute("disabled") (or replace with a single every() check) so the
test no longer fails when additional switch controls are added.
- Around line 197-199: The test uses a fragile DOM index in getActiveTimeToggle
(it calls container.querySelectorAll('[data-testid="switch"]')[2]) which will
break if switch order changes; update the selector to target a stable identifier
(e.g., add a dedicated data-testid like data-testid="active-time-switch" or an
id on the active time toggle in the OptionsSection component) and change
getActiveTimeToggle to query for that specific attribute instead of relying on
the [2] index.
- Around line 522-533: Add missing batch-mode badge tests for the Claude and
Gemini providers: copy the pattern used by the existing "shows codex-only badge
in batch mode" test and add two new it blocks that call renderSection({ mode:
"batch", state: createMockState({ routing: { providerType: "claude" } }) }) and
renderSection({ mode: "batch", state: createMockState({ routing: { providerType:
"gemini" } }) }), then assert getBodyText() contains "batchNotes.claudeOnly" and
"batchNotes.geminiOnly" respectively, and unmount each render; use the same
helpers renderSection, createMockState, and getBodyText as in the file.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5ede31d1-7d75-491e-a58a-6235214e0e07

📥 Commits

Reviewing files that changed from the base of the PR and between 5a3ef4f and d1fba16.

📒 Files selected for processing (1)
  • tests/unit/settings/providers/options-section.test.tsx

John Doe and others added 2 commits March 7, 2026 18:25
Store previous scrollend listener in a ref and remove it at the start
of each scrollToSection call, preventing premature unlock when multiple
smooth scrolls overlap during fast sequential tab clicks.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove dead subSectionRefs.options property from OptionsSection
  (parent div in index.tsx already tracks this ref)
- Use filteredNav.find() instead of NAV_BY_ID for tablet/mobile
  sub-row lookup so excludeTabs is respected; remove unused NAV_BY_ID
- Replace non-null assertion with guarded clearTimeout in scrollend
  handler

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
John Doe and others added 7 commits March 7, 2026 19:37
All caused by commit 2e663cd which changed billing model and session
API without updating tests/translations:

- i18n: add missing `prices.badges.multi` key to ja/ru/zh-TW locales
- tests: update cost-calculation expectations to match full-request
  pricing (all tokens at premium rate when context > 200K threshold)
- tests: fix lease-decrement session mock to use
  getResolvedPricingByBillingSource instead of removed method

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ompat

Recharts v3 widened `dataKey` to `string | number | ((obj: any) => any) | undefined`,
which is incompatible with React `Key` and `ReactNode`. Wrap with `String()` in 2 files
to satisfy tsgo in CI. Pre-existing issue from main, not introduced by this branch.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Removing the explicit .map() parameter type exposed ValueType
(string | number | readonly (string|number)[]) which is too wide
for the formatter's (string | number) parameter. Cast item.value.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Bring back the explicit type annotation on .map() callback but extend
dataKey to include ((obj: any) => any) to match Recharts v3 DataKey<any>.
This keeps value/name narrowed for formatter compatibility while making
the annotation assignable from TooltipPayloadEntry.

Replaces the previous approach of removing the annotation entirely,
which exposed ValueType and NameType width issues one by one.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…mpat

Remove the .map() parameter annotation entirely (it causes TS2345 due to
contravariance — our narrower type is not assignable from TooltipPayloadEntry).
Instead, let TS infer the full type and cast only at the two call sites where
formatter needs narrower types: item.value as string|number, item.name as string.

All other usages of item.dataKey, item.name, item.value are compatible with
the wider Recharts v3 types (String() wraps, template literals, ReactNode).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@ding113 ding113 merged commit 1277e97 into ding113:dev Mar 8, 2026
9 checks passed
@github-project-automation github-project-automation bot moved this from Backlog to Done in Claude Code Hub Roadmap Mar 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:i18n area:provider enhancement New feature or request size/M Medium PR (< 500 lines)

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants