fix(web): tighten form field accessibility semantics#3432
Open
MisonL wants to merge 6 commits intoQuantumNous:mainfrom
Open
fix(web): tighten form field accessibility semantics#3432MisonL wants to merge 6 commits intoQuantumNous:mainfrom
MisonL wants to merge 6 commits intoQuantumNous:mainfrom
Conversation
Contributor
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a Responses Stream Bootstrap Recovery feature: new service config/state and helpers, middleware/controller integration to probe upstream channels while keeping SSE connections with : PING during a startup grace window, billing/refund adjustments, SSE error helpers, docs, i18n, frontend accessibility/dom-patch hooks, and tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Distributor as Middleware/Distributor
participant BootstrapSvc as ResponsesBootstrapService
participant Cache as ChannelCache
participant Relay as RelayController
participant Upstream as Channel
Client->>Distributor: POST /v1/responses (stream=true)
Distributor->>BootstrapSvc: EnsureResponsesBootstrapRecoveryState(isStream)
BootstrapSvc-->>Distributor: recoveryState
Distributor->>Cache: CacheGetRandomSatisfiedChannel()
Cache-->>Distributor: nil
Distributor->>BootstrapSvc: CanContinueResponsesBootstrapRecovery(err)
BootstrapSvc-->>Distributor: true
Distributor->>Client: Set event-stream headers, send ": PING"
Distributor->>BootstrapSvc: NextResponsesBootstrapWait(now)
BootstrapSvc-->>Distributor: waitDuration, shouldPing
Distributor->>Distributor: sleep waitDuration (ctx-aware)
Distributor->>Cache: CacheGetRandomSatisfiedChannel()
Cache-->>Distributor: channel
Distributor->>Relay: forward request to Relay via channel
Relay->>Upstream: upstream request
Upstream-->>Relay: streaming response
Relay-->>Client: stream payload chunks
sequenceDiagram
participant Client
participant RelayCtrl as Relay Controller
participant BootstrapSvc as ResponsesBootstrapService
participant Cache as ChannelCache
participant Upstream as Channel
Client->>RelayCtrl: POST /v1/responses (stream)
RelayCtrl->>BootstrapSvc: EnsureResponsesBootstrapRecoveryState(isStream)
BootstrapSvc-->>RelayCtrl: recoveryState
loop Outer bootstrap loop
RelayCtrl->>RelayCtrl: reset retry params
loop Inner retries
RelayCtrl->>Cache: select channel
Cache-->>RelayCtrl: channel or error
RelayCtrl->>Upstream: relay request
Upstream-->>RelayCtrl: API error (retryable)
end
RelayCtrl->>BootstrapSvc: CanContinueResponsesBootstrapRecovery(err)
BootstrapSvc-->>RelayCtrl: true
RelayCtrl->>RelayCtrl: releaseBootstrapRecoveryBilling()
RelayCtrl->>BootstrapSvc: NextResponsesBootstrapWait(now)
BootstrapSvc-->>RelayCtrl: waitDuration, shouldPing
alt shouldPing
RelayCtrl->>Client: send ": PING" SSE keepalive
end
RelayCtrl->>RelayCtrl: waitForResponsesBootstrapRecoveryProbe()
end
alt ShouldWriteResponsesBootstrapStreamError
RelayCtrl->>Client: SetEventStreamHeaders + OpenAIErrorEvent
else
RelayCtrl->>Client: JSON error response
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (18)
web/src/components/table/channels/modals/BatchTagModal.jsx (1)
46-50:⚠️ Potential issue | 🟠 MajorAdd a programmatic label for the input (not just
name).
nameimproves selector stability, but this field still lacks an explicit accessible name binding. Placeholder text alone is not a reliable label for assistive tech. Please addaria-labeloraria-labelledby(or a proper<label htmlFor>+id) to ensure this modal input is announced correctly.Suggested patch
<Input placeholder={t('请输入标签名称')} value={batchSetTagValue} onChange={(v) => setBatchSetTagValue(v)} + aria-label={t('请输入要设置的标签名称')} name='components-table-channels-modals-batchtagmodal-input-1' />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/table/channels/modals/BatchTagModal.jsx` around lines 46 - 50, The Input field in BatchTagModal (the Input component with props value={batchSetTagValue}, onChange={(v) => setBatchSetTagValue(v)}, name='components-table-channels-modals-batchtagmodal-input-1') lacks an accessible label; add an explicit accessible name by either adding aria-label="标签名称" (or a localized string via t('...')) to the Input or give the Input an id and pair it with a <label htmlFor="..."> in the modal header/above the input (or use aria-labelledby pointing to that label) so screen readers announce the field correctly.web/src/components/table/model-deployments/modals/ConfirmationDialog.jsx (1)
83-89:⚠️ Potential issue | 🟠 Major
namealone doesn’t fix input accessibility; add a programmatic label association.This field is still unlabeled for assistive tech. Please add an
idand bind it to a visible label (htmlFor) or addaria-label/aria-labelledby.Suggested patch
- <Text> + <Text id='deployment-confirmation-label'> {t('请输入部署名称以完成二次确认')}： <Text code className='ml-1'> {requiredText || t('未知部署')} </Text> </Text> <Input + id='deployment-confirmation-input' + aria-labelledby='deployment-confirmation-label' value={confirmText} onChange={setConfirmText} placeholder={t('再次输入部署名称')} autoFocus name='components-table-model-deployments-modals-confirmationdialog-input-1' />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/table/model-deployments/modals/ConfirmationDialog.jsx` around lines 83 - 89, The Input in ConfirmationDialog.jsx currently only sets a name, leaving it unlabeled for assistive tech; add a programmatic label association by giving the Input an id (e.g., id="confirm-deployment-name") and either render a visible <label htmlFor="confirm-deployment-name"> (using the same translation text via t(...)) or add aria-label or aria-labelledby to the Input (e.g., aria-label={t('再次输入部署名称')}); update any references that rely on the name prop if needed but keep the existing name prop.web/src/components/common/ui/SelectableButtonGroup.jsx (1)
197-202:⚠️ Potential issue | 🟡 MinorHardcoded
nameis identical for all checkboxes in the loop.Since this
Checkboxis rendered inside a.map()overitems, every checkbox in the group receives the samename. For multi-select scenarios (whereactiveValueis an array), this can confuse assistive technologies and cause form submission issues.Consider making the name unique per item:
🛠️ Suggested fix
<Checkbox checked={isActive} onChange={() => onChange(item.value)} style={{ pointerEvents: 'auto' }} - name='components-common-ui-selectablebuttongroup-checkbox-1' + name={`selectablebuttongroup-checkbox-${item.value}`} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/common/ui/SelectableButtonGroup.jsx` around lines 197 - 202, The Checkbox inside the items.map renders the same hardcoded name ('components-common-ui-selectablebuttongroup-checkbox-1') for every item which can break accessibility and form behavior; update the name prop on the Checkbox in SelectableButtonGroup.jsx to be unique per item (e.g., derive from item.value or item.id and/or the index) so each checkbox has a distinct name while keeping the current onChange(item.value) and checked={isActive} behavior; ensure the unique naming is applied consistently for single- and multi-select modes where activeValue may be an array.web/src/components/table/models/modals/MissingModelsModal.jsx (1)
149-160:⚠️ Potential issue | 🟠 MajorAdd
idand accessible label for the search input.The PR aims to "tighten form field accessibility semantics" and fix "unlabeled fields," but this Input lacks proper labeling for screen readers. While a
nameattribute was added, the field is still missing:
- An
idattribute for programmatic association- An associated
<label>element oraria-labelattributePlaceholders alone do not meet WCAG accessibility standards, as they disappear on input and are not reliably announced by assistive technologies.
♿ Proposed fix to add proper labeling
{/* 搜索框 */} <div className='flex items-center justify-end gap-2 w-full mb-4'> <Input + id='missing-models-search' + aria-label={t('搜索模型...')} placeholder={t('搜索模型...')} value={searchKeyword} onChange={(v) => { setSearchKeyword(v); setCurrentPage(1); }} className='!w-full' prefix={<IconSearch />} showClear name='components-table-models-modals-missingmodelsmodal-input-1' /> </div>Alternative approach with visible label:
{/* 搜索框 */} <div className='flex items-center justify-end gap-2 w-full mb-4'> + <label htmlFor='missing-models-search' className='sr-only'> + {t('搜索模型...')} + </label> <Input + id='missing-models-search' placeholder={t('搜索模型...')} value={searchKeyword} onChange={(v) => { setSearchKeyword(v); setCurrentPage(1); }} className='!w-full' prefix={<IconSearch />} showClear name='components-table-models-modals-missingmodelsmodal-input-1' /> </div>Note: The
sr-onlyclass (screen reader only) keeps the label visually hidden while making it accessible to assistive technologies.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/table/models/modals/MissingModelsModal.jsx` around lines 149 - 160, The search Input (component Input with props value={searchKeyword}, onChange and name='components-table-models-modals-missingmodelsmodal-input-1') needs an accessible programmatic label: add a unique id prop (e.g., id='missing-models-search') and either add a visually-hidden <label for="that id">Search models</label> adjacent to the Input or supply an aria-label prop with the same descriptive text; ensure the label text is localized via t('搜索模型...') or a dedicated key and keep existing handlers (setSearchKeyword, setCurrentPage) intact.web/src/components/table/channels/modals/ChannelUpstreamUpdateModal.jsx (1)
252-259:⚠️ Potential issue | 🟡 MinorSearch input still lacks an explicit accessible label.
At Line 254, placeholder text is present, and Line 258 adds
name, butnamedoes not provide a reliable accessible name for assistive tech. Please add an explicit label (aria-labelor<label htmlFor=...>).♿ Suggested fix
<Input prefix={<IconSearch size={14} />} placeholder={t('搜索模型')} + aria-label={t('搜索模型')} value={keyword} onChange={(value) => setKeyword(value)} showClear name='components-table-channels-modals-channelupstreamupdatemodal-input-1' />As per coding guidelines: "Use
useTranslation()hook and callt('中文key')in components."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/table/channels/modals/ChannelUpstreamUpdateModal.jsx` around lines 252 - 259, The search Input in ChannelUpstreamUpdateModal.jsx lacks an explicit accessible name; update the Input element (in the ChannelUpstreamUpdateModal component) to provide an explicit label by adding either an aria-label or an associated <label htmlFor=...> plus an id on the Input, and ensure the label text comes from the i18n hook (useTranslation) via t('中文key'); e.g., add id="channel-upstream-search", add a <label htmlFor="channel-upstream-search">{t('...')}</label> or set aria-label={t('...')} on the Input and keep the existing placeholder and name props.web/src/components/settings/CustomOAuthSetting.jsx (1)
651-662:⚠️ Potential issue | 🟠 MajorAdd accessible label association for the Switch.
While adding the
idattribute improves test stability, the Switch still lacks a programmatic label association for screen readers. The visual label "启用供应商" at line 652 is not connected to the Switch semantically, which prevents assistive technologies from announcing its purpose.Since this PR specifically focuses on tightening accessibility semantics, this Switch should have proper ARIA labeling.
♿ Proposed fix using aria-label
<Space spacing={8} align='center'> - <Text type='secondary'>{t('启用供应商')}</Text> + <Text type='secondary' id='switch-enable-provider-label'>{t('启用供应商')}</Text> <Switch checked={!!formValues.enabled} size='large' onChange={(checked) => mergeFormValues({ enabled: !!checked })} id='components-settings-customoauthsetting-switch-1' + aria-labelledby='switch-enable-provider-label' />Alternatively, if adding an id to the Text element is not preferred, use
aria-labeldirectly:<Switch checked={!!formValues.enabled} size='large' onChange={(checked) => mergeFormValues({ enabled: !!checked })} id='components-settings-customoauthsetting-switch-1' + aria-label={t('启用供应商')} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/settings/CustomOAuthSetting.jsx` around lines 651 - 662, The Switch in CustomOAuthSetting.jsx (the Switch component using checked={!!formValues.enabled}, onChange calling mergeFormValues, and the Text label "启用供应商") lacks a programmatic label for screen readers; fix by adding an accessible label association—either give the Text element a stable id and set the Switch's aria-labelledby to that id, or add a descriptive aria-label on the Switch (e.g., aria-label="启用供应商"); ensure the chosen id/label is unique and kept in sync with the visible text so assistive tech can announce the control.web/src/pages/Setting/Ratio/UpstreamRatioSync.jsx (1)
481-489:⚠️ Potential issue | 🟠 MajorAdd an explicit accessible label for the search input.
At Line 488,
nameimproves targeting, but this input still relies on placeholder text for labeling. Please addaria-label(or an associated<label>+id) so screen readers get a stable accessible name.Proposed fix
<Input prefix={<IconSearch size={14} />} placeholder={t('搜索模型名称')} + aria-label={t('搜索模型名称')} value={searchKeyword} onChange={setSearchKeyword} className='w-full sm:w-64' showClear name='pages-setting-ratio-upstreamratiosync-input-1' />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/Setting/Ratio/UpstreamRatioSync.jsx` around lines 481 - 489, The search Input currently relies only on placeholder text; add a stable accessible name by providing an aria-label (or wire up a <label> with an id) on the Input component used with searchKeyword and setSearchKeyword — e.g., add aria-label="搜索模型名称" (or add id="upstream-model-search" and an associated <label htmlFor="upstream-model-search">) to the Input with name 'pages-setting-ratio-upstreamratiosync-input-1' so screen readers receive a persistent label.web/src/components/common/modals/SecureVerificationModal.jsx (1)
166-195: 🛠️ Refactor suggestion | 🟠 MajorAdd proper label or aria-label for screen reader accessibility.
The input currently relies only on a placeholder, which disappears when typing and is not reliably announced by screen readers. Additionally, the helper text on line 208 should be associated with the input via
aria-describedby.♿ Proposed accessibility improvements
+ id='verification-code-input' placeholder={t('请输入6位验证码或8位备用码')} value={code} onChange={onCodeChange} size='large' maxLength={8} onKeyDown={handleKeyDown} autoFocus={method === '2fa'} disabled={loading} + aria-label={t('两步验证码')} + aria-describedby='verification-code-hint' prefix={ <svgAnd wrap the helper text:
<Typography.Text + id='verification-code-hint' type='tertiary' size='small'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/common/modals/SecureVerificationModal.jsx` around lines 166 - 195, The Input in SecureVerificationModal lacks an accessible label and isn’t connected to its helper text; add an explicit label or aria-label to the Input (e.g., aria-label="verification code" or render a visible <label> for the input) and add aria-describedby on the Input pointing to the helper text’s id (give the helper text element a stable id like "secure-verification-helper") so screen readers announce the helper text; update the Input props (aria-label or id/aria-labelledby and aria-describedby) accordingly in the Input component usage.web/src/components/common/ui/JSONEditor.jsx (2)
444-494: 🛠️ Refactor suggestion | 🟠 MajorStatic
nameattributes on loop-rendered inputs cause duplicates.The inputs inside this loop (
name='components-common-ui-jsoneditor-input-2'on line 459, and the value input fromrenderValueInputwithname='components-common-ui-jsoneditor-input-1'on line 408) are rendered once per key-value pair. This results in multiple form elements sharing identicalnameattributes, which:
- Violates HTML form semantics—
nameshould uniquely identify a field for form submission- May cause unexpected behavior if the form is serialized (only the last value is typically captured)
- Doesn't improve accessibility since
namedoesn't associate inputs with labels (that requiresid+htmlFor)Consider generating unique identifiers per input using the pair's
id:♻️ Suggested approach for unique identifiers
<Input placeholder={t('键名')} value={pair.key} onChange={(newKey) => updateKey(pair.id, newKey)} status={isDuplicate ? 'warning' : undefined} - name='components-common-ui-jsoneditor-input-2' + id={`jsoneditor-key-${pair.id}`} + name={`jsoneditor-key-${pair.id}`} />Apply the same pattern in
renderValueInputby passingpairIdto construct unique identifiers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/common/ui/JSONEditor.jsx` around lines 444 - 494, Loop-rendered inputs use static name attributes causing duplicate names; update the map rendering and renderValueInput call to generate unique names per pair using the pair.id. Specifically, in the key input inside the keyValuePairs.map (the Input using onChange -> updateKey and name='components-common-ui-jsoneditor-input-2'), change the name to include pair.id (e.g., components-common-ui-jsoneditor-input-2-${pair.id}), and modify renderValueInput to accept the pair id (call it pairId) and use it to produce a unique name for the value input (components-common-ui-jsoneditor-input-1-${pairId}), ensuring updateKey, removeKeyValue, and existing handlers remain unchanged.
563-605: 🛠️ Refactor suggestion | 🟠 MajorSame duplicate
nameissue in region editor loop.The inputs at lines 574 and 591 use static
nameattributes but are rendered inside themodelPairs.map()loop, causing the same duplication issue noted above.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/common/ui/JSONEditor.jsx` around lines 563 - 605, The two Input components inside the modelPairs.map render loop (the key Input that calls updateKey(pair.id) and the value Input that calls updateValue(pair.id)) both use a static name prop causing duplicate name attributes; change each name to be unique per item (e.g., append pair.id or index to the name) so the name for the key input and the name for the value input are distinct for each pair and remain tied to pair.id used by updateKey/updateValue/removeKeyValue.web/src/pages/Setting/Payment/SettingsPaymentGatewayWaffo.jsx (2)
596-596:⚠️ Potential issue | 🟠 MajorUse Chinese strings as i18n keys per coding guidelines.
The label uses an English string as the translation key, but the project convention requires Chinese source strings as keys. As per coding guidelines: translation files must use Chinese source strings as keys, and components should call
t('中文key').🌐 Proposed fix to use Chinese i18n key
- <Text strong>{t('Pay Method Name')}</Text> + <Text strong>{t('支付方式名称')}</Text>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/Setting/Payment/SettingsPaymentGatewayWaffo.jsx` at line 596, Replace the English i18n key used in the Text label with the project's Chinese source key: find the usage of t('Pay Method Name') in the SettingsPaymentGatewayWaffo component (the Text strong element) and change the key to the appropriate Chinese string (e.g. '支付方式名称'); also add/update that Chinese key and its translations in the locale JSON files so the key resolves correctly at runtime.
583-583:⚠️ Potential issue | 🟠 MajorUse Chinese strings as i18n keys per coding guidelines.
The label uses an English string as the translation key, but the project convention requires Chinese source strings as keys. As per coding guidelines: translation files must use Chinese source strings as keys, and components should call
t('中文key').🌐 Proposed fix to use Chinese i18n key
- <Text strong>{t('Pay Method Type')}</Text> + <Text strong>{t('支付方式类型')}</Text>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/Setting/Payment/SettingsPaymentGatewayWaffo.jsx` at line 583, Replace the English i18n key used in the JSX expression t('Pay Method Type') with the project's Chinese source string key (e.g., t('支付方式类型') or the exact Chinese key defined in your translations) in the <Text> label inside SettingsPaymentGatewayWaffo.jsx; then update the corresponding translation files to include that Chinese key and its translations. Ensure you modify the instance t('Pay Method Type') to t('中文key') and add/update the Chinese key entry in the i18n resource so translations resolve correctly at runtime.web/src/components/table/channels/modals/CodexOAuthModal.jsx (1)
155-161:⚠️ Potential issue | 🟠 MajorMissing accessibility attributes for the Input field.
The Input component lacks proper accessibility semantics required for assistive technologies:
- No label: The field has no associated
<label>element oraria-labelattribute. Placeholders alone do not satisfy WCAG 3.3.2 (Labels or Instructions) because they disappear when the user starts typing and may not be announced by screen readers.- No
id: Without anidattribute, the field cannot be associated with a label element viafor/htmlFor.- No
autocomplete: The PR objectives mention normalizing autocomplete semantics, but this field lacks an appropriateautocompleteattribute (e.g.,autocomplete="off"orautocomplete="url").This creates a major accessibility barrier for keyboard and screen reader users who need to understand the field's purpose.
♿ Proposed fix to add accessibility attributes
+ <label htmlFor='codex-oauth-callback-url' style={{ display: 'block', marginBottom: 4 }}> + {t('回调 URL')} + </label> <Input + id='codex-oauth-callback-url' value={input} onChange={(value) => setInput(value)} placeholder={t('请粘贴完整回调 URL（包含 code 与 state）')} showClear - name='components-table-channels-modals-codexoauthmodal-input-1' + name='codex-oauth-callback-url' + autocomplete='off' + aria-describedby='codex-oauth-callback-hint' />Then update the hint text to have an
id:- <Text type='tertiary' size='small'> + <Text type='tertiary' size='small' id='codex-oauth-callback-hint'> {t( '说明：生成结果是可直接粘贴到渠道密钥里的 JSON（包含 access_token / refresh_token / account_id）。', )} </Text>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/table/channels/modals/CodexOAuthModal.jsx` around lines 155 - 161, The Input component used in CodexOAuthModal (props: value, onChange, placeholder, showClear, name) is missing accessibility attributes; add a stable id (e.g., id="codex-oauth-callback"), provide either an associated <label> (or set aria-label/aria-labelledby) that describes "回调 URL（包含 code 与 state）", and include an appropriate autocomplete attribute (e.g., autocomplete="url" or "off"); if you keep a hint text element, give it an id and reference it via aria-describedby on the Input to expose helper text to screen readers.web/src/components/table/model-deployments/modals/CreateDeploymentModal.jsx (1)
901-910:⚠️ Potential issue | 🟠 MajorApply a shared name to RadioGroup instead of individual Radio options.
Both RadioGroup instances use unique
nameattributes on individual Radio children instead of a shared name at the RadioGroup level. This violates native radio button semantics and accessibility expectations per@douyinfe/semi-uidocumentation.Affected locations:
- Lines 908–909 (imageMode RadioGroup)
- Lines 1411–1412 (priceCurrency RadioGroup)
🔧 Suggested fix
<RadioGroup type='button' + name='components-table-model-deployments-modals-createdeploymentmodal-image-mode' value={imageMode} onChange={(value) => setImageMode(value?.target?.value ?? value) } > - <Radio value='builtin' name='components-table-model-deployments-modals-createdeploymentmodal-radio-1'>{t('内置 Ollama 镜像')}</Radio> - <Radio value='custom' name='components-table-model-deployments-modals-createdeploymentmodal-radio-2'>{t('自定义镜像')}</Radio> + <Radio value='builtin'>{t('内置 Ollama 镜像')}</Radio> + <Radio value='custom'>{t('自定义镜像')}</Radio> </RadioGroup> @@ <RadioGroup type='button' + name='components-table-model-deployments-modals-createdeploymentmodal-price-currency' value={priceCurrency} onChange={handleCurrencyChange} > - <Radio value='usdc' name='components-table-model-deployments-modals-createdeploymentmodal-radio-3'>USDC</Radio> - <Radio value='iocoin' name='components-table-model-deployments-modals-createdeploymentmodal-radio-4'>IOCOIN</Radio> + <Radio value='usdc'>USDC</Radio> + <Radio value='iocoin'>IOCOIN</Radio> </RadioGroup>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/table/model-deployments/modals/CreateDeploymentModal.jsx` around lines 901 - 910, The Radio children currently set individual name attributes which breaks native radio semantics; move the shared name to the RadioGroup component for the imageMode and priceCurrency groups so each RadioGroup has a single name and remove name props from its Radio children (look for the RadioGroup instances controlling imageMode and priceCurrency and the Radio children with names 'components-table-model-deployments-modals-createdeploymentmodal-radio-1/2' and the priceCurrency equivalents), ensuring the RadioGroup receives the shared name prop and onChange still uses setImageMode (or the price currency setter) as before.web/src/components/table/users/modals/EditUserModal.jsx (1)
233-275:⚠️ Potential issue | 🟠 MajorUse
autoComplete(camelCase) for proper browser autofill support in React JSX.Lines 238, 250, 262, 273, and 324 use lowercase
autocompleteinstead ofautoComplete. In React JSX, HTML attributes must be camelCased. While Semi Design's Form.Input forwards props to the native input element, the camelCase convention ensures consistent behavior and aligns with the codebase pattern (see RechargeCard.jsx, NotificationSettings.jsx, RegisterForm.jsx which all useautoComplete).Suggested fix
- autocomplete='username' + autoComplete='username' ... - autocomplete='new-password' + autoComplete='new-password' ... - autocomplete='name' + autoComplete='name' ... - autocomplete='off' + autoComplete='off'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/table/users/modals/EditUserModal.jsx` around lines 233 - 275, The JSX attributes for autofill are using lowercase "autocomplete" on multiple Form.Inputs inside EditUserModal (fields: 'username', 'password', 'display_name', 'remark' and others) which prevents proper React/browser behavior; update each Form.Input prop to use camelCase "autoComplete" instead of "autocomplete" (e.g., in the Form.Input components for username, password, display_name, remark) so the prop forwards correctly to the native input and matches the codebase convention.web/src/components/settings/SystemSetting.jsx (1)
1309-1327:⚠️ Potential issue | 🟠 MajorThese unregistered passkey Select controls can lose their values when editing other form fields.
Both
passkey.user_verification(lines 1309-1327) andpasskey.attachment_preference(lines 1340-1355) use bare<Select>components with onChange handlers that directly callsetInputs. However,handleFormChange(line 365) replaces the entireinputsobject whenever any registered Form field changes. A user can change one of these selects and then edit another passkey field, causing the select value to be overwritten by the form's registered values.Sync these select changes to the form via
formApiRef.current?.setValue():Fix
onChange={(value) => { setInputs((prev) => ({ ...prev, 'passkey.user_verification': value, })); formApiRef.current?.setValue('passkey.user_verification', value); } }Apply the same sync for
passkey.attachment_preference.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/settings/SystemSetting.jsx` around lines 1309 - 1327, The two uncontrolled Selects for passkey.user_verification and passkey.attachment_preference currently call setInputs directly and get clobbered by handleFormChange which replaces the whole inputs object; update their onChange handlers to both update local state (setInputs) and also sync the value into the registered form via formApiRef.current?.setValue(...) so the form's registered values stay in sync with these Selects; apply the same change to both Selects so edits to other form fields won't overwrite their values.web/src/components/table/task-logs/TaskLogsFilters.jsx (1)
92-103:⚠️ Potential issue | 🟠 MajorAssociate
task_id-labelwith the input.The new hidden label is never referenced, so the field still relies on its placeholder for an accessible name.
Suggested fix
<Form.Input field='task_id' name='task_id' + aria-labelledby='task_id-label' prefix={<IconSearch />} placeholder={t('任务 ID')} showClear🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/table/task-logs/TaskLogsFilters.jsx` around lines 92 - 103, The hidden label with id 'task_id-label' isn't associated with the input, so update the Form.Input for the task id (the component using field='task_id' / name='task_id') to reference that label — either give the input an id that matches the label or add aria-labelledby='task_id-label' (or aria-label tied to the label) so the hidden <span id='task_id-label'> is used as the accessible name for the input.web/src/components/table/mj-logs/MjLogsFilters.jsx (1)
92-103:⚠️ Potential issue | 🟠 MajorAssociate
mj_id-labelwith the input.The new hidden label is never referenced, so the field still relies on its placeholder for an accessible name.
Suggested fix
<Form.Input field='mj_id' name='mj_id' + aria-labelledby='mj_id-label' prefix={<IconSearch />} placeholder={t('任务 ID')} showClear🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/table/mj-logs/MjLogsFilters.jsx` around lines 92 - 103, The hidden label element with id 'mj_id-label' isn't associated with the input, so add an accessible association on the Form.Input in MjLogsFilters.jsx: locate the Form.Input with field/name 'mj_id' and add an attribute to reference the label (e.g., aria-labelledby='mj_id-label') or alternatively give the input an id and change the span to use htmlFor, ensuring the input is programmatically linked to the label.
🟡 Minor comments (7)
web/src/components/table/task-logs/modals/ColumnSelectorModal.jsx-93-93 (1)
93-93:⚠️ Potential issue | 🟡 MinorUse a unique per-column checkbox
nameinstead of one shared value.Line 93 assigns the same
nameto all mapped checkboxes, which reduces per-field traceability and makes selectors less specific than intended.Proposed fix
- name='components-table-task-logs-modals-columnselectormodal-checkbox-2' + name={`components-table-task-logs-modals-columnselectormodal-checkbox-${column.key}`}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/table/task-logs/modals/ColumnSelectorModal.jsx` at line 93, In ColumnSelectorModal.jsx, the mapped checkbox elements currently reuse the static name 'components-table-task-logs-modals-columnselectormodal-checkbox-2', so update the checkbox name to be unique per column (for example by concatenating the column id or the map index) inside the map that renders each checkbox (the render where props like checked/onChange are assigned); ensure the name uses a unique identifier such as `${namePrefix}-${column.id}` or `${namePrefix}-${index}` so selectors and form traces are specific to each column checkbox.web/src/components/table/channels/modals/EditChannelModal.jsx-3222-3233 (1)
3222-3233:⚠️ Potential issue | 🟡 MinorAwait clipboard copy before showing success.
copy(inputs.models.join(','))is async elsewhere in this file, but here success is shown immediately. This can report success even when copy fails.🧩 Suggested fix
- onClick={() => { + onClick={async () => { if (inputs.models.length === 0) { showInfo(t('没有模型可以复制')); return; } - try { - copy(inputs.models.join(',')); + try { + const ok = await copy(inputs.models.join(',')); + if (!ok) { + showError(t('复制失败')); + return; + } showSuccess(t('模型列表已复制到剪贴板')); } catch (error) { showError(t('复制失败')); } }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/table/channels/modals/EditChannelModal.jsx` around lines 3222 - 3233, The click handler currently calls copy(inputs.models.join(',')) synchronously and shows success immediately; make the onClick handler async, await the promise from copy(inputs.models.join(',')), and only call showSuccess after the await succeeds, with showError in the catch block to handle failures; update the anonymous onClick function (the handler referencing inputs.models, copy, showSuccess, showError, and showInfo) to be async and use await so the success message reflects the actual result of the clipboard operation.web/src/components/table/channels/modals/EditChannelModal.jsx-3151-3169 (1)
3151-3169:⚠️ Potential issue | 🟡 MinorAdd keyboard activation support for this button element.
This
<span>element is marked withrole='button'and is focusable viatabIndex={0}, but it lacks keyboard activation handlers for Enter and Space keys. Convert it to a real<button>element for proper accessibility:Suggested fix
- <span + <button + type='button' className='cursor-pointer select-none' - role='button' - tabIndex={0} title={t('点击复制模型名称')} onClick={async (e) => { e.stopPropagation(); const ok = await copy(modelName); if (ok) { showSuccess( t('已复制：{{name}}', { name: modelName }), ); } else { showError(t('复制失败')); } }} + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + e.currentTarget.click(); + } + }} > {optionNode.label || modelName} - </span> + </button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/table/channels/modals/EditChannelModal.jsx` around lines 3151 - 3169, Replace the non-semantic <span> (with role='button' and tabIndex) with a real <button type="button"> to provide built-in keyboard activation and accessibility; keep the existing className ('cursor-pointer select-none'), title, the async onClick handler that calls copy(modelName) and uses showSuccess/showError with t(...), and preserve e.stopPropagation and the inner text (optionNode.label || modelName); remove role and tabIndex attributes after converting to a button so visual styling and behavior remain identical while enabling Enter/Space activation.web/src/components/table/model-deployments/modals/CreateDeploymentModal.jsx-1211-1211 (1)
1211-1211:⚠️ Potential issue | 🟡 MinorDynamic row inputs still have duplicate
namevalues.Line 1211, Line 1245, Line 1287, Line 1302, Line 1346, and Line 1363 are inside mapped rows but use fixed names, so repeated rows share identical names. This makes selectors less specific and can create ambiguity for tooling/autofill.
🔧 Suggested fix
- name='components-table-model-deployments-modals-createdeploymentmodal-input-2' + name={`components-table-model-deployments-modals-createdeploymentmodal-entrypoint-${index}`} @@ - name='components-table-model-deployments-modals-createdeploymentmodal-input-3' + name={`components-table-model-deployments-modals-createdeploymentmodal-args-${index}`} @@ - name='components-table-model-deployments-modals-createdeploymentmodal-input-4' + name={`components-table-model-deployments-modals-createdeploymentmodal-env-key-${index}`} @@ - name='components-table-model-deployments-modals-createdeploymentmodal-input-5' + name={`components-table-model-deployments-modals-createdeploymentmodal-env-value-${index}`} @@ - name='components-table-model-deployments-modals-createdeploymentmodal-input-6' + name={`components-table-model-deployments-modals-createdeploymentmodal-secret-env-key-${index}`} @@ - name='components-table-model-deployments-modals-createdeploymentmodal-input-7' + name={`components-table-model-deployments-modals-createdeploymentmodal-secret-env-value-${index}`}Also applies to: 1245-1245, 1287-1287, 1302-1302, 1346-1346, 1363-1363
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/table/model-deployments/modals/CreateDeploymentModal.jsx` at line 1211, The mapped row inputs in CreateDeploymentModal.jsx use fixed name attributes (e.g., name='components-table-model-deployments-modals-createdeploymentmodal-input-2') causing duplicate names across rows; update each input inside the map (the JSX where rows are rendered in CreateDeploymentModal) to build a unique name per row by appending the map index or the row's unique id (e.g., `${baseName}-${index}` or `${baseName}-${row.id}`) for all occurrences referenced (the inputs at the locations currently using the fixed string), and ensure any handlers or tests that rely on the name are updated to use the new dynamic name generation consistently.web/src/components/settings/SystemSetting.jsx-946-947 (1)
946-947:⚠️ Potential issue | 🟡 MinorUse one shared radio-group name or remove individual names entirely.
Each
Radioshould not have its ownnameattribute. Radio grouping semantics should be managed at theRadioGrouplevel. This matches the pattern used elsewhere in the codebase (e.g., SettingsPerformance.jsx, ModelPricingEditor.jsx) where the name is on the group. ForForm.RadioGroup, the form binding is already managed via thefieldattribute, so individual Radio names are unnecessary.Applies to lines 946–947 and 1023–1024.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/settings/SystemSetting.jsx` around lines 946 - 947, Remove per-Radio name attributes and instead rely on the parent RadioGroup (or Form.RadioGroup) for grouping; update the Radio elements in SystemSetting.jsx (the Radio components with value='whitelist' and value='blacklist' and the other pair at the later block) to eliminate their individual name props, and if a name is required for accessibility/consistency, add a single shared name on the enclosing RadioGroup/Form.RadioGroup (or rely on the existing field binding) rather than assigning names on each Radio.web/src/components/table/usage-logs/UsageLogsFilters.jsx-95-106 (1)
95-106:⚠️ Potential issue | 🟡 MinorScreen-reader labels are not programmatically associated with inputs.
The
sr-only<span>elements (e.g.,id="token_name-label") provide visually hidden text, butForm.Inputdoes not reference them viaaria-labelledby. Without this linkage, assistive technologies may not announce the label when focusing the input. Consider addingaria-labelledbyto each input or wrapping the input inside a<label>.♻️ Suggested association pattern
<span id='token_name-label' className='sr-only'> {t('令牌名称')} </span> <Form.Input field='token_name' name='token_name' + aria-labelledby='token_name-label' prefix={<IconSearch />} placeholder={t('令牌名称')} showClear pure size='small' />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/components/table/usage-logs/UsageLogsFilters.jsx` around lines 95 - 106, The sr-only span with id "token_name-label" is not programmatically associated with the corresponding input, so update the Form.Input for the token_name field to include aria-labelledby="token_name-label" (or wrap the input in a <label> that references the same id) so screen readers announce the hidden label; apply the same pattern for other inputs with sr-only spans and ensure the unique ids (e.g., token_name-label) match the aria-labelledby values on the Form.Input components.service/responses_bootstrap_recovery.go-73-76 (1)
73-76:⚠️ Potential issue | 🟡 MinorMatch
/v1/responseson a path boundary.
strings.HasPrefix(path, "/v1/responses")also matches sibling paths like/v1/responses123and/v1/responses-foo, so this can enable recovery on unrelated routes. Match the exact path or a/v1/responses/descendant instead.Suggested fix
func IsResponsesBootstrapRecoveryPath(path string) bool { - return strings.HasPrefix(path, "/v1/responses") && + return (path == "/v1/responses" || strings.HasPrefix(path, "/v1/responses/")) && !strings.HasPrefix(path, "/v1/responses/compact") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@service/responses_bootstrap_recovery.go` around lines 73 - 76, The current IsResponsesBootstrapRecoveryPath uses strings.HasPrefix(path, "/v1/responses") which also matches sibling routes like "/v1/responses-foo"; change the check to allow only the exact path "/v1/responses" or descendants under "/v1/responses/" (i.e. path == "/v1/responses" || strings.HasPrefix(path, "/v1/responses/")) while preserving the existing exclusion for the compact endpoint (keep the !strings.HasPrefix(path, "/v1/responses/compact") condition); update the function IsResponsesBootstrapRecoveryPath accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5eb41fed-31a0-4fc8-9456-76ee58c0e2be
📒 Files selected for processing (105)
constant/context_key.gocontroller/relay.godocs/installation/responses-stream-bootstrap-recovery.mdmiddleware/distributor.gomiddleware/distributor_bootstrap_integration_test.gomiddleware/utils.gomiddleware/utils_test.gorelay/helper/common.gorelay/helper/stream_scanner.gorelay/helper/stream_scanner_test.goservice/responses_bootstrap_recovery.goservice/responses_bootstrap_recovery_test.gosetting/operation_setting/general_setting.goweb/src/components/auth/LoginForm.jsxweb/src/components/auth/RegisterForm.jsxweb/src/components/common/DocumentRenderer/index.jsxweb/src/components/common/modals/RiskAcknowledgementModal.jsxweb/src/components/common/modals/SecureVerificationModal.jsxweb/src/components/common/ui/JSONEditor.jsxweb/src/components/common/ui/SelectableButtonGroup.jsxweb/src/components/layout/PageLayout.jsxweb/src/components/playground/ConfigManager.jsxweb/src/components/playground/CustomInputRender.jsxweb/src/components/playground/CustomRequestEditor.jsxweb/src/components/playground/ImageUrlInput.jsxweb/src/components/playground/MessageContent.jsxweb/src/components/playground/ParameterControl.jsxweb/src/components/playground/SettingsPanel.jsxweb/src/components/settings/ChannelSelectorModal.jsxweb/src/components/settings/CustomOAuthSetting.jsxweb/src/components/settings/OperationSetting.jsxweb/src/components/settings/SystemSetting.jsxweb/src/components/settings/personal/cards/AccountManagement.jsxweb/src/components/settings/personal/cards/NotificationSettings.jsxweb/src/components/settings/personal/components/TwoFASetting.jsxweb/src/components/table/channels/ChannelsActions.jsxweb/src/components/table/channels/ChannelsColumnDefs.jsxweb/src/components/table/channels/ChannelsFilters.jsxweb/src/components/table/channels/modals/BatchTagModal.jsxweb/src/components/table/channels/modals/ChannelUpstreamUpdateModal.jsxweb/src/components/table/channels/modals/CodexOAuthModal.jsxweb/src/components/table/channels/modals/ColumnSelectorModal.jsxweb/src/components/table/channels/modals/EditChannelModal.jsxweb/src/components/table/channels/modals/ModelSelectModal.jsxweb/src/components/table/channels/modals/ModelTestModal.jsxweb/src/components/table/channels/modals/OllamaModelModal.jsxweb/src/components/table/channels/modals/ParamOverrideEditorModal.jsxweb/src/components/table/channels/modals/SingleModelSelectModal.jsxweb/src/components/table/mj-logs/MjLogsFilters.jsxweb/src/components/table/mj-logs/modals/ColumnSelectorModal.jsxweb/src/components/table/model-deployments/modals/ColumnSelectorModal.jsxweb/src/components/table/model-deployments/modals/ConfirmationDialog.jsxweb/src/components/table/model-deployments/modals/CreateDeploymentModal.jsxweb/src/components/table/model-deployments/modals/UpdateConfigModal.jsxweb/src/components/table/model-deployments/modals/ViewLogsModal.jsxweb/src/components/table/model-pricing/layout/header/SearchActions.jsxweb/src/components/table/model-pricing/view/card/PricingCardView.jsxweb/src/components/table/models/ModelsFilters.jsxweb/src/components/table/models/modals/MissingModelsModal.jsxweb/src/components/table/models/modals/UpstreamConflictModal.jsxweb/src/components/table/redemptions/RedemptionsFilters.jsxweb/src/components/table/redemptions/modals/EditRedemptionModal.jsxweb/src/components/table/task-logs/TaskLogsFilters.jsxweb/src/components/table/task-logs/modals/ColumnSelectorModal.jsxweb/src/components/table/tokens/TokensColumnDefs.jsxweb/src/components/table/tokens/TokensFilters.jsxweb/src/components/table/tokens/modals/CCSwitchModal.jsxweb/src/components/table/tokens/modals/EditTokenModal.jsxweb/src/components/table/usage-logs/UsageLogsFilters.jsxweb/src/components/table/usage-logs/modals/ColumnSelectorModal.jsxweb/src/components/table/users/UsersFilters.jsxweb/src/components/table/users/modals/AddUserModal.jsxweb/src/components/table/users/modals/EditUserModal.jsxweb/src/components/table/users/modals/UserBindingManagementModal.jsxweb/src/components/topup/InvitationCard.jsxweb/src/components/topup/RechargeCard.jsxweb/src/components/topup/modals/TopupHistoryModal.jsxweb/src/components/topup/modals/TransferModal.jsxweb/src/hooks/common/useFormFieldA11yPatch.jsweb/src/hooks/common/useRepeatingDomPatch.jsweb/src/i18n/locales/en.jsonweb/src/i18n/locales/fr.jsonweb/src/i18n/locales/ja.jsonweb/src/i18n/locales/ru.jsonweb/src/i18n/locales/vi.jsonweb/src/i18n/locales/zh-CN.jsonweb/src/i18n/locales/zh-TW.jsonweb/src/pages/Home/index.jsxweb/src/pages/PrivacyPolicy/index.jsxweb/src/pages/Setting/Chat/SettingsChats.jsxweb/src/pages/Setting/Dashboard/SettingsAPIInfo.jsxweb/src/pages/Setting/Dashboard/SettingsAnnouncements.jsxweb/src/pages/Setting/Dashboard/SettingsFAQ.jsxweb/src/pages/Setting/Dashboard/SettingsUptimeKuma.jsxweb/src/pages/Setting/Operation/SettingsChannelAffinity.jsxweb/src/pages/Setting/Operation/SettingsGeneral.jsxweb/src/pages/Setting/Operation/SettingsHeaderNavModules.jsxweb/src/pages/Setting/Operation/SettingsLog.jsxweb/src/pages/Setting/Operation/SettingsSidebarModulesAdmin.jsxweb/src/pages/Setting/Payment/SettingsPaymentGatewayCreem.jsxweb/src/pages/Setting/Payment/SettingsPaymentGatewayWaffo.jsxweb/src/pages/Setting/Performance/SettingsPerformance.jsxweb/src/pages/Setting/Ratio/UpstreamRatioSync.jsxweb/src/pages/Setting/Ratio/components/ModelPricingEditor.jsxweb/src/pages/UserAgreement/index.jsx
web/src/components/settings/personal/cards/NotificationSettings.jsx
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/src/components/settings/personal/cards/NotificationSettings.jsx`:
- Line 930: The switch element IDs in NotificationSettings.jsx are built using
`${section}` and `${module}`, which stringify to "[object Object]" and cause
duplicate IDs; update the ID construction to use the unique keys (e.g.,
`section.key` and `module.key`) instead of the whole objects wherever IDs are
built for switches (look for the id template like
`components-settings-personal-cards-notificationsettings-switch-1-${section}`
and its module counterpart) so each switch has a stable, unique identifier
derived from section.key and module.key.
- Around line 257-264: The gotifyPriority block creates duplicate IDs because it
assigns id='gotifyPriority' to the wrapper and inner input; mirror the pattern
used for warningThreshold by first finding the wrapper element (the element
currently holding id='gotifyPriority'), move/rename that wrapper's id to
'gotifyPriority-wrapper', then set the inner input's id and name to
'gotifyPriority' and autocomplete off; update the code that queries/assigns
using the gotifyPriorityInput variable so the wrapper is renamed before
assigning the input id to avoid duplicate IDs and restore proper label
association.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3adabf5e-1ea6-41f0-811d-1f9b339d8286
📒 Files selected for processing (4)
relay/helper/stream_scanner_test.goweb/src/components/settings/personal/cards/NotificationSettings.jsxweb/src/i18n/locales/zh-CN.jsonweb/src/pages/Setting/Operation/SettingsHeaderNavModules.jsx
✅ Files skipped from review due to trivial changes (1)
- web/src/pages/Setting/Operation/SettingsHeaderNavModules.jsx
🚧 Files skipped from review as they are similar to previous changes (1)
- relay/helper/stream_scanner_test.go
web/src/components/settings/personal/cards/NotificationSettings.jsx
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
middleware/distributor.go (1)
137-149:⚠️ Potential issue | 🟠 MajorDon’t report canceled or expired requests as
ModelNotFound.
getChannelWithBootstrapGrace()can now returnc.Request.Context().Err()from Lines 375-377. This branch still maps every non-nil error toMsgDistributorGetChannelFailedwithtypes.ErrorCodeModelNotFound, so client cancels and grace-window timeout/cancellation will be mislabeled as channel/model availability failures.🛠️ Suggested handling
channel, selectGroup, err = getChannelWithBootstrapGrace(c, modelRequest.Model, usingGroup, bootstrapState) if err != nil { + switch { + case errors.Is(err, context.Canceled): + return + case errors.Is(err, context.DeadlineExceeded): + abortWithOpenAiMessage(c, http.StatusGatewayTimeout, err.Error()) + return + } showGroup := usingGroupAdd
contextto the imports forcontext.Canceledandcontext.DeadlineExceeded.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@middleware/distributor.go` around lines 137 - 149, The error path after getChannelWithBootstrapGrace currently maps all non-nil errors to ModelNotFound; instead detect context cancellation/timeouts and handle them separately: add "context" to imports and, where getChannelWithBootstrapGrace returns err, check errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) and call abortWithOpenAiMessage with an appropriate timeout/cancel HTTP status (e.g. http.StatusRequestTimeout) and a cancellation-specific error code (or a new types.ErrorCodeRequestCanceled constant if one exists), otherwise proceed to build the MsgDistributorGetChannelFailed message and keep the existing types.ErrorCodeModelNotFound handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@middleware/distributor.go`:
- Around line 83-87: The handler currently maps any bootstrapErr from
EnsureResponsesBootstrapRecoveryStateFromRequest to a distributor "invalid
request" response, which misclassifies context cancellation/deadline errors
returned by getChannelWithBootstrapGrace; update the error handling in the
bootstrap block in middleware/distributor.go to detect context errors (use
errors.Is(bootstrapErr, context.Canceled) and errors.Is(bootstrapErr,
context.DeadlineExceeded) or compare to c.Request.Context().Err()), and for
those cases propagate the context error (e.g. call c.Error(bootstrapErr) and
return or otherwise return the original context error status) instead of mapping
them to MsgDistributorGetChannelFailed/MsgDistributorInvalidRequest; leave
non-context errors to be translated to the existing abortWithOpenAiMessage path
so channel-selection failures remain mapped to MsgDistributorGetChannelFailed.
In `@web/src/i18n/locales/fr.json`:
- Around line 3313-3323: Replace the English values in the French locale for the
listed keys with proper French translations: update the entries for "启用
Responses 流启动恢复", "仅在 /v1/responses 流式请求首包前生效，用于短时故障恢复。", "启动恢复等待窗口（秒）",
"在该时间窗口内持续探测可用渠道，超时后返回真实错误。", "渠道探测间隔（毫秒）", "每次重新探测可用渠道之间的等待时间。", "SSE 保活间隔（秒）",
"在等待恢复期间发送 : PING，帮助客户端保持连接。", "可触发启动恢复的状态码", "填写 JSON 数组，例如
[401,403,429,500,502,503,504]。", and the long descriptive key about Responses
stream startup recovery by providing accurate French translations (replace the
current English strings in web/src/i18n/locales/fr.json with French text while
preserving the original JSON keys and punctuation).
- Around line 3312-3323: The new i18n entries were placed after the closing of
the existing "translation" object so they became top-level siblings instead of
keys under "translation"; move the block of keys ("启用 Responses 流启动恢复", "仅在
/v1/responses 流式请求首包前生效，用于短时故障恢复。", etc.) inside the existing "translation"
object in web/src/i18n/locales/fr.json (ensure the trailing commas/braces are
adjusted so the "translation" object remains a single flat JSON object with the
Chinese source strings as keys).
---
Outside diff comments:
In `@middleware/distributor.go`:
- Around line 137-149: The error path after getChannelWithBootstrapGrace
currently maps all non-nil errors to ModelNotFound; instead detect context
cancellation/timeouts and handle them separately: add "context" to imports and,
where getChannelWithBootstrapGrace returns err, check errors.Is(err,
context.Canceled) || errors.Is(err, context.DeadlineExceeded) and call
abortWithOpenAiMessage with an appropriate timeout/cancel HTTP status (e.g.
http.StatusRequestTimeout) and a cancellation-specific error code (or a new
types.ErrorCodeRequestCanceled constant if one exists), otherwise proceed to
build the MsgDistributorGetChannelFailed message and keep the existing
types.ErrorCodeModelNotFound handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6830a420-aae4-49de-a8a6-4256ae5723f4
📒 Files selected for processing (10)
middleware/distributor.goweb/src/components/table/channels/ChannelsColumnDefs.jsxweb/src/components/table/channels/modals/EditChannelModal.jsxweb/src/i18n/locales/en.jsonweb/src/i18n/locales/fr.jsonweb/src/i18n/locales/ja.jsonweb/src/i18n/locales/ru.jsonweb/src/i18n/locales/vi.jsonweb/src/i18n/locales/zh-TW.jsonweb/src/pages/Setting/Performance/SettingsPerformance.jsx
✅ Files skipped from review due to trivial changes (5)
- web/src/components/table/channels/ChannelsColumnDefs.jsx
- web/src/pages/Setting/Performance/SettingsPerformance.jsx
- web/src/i18n/locales/ja.json
- web/src/i18n/locales/en.json
- web/src/i18n/locales/zh-TW.json
🚧 Files skipped from review as they are similar to previous changes (2)
- web/src/i18n/locales/ru.json
- web/src/i18n/locales/vi.json
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
middleware/distributor.go (1)
143-156:⚠️ Potential issue | 🟠 MajorThe grace-loop error path still turns cancellations into
model_not_found.Line 383 returns
c.Request.Context().Err(), but Lines 144-155 still map every non-nil error fromgetChannelWithBootstrapGracetoMsgDistributorGetChannelFailed/ErrorCodeModelNotFound. That means canceled or timed-out streams are still reported as channel-selection failures on this path, and disconnect-driven ping write failures will also land here unless you checkc.Request.Context().Err()first.🐛 Suggested fix
channel, selectGroup, err = getChannelWithBootstrapGrace(c, modelRequest.Model, usingGroup, bootstrapState) if err != nil { + if ctxErr := c.Request.Context().Err(); ctxErr != nil || + errors.Is(err, context.Canceled) || + errors.Is(err, context.DeadlineExceeded) { + if ctxErr != nil { + err = ctxErr + } + c.Error(err) + c.Abort() + return + } showGroup := usingGroup if usingGroup == "auto" { showGroup = fmt.Sprintf("auto(%s)", selectGroup) }Also applies to: 381-383
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@middleware/distributor.go` around lines 143 - 156, The error handling after calling getChannelWithBootstrapGrace incorrectly maps cancellations/timeouts to MsgDistributorGetChannelFailed/ErrorCodeModelNotFound; update the block that inspects err from getChannelWithBootstrapGrace (the call that assigns channel, selectGroup, err) to first check c.Request.Context().Err() (or inspect err for context.Canceled/context.DeadlineExceeded) and short-circuit by aborting with a context-cancellation/timeout response (e.g., propagate the context error or use an appropriate HTTP status) instead of calling abortWithOpenAiMessage with ErrorCodeModelNotFound; only map and log genuine channel-selection/database errors to MsgDistributorGetChannelFailed/ErrorCodeModelNotFound.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/src/i18n/locales/fr.json`:
- Around line 3317-3319: The French locale entries use Chinese source keys that
don't match the UI's t(...) keys, causing fallbacks; open the component
web/src/pages/Setting/Operation/SettingsGeneral.jsx, find the exact t("...")
source keys used for the "rescan/wait interval" and the "send : PING during
recovery" strings, and replace the fr.json keys "每次重新探测可用渠道之间的等待时间。" and
"在等待恢复期间发送 : PING，帮助客户端保持连接。" with those exact source key strings (keeping the
current French translations as the values) so the lookup in SettingsGeneral.jsx
will find the French translations.
---
Duplicate comments:
In `@middleware/distributor.go`:
- Around line 143-156: The error handling after calling
getChannelWithBootstrapGrace incorrectly maps cancellations/timeouts to
MsgDistributorGetChannelFailed/ErrorCodeModelNotFound; update the block that
inspects err from getChannelWithBootstrapGrace (the call that assigns channel,
selectGroup, err) to first check c.Request.Context().Err() (or inspect err for
context.Canceled/context.DeadlineExceeded) and short-circuit by aborting with a
context-cancellation/timeout response (e.g., propagate the context error or use
an appropriate HTTP status) instead of calling abortWithOpenAiMessage with
ErrorCodeModelNotFound; only map and log genuine channel-selection/database
errors to MsgDistributorGetChannelFailed/ErrorCodeModelNotFound.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3ab71ced-7166-4a75-a13f-cb139e9ce560
📒 Files selected for processing (3)
middleware/distributor.goweb/src/components/settings/personal/cards/NotificationSettings.jsxweb/src/i18n/locales/fr.json
Author
|
@coderabbitai please review the latest changes on this PR, especially the remaining unresolved threads on |
Contributor
🧩 Analysis chainLet me look at the existing review comments on the PR and the specific files mentioned. 🏁 Script executed: Length of output: 180055 🏁 Script executed: Length of output: 265372
NotificationSettings.jsx
fr.json
Optional polish (non-blocking)
Net: The previously unresolved items on both files look resolved to me. Nice work tightening the semantics here. ✅ Actions performedReview triggered.
|
MisonL
force-pushed
the
feat/webui-form-field-a11y-fixes
branch
from
69ce53a to
e2bddbd
Compare
MisonL
force-pushed
the
feat/webui-form-field-a11y-fixes
branch
from
e2bddbd to
00e79f9
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📝 变更描述 / Description
responses bootstrap recovery的无关后端改动。EditChannelModal中一段误混入的损坏 JSX，保证该分支自身可解析。🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
📸 运行证明 / Proof of Work
git diff --checkPATH=/Volumes/Work/code/new-api/web/node_modules/.bin:$PATH eslint src/components/table/channels/modals/EditChannelModal.jsx