Conversation
📝 WalkthroughWalkthrough此 PR 将硬编码的规则数与文本长度限制替换为导出的 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request replaces hardcoded limits for provider allowlist and redirect rules with dynamic constants, increasing the maximum allowed items to 100,000 and the text length to 4,096 characters across localization files, UI components, and validation schemas. While the change provides more flexibility, a significant performance risk was identified: rendering and validating 100,000 complex React components or large JSON payloads could lead to browser crashes, API timeouts, or DoS vulnerabilities. It is recommended to implement UI virtualization and more efficient validation if such high limits are truly necessary.
|
|
||
| export const PROVIDER_RULE_LIMITS = { | ||
| // 模型白名单 / 重定向规则保存在 JSONB 中，这里统一放宽到支持大规模路由表 | ||
| MAX_ITEMS: 100_000, |
There was a problem hiding this comment.
Increasing MAX_ITEMS to 100,000 entries introduces significant performance and stability risks with the current implementation:
- UI Performance: The
AllowedModelRuleEditorandModelRedirectEditorcomponents render the entire list using.map(). Rendering 100,000 complex React components (with inputs, selects, and buttons) will likely cause the browser to hang or crash. UI virtualization (e.g.,react-window) is necessary for lists of this size. - Validation Overhead: Zod validation of 100,000 items, especially with
superRefinecallingsafe-regexon each entry, will block the Node.js event loop, potentially leading to API timeouts or service unavailability (DoS risk). - Payload Size: A 100k-entry JSON object can be several megabytes, which may exceed default request body limits in proxies or application servers.
Consider if such a high limit is truly required, or implement the necessary optimizations (virtualization, efficient validation) alongside this change.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const MAX_PROVIDER_RULES = 100_000; | ||
| const MAX_PROVIDER_RULE_TEXT_LENGTH = 4_096; |
There was a problem hiding this comment.
MAX_PROVIDER_RULES and MAX_PROVIDER_RULE_TEXT_LENGTH duplicate the values from PROVIDER_RULE_LIMITS rather than importing them. If the constants are updated in the future the tests will silently continue asserting the old values, undermining their regression value.
| const MAX_PROVIDER_RULES = 100_000; | |
| const MAX_PROVIDER_RULE_TEXT_LENGTH = 4_096; | |
| import { PROVIDER_RULE_LIMITS } from "@/lib/constants/provider.constants"; | |
| const MAX_PROVIDER_RULES = PROVIDER_RULE_LIMITS.MAX_ITEMS; | |
| const MAX_PROVIDER_RULE_TEXT_LENGTH = PROVIDER_RULE_LIMITS.MAX_TEXT_LENGTH; |
The same pattern applies in tests/unit/lib/provider-model-redirect-schema.test.ts lines 7–8.
Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/unit/lib/provider-allowed-model-schema.test.ts
Line: 7-8
Comment:
**Test limits not imported from source constant**
`MAX_PROVIDER_RULES` and `MAX_PROVIDER_RULE_TEXT_LENGTH` duplicate the values from `PROVIDER_RULE_LIMITS` rather than importing them. If the constants are updated in the future the tests will silently continue asserting the old values, undermining their regression value.
```suggestion
import { PROVIDER_RULE_LIMITS } from "@/lib/constants/provider.constants";
const MAX_PROVIDER_RULES = PROVIDER_RULE_LIMITS.MAX_ITEMS;
const MAX_PROVIDER_RULE_TEXT_LENGTH = PROVIDER_RULE_LIMITS.MAX_TEXT_LENGTH;
```
The same pattern applies in `tests/unit/lib/provider-model-redirect-schema.test.ts` lines 7–8.
How can I resolve this? If you propose a fix, please make it concise.| describe("provider-allowed-model-schema", () => { | ||
| it("accepts match patterns longer than 255 characters when still within the new cap", () => { | ||
| const result = PROVIDER_ALLOWED_MODEL_RULE_SCHEMA.safeParse({ | ||
| matchType: "exact", | ||
| pattern: "m".repeat(MAX_PROVIDER_RULE_TEXT_LENGTH), | ||
| }); | ||
|
|
||
| expect(result.success).toBe(true); | ||
| }); |
There was a problem hiding this comment.
patternTooLong
The test suite verifies acceptance at exactly MAX_PROVIDER_RULE_TEXT_LENGTH characters but never asserts that MAX_PROVIDER_RULE_TEXT_LENGTH + 1 is rejected. Without the negative case there is no guard against someone accidentally removing or relaxing the .max() constraint on the pattern field. Consider adding:
it("rejects match patterns longer than the cap", () => {
const result = PROVIDER_ALLOWED_MODEL_RULE_SCHEMA.safeParse({
matchType: "exact",
pattern: "m".repeat(MAX_PROVIDER_RULE_TEXT_LENGTH + 1),
});
expect(result.success).toBe(false);
});The same gap exists in tests/unit/lib/provider-model-redirect-schema.test.ts for the source / target fields.
Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/unit/lib/provider-allowed-model-schema.test.ts
Line: 17-25
Comment:
**Missing rejection test for `patternTooLong`**
The test suite verifies acceptance at exactly `MAX_PROVIDER_RULE_TEXT_LENGTH` characters but never asserts that `MAX_PROVIDER_RULE_TEXT_LENGTH + 1` is rejected. Without the negative case there is no guard against someone accidentally removing or relaxing the `.max()` constraint on the `pattern` field. Consider adding:
```ts
it("rejects match patterns longer than the cap", () => {
const result = PROVIDER_ALLOWED_MODEL_RULE_SCHEMA.safeParse({
matchType: "exact",
pattern: "m".repeat(MAX_PROVIDER_RULE_TEXT_LENGTH + 1),
});
expect(result.success).toBe(false);
});
```
The same gap exists in `tests/unit/lib/provider-model-redirect-schema.test.ts` for the `source` / `target` fields.
How can I resolve this? If you propose a fix, please make it concise.
github-actions
Bot
added
bug
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/unit/lib/provider-model-redirect-schema.test.ts (1)
7-8: 建议测试直接复用PROVIDER_RULE_LIMITS，避免常量漂移。当前测试把 100000/4096 再写一遍，后续若调整限制，测试可能“意外通过但语义过时”。建议从
@/lib/constants/provider.constants直接导入。可参考改法
+import { PROVIDER_RULE_LIMITS } from "@/lib/constants/provider.constants"; import { describe, expect, it } from "vitest"; import { PROVIDER_MODEL_REDIRECT_RULE_LIST_SCHEMA, PROVIDER_MODEL_REDIRECT_RULE_SCHEMA, } from "@/lib/provider-model-redirect-schema"; -const MAX_PROVIDER_RULES = 100_000; -const MAX_PROVIDER_RULE_TEXT_LENGTH = 4_096; +const { MAX_ITEMS: MAX_PROVIDER_RULES, MAX_TEXT_LENGTH: MAX_PROVIDER_RULE_TEXT_LENGTH } = + PROVIDER_RULE_LIMITS;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/lib/provider-model-redirect-schema.test.ts` around lines 7 - 8, The test redefines numeric limits (MAX_PROVIDER_RULES, MAX_PROVIDER_RULE_TEXT_LENGTH) which can drift from the real constants; instead import and use the canonical PROVIDER_RULE_LIMITS from provider.constants (e.g., use PROVIDER_RULE_LIMITS.maxRules and PROVIDER_RULE_LIMITS.maxRuleTextLength) in the test to ensure it always reflects the source-of-truth; replace occurrences of MAX_PROVIDER_RULES and MAX_PROVIDER_RULE_TEXT_LENGTH in the test with the corresponding fields from PROVIDER_RULE_LIMITS and remove the duplicated constants.
🤖 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/model-redirect-editor.tsx:
- Around line 129-130: The current check against PROVIDER_RULE_LIMITS.MAX_ITEMS
and the full rendering of redirects in model-redirect-editor.tsx will not scale
to 100000 items; refactor the component to use a virtualized list (e.g.,
react-window/react-virtualized) or server-side pagination for the rendering
layer (replace full-map rendering of redirects) and add a cached Set of rule
keys used by validation logic (instead of scanning redirects linearly) so
methods that call setError / validate new entries consult the cached keys;
ensure functions/components like the list renderer and the validation helper
update the cache incrementally on add/edit/delete to avoid repeated O(n) scans.
---
Nitpick comments:
In `@tests/unit/lib/provider-model-redirect-schema.test.ts`:
- Around line 7-8: The test redefines numeric limits (MAX_PROVIDER_RULES,
MAX_PROVIDER_RULE_TEXT_LENGTH) which can drift from the real constants; instead
import and use the canonical PROVIDER_RULE_LIMITS from provider.constants (e.g.,
use PROVIDER_RULE_LIMITS.maxRules and PROVIDER_RULE_LIMITS.maxRuleTextLength) in
the test to ensure it always reflects the source-of-truth; replace occurrences
of MAX_PROVIDER_RULES and MAX_PROVIDER_RULE_TEXT_LENGTH in the test with the
corresponding fields from PROVIDER_RULE_LIMITS and remove the duplicated
constants.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f97dbe47-2aca-4714-bddb-ed8b5b3ff9a7
📒 Files selected for processing (19)
messages/en/settings/providers/form/allowedModelRules.jsonmessages/en/settings/providers/form/modelRedirect.jsonmessages/ja/settings/providers/form/allowedModelRules.jsonmessages/ja/settings/providers/form/modelRedirect.jsonmessages/ru/settings/providers/form/allowedModelRules.jsonmessages/ru/settings/providers/form/modelRedirect.jsonmessages/zh-CN/settings/providers/form/allowedModelRules.jsonmessages/zh-CN/settings/providers/form/modelRedirect.jsonmessages/zh-TW/settings/providers/form/allowedModelRules.jsonmessages/zh-TW/settings/providers/form/modelRedirect.jsonsrc/app/[locale]/settings/providers/_components/allowed-model-rule-editor.tsxsrc/app/[locale]/settings/providers/_components/model-redirect-editor.tsxsrc/lib/constants/provider.constants.tssrc/lib/provider-allowed-model-schema.tssrc/lib/provider-model-redirect-schema.tstests/unit/lib/provider-allowed-model-schema.test.tstests/unit/lib/provider-model-redirect-schema.test.tstests/unit/settings/providers/allowed-model-rule-editor.test.tsxtests/unit/settings/providers/model-redirect-editor.test.tsx
| if (redirects.length >= PROVIDER_RULE_LIMITS.MAX_ITEMS) { | ||
| setError(t("maxRules", { max: PROVIDER_RULE_LIMITS.MAX_ITEMS })); |
There was a problem hiding this comment.
将前端可编辑上限放宽到 100000 会触发大列表性能瓶颈。
当前组件仍是全量渲染 + 多处线性操作；当规则规模接近新上限时，页面会出现明显卡顿甚至阻塞。建议至少补充分页/虚拟列表（渲染层）并缓存规则键集合（校验层），否则该上限在 UI 上不可用。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/`[locale]/settings/providers/_components/model-redirect-editor.tsx
around lines 129 - 130, The current check against PROVIDER_RULE_LIMITS.MAX_ITEMS
and the full rendering of redirects in model-redirect-editor.tsx will not scale
to 100000 items; refactor the component to use a virtualized list (e.g.,
react-window/react-virtualized) or server-side pagination for the rendering
layer (replace full-map rendering of redirects) and add a cached Set of rule
keys used by validation logic (instead of scanning redirects linearly) so
methods that call setError / validate new entries consult the cached keys;
ensure functions/components like the list renderer and the validation helper
update the cache incrementally on add/edit/delete to avoid repeated O(n) scans.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4e46e673e9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| export const PROVIDER_RULE_LIMITS = { | ||
| // 模型白名单 / 重定向规则保存在 JSONB 中，这里统一放宽到支持大规模路由表 | ||
| MAX_ITEMS: 100_000, |
There was a problem hiding this comment.
Raising MAX_ITEMS to 100,000 introduces a hot-path performance regression because request routing still does linear rule scans (matchesAllowedModelRules in provider selection and findMatchingProviderModelRedirectRule in redirect application), and regex rules are evaluated via matchesPattern without precompiled caches. With this cap, a single request can trigger huge per-request CPU work when providers carry large rule sets, which can noticeably degrade latency or cause timeouts under load; the higher cap should only ship with indexed/compiled matching (or a lower safe limit).
Useful? React with 👍 / 👎.
| pattern: "", | ||
| }; | ||
| const MAX_ALLOWED_MODEL_RULES = 100; | ||
| const MAX_ALLOWED_MODEL_RULES = PROVIDER_RULE_LIMITS.MAX_ITEMS; |
There was a problem hiding this comment.
The editors now permit 100,000 entries, but the settings UIs still render full rule arrays in normal React lists (no virtualization/pagination), so loading a provider with a very large routing table can lock up the browser before the user can edit anything. This makes the new limit practically unusable in the admin UI and should be paired with virtualized list rendering or a smaller UI-facing cap.
Useful? React with 👍 / 👎.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Summary
No significant issues identified in this PR.
PR Size: M
- Lines changed: 318
- Files changed: 19
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Clean
- Type safety - Clean
- Documentation accuracy - Clean
- Test coverage - Adequate
- Code clarity - Good
Automated review by Codex AI
There was a problem hiding this comment.
- Reviewed PR
#1000(fix: relax provider routing rule limits) across the 19 changed files and applied thesize/Mlabel. - No diff-scoped issues met the reporting threshold, so I did not post any inline review comments.
- Submitted the PR-level review summary as a
COMMENTEDreview viagh pr review.
github-project-automation
Bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
1 participant
Summary
Raise provider model redirect and allowlist rule caps from 100 to 100,000 entries, increase per-rule text length cap from 255 to 4,096 characters, and centralize both limits through a shared
PROVIDER_RULE_LIMITSconstant. Updates editor-level guards, server-side Zod schemas, i18n copy across all 5 languages, and adds regression tests for the relaxed limits.Problem
The previous hard-coded limits of 100 rules and 255-character text fields were too restrictive for large-scale routing tables (e.g., organizations with many model variants or complex regex-based routing patterns). Both the frontend editors and backend schemas enforced these low caps independently with duplicated magic numbers.
Solution
PROVIDER_RULE_LIMITSconstant insrc/lib/constants/provider.constants.tswithMAX_ITEMS: 100_000andMAX_TEXT_LENGTH: 4_096allowed-model-rule-editor.tsx,model-redirect-editor.tsx) to reference the shared constant instead of inline100/255valuesprovider-allowed-model-schema.ts,provider-model-redirect-schema.ts) to use the same constant{max}interpolation so limit values stay in sync automaticallyChanges
Core Changes
src/lib/constants/provider.constants.ts-- AddedPROVIDER_RULE_LIMITSconstant (MAX_ITEMS: 100_000,MAX_TEXT_LENGTH: 4_096)src/lib/provider-allowed-model-schema.ts-- Updated.max()calls on both rule array and pattern string to use the shared constantsrc/lib/provider-model-redirect-schema.ts-- Updated.max()calls on rule array, source, and target strings to use the shared constantsrc/app/[locale]/settings/providers/_components/allowed-model-rule-editor.tsx-- Replaced inline100/255withPROVIDER_RULE_LIMITS; passes{max}to i18n messagessrc/app/[locale]/settings/providers/_components/model-redirect-editor.tsx-- Same treatment for redirect editori18n Updates
maxRules,patternTooLong,sourceTooLong,targetTooLong, andquickAddReachedLimitmessages in all 5 languages to use{max}interpolation instead of hard-coded numbersTests Added
tests/unit/lib/provider-allowed-model-schema.test.ts(new) -- Verifies patterns up to 4,096 chars are accepted; validates 100,000-entry list passes and 100,001-entry list is rejectedtests/unit/lib/provider-model-redirect-schema.test.ts(new) -- Same boundary tests for redirect rulestests/unit/settings/providers/allowed-model-rule-editor.test.tsx-- New test proving 100+ rules no longer block the add buttontests/unit/settings/providers/model-redirect-editor.test.tsx-- New test proving 100+ redirect rules can still be added; updated existing length validation test to reflect the new 4,097-char boundaryBreaking Changes
None. The limits are only relaxed (widened), so all previously valid configurations remain valid. No exports were removed or renamed.
Testing
Automated Tests
Verification Commands
Notes
bun run testcurrently has existing suite-level failures on both this branch and untouched main, includingtests/unit/actions/providers-undo-engine.test.tsandtests/unit/lib/provider-patch-contract.test.tswhen run in the full suite. Those files pass in isolation on both trees, so this PR does not include a fix for that baseline issue.Checklist
Description enhanced by Claude AI
Greptile Summary
Raises provider routing rule caps from 100 → 100,000 entries and 255 → 4,096 characters by introducing a shared
PROVIDER_RULE_LIMITSconstant, then threading it through both Zod schemas, both editor components, and all 5 i18n locales. The approach is clean and internally consistent — the previous concerns about duplicated test constants and missing{max}interpolation are fully resolved in this version.Confidence Score: 5/5
Safe to merge — all remaining findings are P2 suggestions with no correctness or data-integrity risk on the changed paths.
The constant centralization, schema updates, i18n changes, and tests are all well-executed. The only open finding is a P2 UX-performance note about the absence of list virtualization in the editors, which is a pre-existing architectural pattern rather than a regression introduced by this PR. No P0 or P1 issues were found.
No files require special attention for merge readiness. The editor components warrant a follow-up for list virtualization if large rule sets become common in practice.
Vulnerabilities
No security concerns identified. Regex inputs continue to be guarded by
safeRegex(ReDoS protection) and schema-levelsuperRefineon both editor and server-side paths. Relaxing the length cap to 4,096 characters does not introduce new injection vectors.Important Files Changed
PROVIDER_RULE_LIMITSconstant correctly centralizesMAX_ITEMS: 100_000andMAX_TEXT_LENGTH: 4_096for both schema and editor use..max()calls updated to usePROVIDER_RULE_LIMITS; consistent acrossINPUT_LIST_SCHEMAandLIST_SCHEMAvariants..max()and list.max()all updated to usePROVIDER_RULE_LIMITS; no regressions introduced.PROVIDER_RULE_LIMITSand passes{max}to i18n; rule list rendering is unbounded and could freeze browsers at very high counts.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[PROVIDER_RULE_LIMITS\nMAX_ITEMS 100000\nMAX_TEXT_LENGTH 4096] --> B[provider-allowed-model-schema.ts\n.max on pattern + list] A --> C[provider-model-redirect-schema.ts\n.max on source, target + list] A --> D[allowed-model-rule-editor.tsx\nUI guard + i18n interpolation] A --> E[model-redirect-editor.tsx\nUI guard + i18n interpolation] B --> F[(JSONB: allowed_models\nPostgreSQL)] C --> G[(JSONB: model_redirects\nPostgreSQL)] D --> H[i18n messages\nzh-CN / zh-TW / en / ja / ru] E --> HPrompt To Fix All With AI
Reviews (2): Last reviewed commit: "test: reuse provider rule limit constant..." | Re-trigger Greptile