feat: hide fake streaming whitelist and responses WS from system settings UI#1162
Merged
Conversation
…ings UI - DEFAULT_FAKE_STREAMING_WHITELIST changed to [] - Removed Fake Streaming Whitelist editor from settings form - Removed OpenAI Responses WebSocket toggle from settings form - Keep state vars and save payload intact - Update related tests
📝 Walkthrough高层概览此 PR 移除系统设置表单中的两个配置 UI 部分（OpenAI WebSocket 响应开关和完整的假流白名单编辑器），将默认假流白名单从四个图像模型列表改为空数组，并更新所有相关测试以验证新的行为。 变更假流配置移除
预估代码审查工作量🎯 2 (简单) | ⏱️ ~12 分钟 可能相关的 PR
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/unit/actions/system-config-fake-streaming-setting.test.ts (1)
63-63: ⚡ Quick win避免在测试中重复定义默认常量。
Line [63] 本地常量与
src/types/system-config.ts的默认值语义重复，后续再次调整默认值时容易出现测试与实现漂移。建议直接复用源常量。建议修改
+import { DEFAULT_FAKE_STREAMING_WHITELIST } from "@/types/system-config"; ... -const DEFAULT_FAKE_STREAMING_MODELS: { model: string; groupTags: string[] }[] = []; +const DEFAULT_FAKE_STREAMING_MODELS = [...DEFAULT_FAKE_STREAMING_WHITELIST];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/actions/system-config-fake-streaming-setting.test.ts` at line 63, The test defines a local DEFAULT_FAKE_STREAMING_MODELS which duplicates the canonical default; instead of redefining it, import and reuse the exported DEFAULT_FAKE_STREAMING_MODELS from the module where the default is defined (the system-config types module) and remove the local declaration; update the test's imports to reference that exported symbol so future changes to the source default stay in sync with the tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/unit/actions/system-config-fake-streaming-setting.test.ts`:
- Line 63: The test defines a local DEFAULT_FAKE_STREAMING_MODELS which
duplicates the canonical default; instead of redefining it, import and reuse the
exported DEFAULT_FAKE_STREAMING_MODELS from the module where the default is
defined (the system-config types module) and remove the local declaration;
update the test's imports to reference that exported symbol so future changes to
the source default stay in sync with the tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 900ae85a-69d7-489b-bf24-382fbb540a97
📒 Files selected for processing (4)
src/app/[locale]/settings/config/_components/system-settings-form.tsxsrc/types/system-config.tstests/unit/actions/system-config-fake-streaming-setting.test.tstests/unit/settings/system-settings-form-fake-streaming.test.tsx
💤 Files with no reviewable changes (1)
- src/app/[locale]/settings/config/_components/system-settings-form.tsx
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request removes the user interface for configuring the OpenAI Responses WebSocket and the Fake Streaming Whitelist from the system settings form. Additionally, the default fake streaming whitelist has been changed to an empty array, and the associated unit tests have been updated to verify that these UI components are no longer rendered and that the new default is correctly handled. I have no feedback to provide.
Contributor
🧪 测试结果
总体结果: ✅ 所有测试通过 |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review Summary
No significant issues identified in this PR.
PR Size: M
- Lines changed: 232
- Files changed: 4
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Clean
- Type safety - Clean
- Documentation accuracy - Clean
- Test coverage - Adequate
- Code clarity - Good
Automated review by Codex AI
Contributor
There was a problem hiding this comment.
- Identified open PR
#1162and reviewed the full diff across the 4 changed files. - Applied size label
size/M(20 additions, 212 deletions; 232 lines changed). - Posted a PR review summary (
gh pr review --comment) with “No significant issues identified” and no inline comments (no findings met the ≥80 confidence threshold). - Note: local lint/test validation wasn’t run here because
bunxisn’t available in this runner (bunx: command not found).
github-project-automation
Bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
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.
Summary
This PR hides two advanced system settings from the UI while preserving their backend functionality:
Backend state variables and save payload remain intact - these features can still be configured via API or database.
Problem
The Fake Streaming feature (added in PR #1132) and OpenAI Responses WebSocket (addressed in PRs #1153/#1154) were exposed as configurable UI elements in system settings. However:
Solution
UI Simplification
Default Policy Change
Related Issues & PRs
Context / History
dev分支 + CPA 上游：/v1/responsesWebSocket 连接成功后 Codex 仍报 Connection reset without closing handshake #1150 - WebSocket connection issues with Codex CLI on /v1/responses endpoint (ongoing issue)Implementation History
Changes
Verification
Backend Compatibility
Breaking Changes
None - this is a UI-only change. The features remain functional via:
Migration Notes
Deployments that relied on the default 4-model whitelist being auto-enabled will need to:
Pre-configured whitelist (pre-0.7.5):
Testing Checklist
Description enhanced by Claude AI - adds related Issue/PR context for better traceability
Greptile Summary
This PR hides the Fake Streaming Whitelist editor and the OpenAI Responses WebSocket toggle from the system settings UI while keeping both settings' state variables and save-payload fields intact, so previously configured values continue to be round-tripped to the server on every save.
DEFAULT_FAKE_STREAMING_WHITELISTis changed from 4 hardcoded image-model entries to[], meaning installations with no persisted whitelist (DBnull) will now default to fake streaming disabled instead of the prior four-model set.GroupMultiSelect,Plus,Radio,Trash2) but retains the state and persistence logic, so the feature is hidden rather than removed.Confidence Score: 4/5
Safe to merge; the only user-visible behaviour change is that fresh installations default to fake streaming disabled, which is the stated intent.
The form correctly keeps both hidden settings in state and in the save payload, so no data is silently dropped on save. The default change from four image models to an empty list is intentional, but deployments that relied on the implicit whitelist without ever explicitly saving it will silently lose fake-streaming for those models after the next settings save.
tests/unit/settings/system-settings-form-fake-streaming.test.tsx — the all-locales test validates strings for removed UI without explanation.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[SystemSettingsForm mounts] --> B[useState: fakeStreamingWhitelist\ninitialised from initialSettings] A --> C[useState: enableOpenaiResponsesWebsocket\ninitialised from initialSettings] B --> D{UI editor\nrendered?} D -- Before this PR --> E[Fake Streaming Whitelist editor shown to user] D -- After this PR --> F[Editor hidden] C --> G{WebSocket toggle rendered?} G -- Before this PR --> H[Toggle shown to user] G -- After this PR --> I[Toggle hidden] F --> J[User clicks Save] I --> J E --> J H --> J J --> K[saveSystemSettings called with both values still in payload] K --> L[(DB values persisted unchanged)]Comments Outside Diff (1)
tests/unit/settings/system-settings-form-fake-streaming.test.tsx, line 267-284 (link)The
"all locales define fake streaming labels"test still asserts that every locale definestitle,description,modelLabel,groupsLabel,allGroupsHint,addModel,remove,modelPlaceholder, andemptyStatefor thefakeStreamingsection — but the form that renders those strings was deleted in this PR. The test will continue to pass (the message files still have the strings), but it now gives false coverage confidence for UI that isn't reachable. If the intent is to preserve the strings as a re-enablement safety net, a short comment stating that would prevent a future contributor from being confused by why the test exists.Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "feat: hide fake streaming whitelist and ..." | Re-trigger Greptile