fix: increase group_tag varchar limit from 50 to 255#940
Conversation
📝 WalkthroughWalkthrough将 providers.group_tag 的最大长度从 50 扩展到 255，包含数据库迁移、Drizzle 模式与后端验证更新；同时调整构建草稿的分隔符（去除逗号后空格）、TagInput 去重为不区分大小写，并引入 UI 层与总长度检查间的不一致性。 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. Important Merge conflicts detected (Beta)
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical limitation where the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| case "request_timeout_non_streaming_ms": | ||
| return typeof value === "number" && Number.isFinite(value); | ||
| case "group_tag": | ||
| return typeof value === "string" && value.length <= 255; |
There was a problem hiding this comment.
set value for group_tag
The current validation only enforces a maximum length, but allows group_tag to be set to an empty string "" via the batch patch API. Through the UI this never happens (an empty tag array is sent as { clear: true } → stored as null), but a direct API call with { group_tag: { set: "" } } would store "" in the database instead of null.
The single-edit path handles this correctly with an explicit ternary in provider-form/index.tsx (line 364): group_tag: state.routing.groupTag.length > 0 ? ... : null. Consider adding a minimum length check to the batch contract to keep both paths consistent:
| return typeof value === "string" && value.length <= 255; | |
| case "group_tag": | |
| return typeof value === "string" && value.length > 0 && value.length <= 255; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/provider-patch-contract.ts
Line: 237
Comment:
**Empty string is a valid `set` value for `group_tag`**
The current validation only enforces a maximum length, but allows `group_tag` to be set to an empty string `""` via the batch patch API. Through the UI this never happens (an empty tag array is sent as `{ clear: true }` → stored as `null`), but a direct API call with `{ group_tag: { set: "" } }` would store `""` in the database instead of `null`.
The single-edit path handles this correctly with an explicit ternary in `provider-form/index.tsx` (line 364): `group_tag: state.routing.groupTag.length > 0 ? ... : null`. Consider adding a minimum length check to the batch contract to keep both paths consistent:
```suggestion
case "group_tag":
return typeof value === "string" && value.length > 0 && value.length <= 255;
```
How can I resolve this? If you propose a fix, please make it concise.![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 effectively increases the group_tag character limit from 50 to 255 across various parts of the application, including the database schema, validation, and frontend forms. The changes are consistent and address the issue described. My review includes a couple of suggestions to improve maintainability by extracting hardcoded values into constants. I've also noted that src/lib/validation/schemas.ts contains a large number of stylistic formatting changes that are out of scope for this fix; it would be better to handle such changes in a separate pull request to keep this one focused.
I am having trouble creating individual review comments. Click here to see my feedback.
src/app/[locale]/settings/providers/_components/forms/provider-form.legacy.tsx (893-899)
The magic number 50 is used here for the maximum length of a single tag. To improve readability and maintainability, it would be better to define this as a constant at the top of the file (e.g., const MAX_SINGLE_TAG_LENGTH = 50;) and use that constant in both maxTagLength and the too_long message.
src/app/[locale]/settings/providers/_components/forms/provider-form/sections/routing-section.tsx (161-167)
Similar to the legacy form, the magic number 50 is used here for the maximum length of a single tag. It would be best to define this as a constant to improve maintainability, for example const MAX_SINGLE_TAG_LENGTH = 50;, and use it here.
src/lib/validation/schemas.ts (24-46)
This file contains numerous formatting changes that seem out of scope for this PR's objective of increasing the group_tag limit. While consistent formatting is good, mixing it with functional changes makes the review harder and pollutes the commit history for this file. It's best practice to separate formatting/refactoring changes into their own PRs. Please consider reverting the stylistic changes in this file and applying them in a separate commit or pull request.
![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
This is a well-coordinated fix that increases the group_tag varchar limit from 50 to 255 across all layers of the application. The changes are consistent and properly propagated through the stack.
PR Size: S
- Lines changed: 4064 (4052 additions, 12 deletions)
- Files changed: 11
- Note: ~4000 lines are auto-generated Drizzle snapshot metadata; actual meaningful code changes are ~15 lines
Changes Verified
| Layer | File | Change | Status |
|---|---|---|---|
| DB Schema | src/drizzle/schema.ts |
varchar(50) -> varchar(255) |
Correct |
| Migration | drizzle/0083_young_killraven.sql |
ALTER COLUMN statement |
Correct |
| Validation | src/lib/validation/schemas.ts |
max(50) -> max(255) in Create + Update |
Correct |
| Patch Contract | src/lib/provider-patch-contract.ts |
Added value.length <= 255 check |
Correct |
| Frontend (legacy) | provider-form.legacy.tsx |
Constant + comment updated | Correct |
| Frontend (new) | routing-section.tsx |
Total limit 255, individual tag limit 50 | Correct |
| Batch Edit | build-patch-draft.ts |
join(", ") -> join(",") |
Correct |
| Test | build-patch-draft.test.ts |
Assertion updated | Correct |
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Observations
-
Proper layer coordination: The limit change is correctly propagated through DB schema, backend validation, batch patch contract, and frontend forms.
-
Smart separation of concerns: The PR correctly distinguishes between:
- Total serialized length (255 chars) - the comma-separated string stored in DB
- Individual tag length (50 chars) - each tag name's limit
-
Join format consistency: Fixed the
join(", ")vsjoin(",")inconsistency between batch and single edit paths. -
Comment accuracy: Updated the Chinese comment in
provider-form.legacy.tsxto reflect the new limit. -
Migration safety: The
ALTER TABLE ... SET DATA TYPE varchar(255)is metadata-only for PostgreSQL (increasing varchar length), no table rewrite required.
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 Claude AI
Multiple comma-joined tags easily exceeded the 50-char DB limit.
- Schema: varchar(50) -> varchar(255) with migration (0084)
- Zod: max(50) -> max(255) in Create/UpdateProviderSchema
- Frontend: GROUP_TAG_MAX_TOTAL_LENGTH 50 -> 255 (total serialized length)
- Frontend: maxTagLength kept at 50 (single tag name limit)
- Batch edit: join(", ") -> join(",") to match single-edit format
- Patch contract: add group_tag <= 255 length validation
- Test: update build-patch-draft assertion to match new join format
6a697ca to
b21fca3
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/unit/settings/providers/build-patch-draft.test.ts (1)
156-164:⚠️ Potential issue | 🟡 Minor建议补充
group_tag长度边界测试（255/256）。当前用例已覆盖分隔符变更，但建议再补两组断言：
length === 255通过、length === 256拒绝（走 patch contract / schema 校验），以锁住这次上限调整的核心行为。
As per coding guidelines, "**/*.test.{ts,tsx,js,jsx}: All new features must have unit test coverage of at least 80%".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/settings/providers/build-patch-draft.test.ts` around lines 156 - 164, Add two unit tests to tests/unit/settings/providers/build-patch-draft.test.ts around the existing case for buildPatchDraftFromFormState: one that sets state.routing.groupTag to a joined string of length exactly 255 and asserts draft.group_tag equals { set: "<255-char-string>" } (i.e., accepted), and another that sets state.routing.groupTag to produce a joined string of length 256 and asserts that buildPatchDraftFromFormState fails validation or produces a rejection according to the patch contract/schema (e.g., throws or returns an error/invalid draft). Use the same pattern as the existing test (mutate state.routing.groupTag and dirty = new Set(["routing.groupTag"])) and target the output symbol group_tag from buildPatchDraftFromFormState to lock the boundary behavior.
🧹 Nitpick comments (3)
src/lib/provider-patch-contract.ts (1)
236-237: 建议在契约层补齐“单个标签 <= 50”校验，避免绕过前端限制。现在只校验总长度 255，外部调用仍可写入超长单标签（例如单个 200 字符），会和前端规则不一致。建议在这里同步做分项校验。
可选改法（示例）
case "group_tag": - return typeof value === "string" && value.length <= 255; + if (typeof value !== "string" || value.length > 255) { + return false; + } + return value + .split(",") + .map((tag) => tag.trim()) + .filter(Boolean) + .every((tag) => tag.length <= 50);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/provider-patch-contract.ts` around lines 236 - 237, In the "group_tag" validation branch in provider-patch-contract.ts (the case "group_tag" clause) add per-tag length checks: ensure value is a string, its total length <= 255, then split the string by commas (trim each piece), reject if any individual tag is empty or longer than 50 characters; keep the overall total length check and return false if any of these checks fail. This aligns the contract with the front-end rule "single tag <= 50".src/app/[locale]/settings/providers/_components/forms/provider-form/sections/routing-section.tsx (2)
161-167: 实现正确，建议提取魔数常量。
maxTagLength={50}是单个标签的字符限制，与too_long错误消息中的{ max: 50 }一致。逻辑正确，但建议将50提取为命名常量（如MAX_GROUP_TAG_NAME_LENGTH），以便与GROUP_TAG_MAX_TOTAL_LENGTH形成清晰对比，并与provider-form.legacy.tsx保持同步。♻️ 可选重构建议
+const MAX_GROUP_TAG_NAME_LENGTH = 50; const GROUP_TAG_MAX_TOTAL_LENGTH = 255; // ... - maxTagLength={50} + maxTagLength={MAX_GROUP_TAG_NAME_LENGTH} suggestions={groupSuggestions} onInvalidTag={(_tag, reason) => { const messages: Record<string, string> = { empty: tUI("emptyTag"), duplicate: tUI("duplicateTag"), - too_long: tUI("tooLong", { max: 50 }), + too_long: tUI("tooLong", { max: MAX_GROUP_TAG_NAME_LENGTH }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/settings/providers/_components/forms/provider-form/sections/routing-section.tsx around lines 161 - 167, The magic number 50 is used in maxTagLength and the too_long message; extract it to a named constant (e.g., MAX_GROUP_TAG_NAME_LENGTH) and replace both uses in the component (references: maxTagLength prop, onInvalidTag messages object, too_long message interpolation) so the limit is centralized and clearly contrasted with GROUP_TAG_MAX_TOTAL_LENGTH; also ensure the same constant is used or synchronized with provider-form.legacy.tsx to keep behaviour consistent across files.
28-28: 常量定义正确，但存在跨文件重复。
GROUP_TAG_MAX_TOTAL_LENGTH = 255与数据库 schema 的 varchar(255) 限制一致。但此常量在provider-form.legacy.tsx中也有定义，建议提取到共享常量文件（如@/lib/constants/provider.constants.ts）以避免重复和不同步风险。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/settings/providers/_components/forms/provider-form/sections/routing-section.tsx at line 28, GROUP_TAG_MAX_TOTAL_LENGTH is defined in routing-section.tsx and duplicated in provider-form.legacy.tsx; extract this value into a single shared constants module (e.g., provider.constants) and export GROUP_TAG_MAX_TOTAL_LENGTH = 255 from there, then update routing-section.tsx and provider-form.legacy.tsx to import and use that exported constant and remove their local definitions to avoid duplication and drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@drizzle/0083_young_killraven.sql`:
- Line 1: Do not hand-edit the SQL migration file 0083_young_killraven.sql;
instead update the providers table definition in src/drizzle/schema.ts to change
the group_tag column to varchar(255) (or the equivalent Drizzle column type),
then run the project generator command bun run db:generate to produce the
correct migration and journal entries; finally remove the manually created
0083_young_killraven.sql so the repo only contains generated migrations
consistent with Drizzle metadata.
In `@src/lib/validation/schemas.ts`:
- Line 444: The hardcoded Chinese error message in the zod schema for group_tag
must be replaced with an i18n key; locate the zod rule using group_tag:
z.string().max(...).nullable().optional() (and the similar occurrence later) and
change the second argument (the user-facing error message) to use the i18n
translation key (for example via the project i18n helper like
t('validation.group_tag.max_length') or a shared validation key) so that all
user-facing strings are resolved through i18n for supported locales; ensure both
occurrences (the group_tag max message and the duplicate instance in the file)
are updated to use the same i18n key/format.
---
Outside diff comments:
In `@tests/unit/settings/providers/build-patch-draft.test.ts`:
- Around line 156-164: Add two unit tests to
tests/unit/settings/providers/build-patch-draft.test.ts around the existing case
for buildPatchDraftFromFormState: one that sets state.routing.groupTag to a
joined string of length exactly 255 and asserts draft.group_tag equals { set:
"<255-char-string>" } (i.e., accepted), and another that sets
state.routing.groupTag to produce a joined string of length 256 and asserts that
buildPatchDraftFromFormState fails validation or produces a rejection according
to the patch contract/schema (e.g., throws or returns an error/invalid draft).
Use the same pattern as the existing test (mutate state.routing.groupTag and
dirty = new Set(["routing.groupTag"])) and target the output symbol group_tag
from buildPatchDraftFromFormState to lock the boundary behavior.
---
Nitpick comments:
In
`@src/app/`[locale]/settings/providers/_components/forms/provider-form/sections/routing-section.tsx:
- Around line 161-167: The magic number 50 is used in maxTagLength and the
too_long message; extract it to a named constant (e.g.,
MAX_GROUP_TAG_NAME_LENGTH) and replace both uses in the component (references:
maxTagLength prop, onInvalidTag messages object, too_long message interpolation)
so the limit is centralized and clearly contrasted with
GROUP_TAG_MAX_TOTAL_LENGTH; also ensure the same constant is used or
synchronized with provider-form.legacy.tsx to keep behaviour consistent across
files.
- Line 28: GROUP_TAG_MAX_TOTAL_LENGTH is defined in routing-section.tsx and
duplicated in provider-form.legacy.tsx; extract this value into a single shared
constants module (e.g., provider.constants) and export
GROUP_TAG_MAX_TOTAL_LENGTH = 255 from there, then update routing-section.tsx and
provider-form.legacy.tsx to import and use that exported constant and remove
their local definitions to avoid duplication and drift.
In `@src/lib/provider-patch-contract.ts`:
- Around line 236-237: In the "group_tag" validation branch in
provider-patch-contract.ts (the case "group_tag" clause) add per-tag length
checks: ensure value is a string, its total length <= 255, then split the string
by commas (trim each piece), reject if any individual tag is empty or longer
than 50 characters; keep the overall total length check and return false if any
of these checks fail. This aligns the contract with the front-end rule "single
tag <= 50".
🪄 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: f0126662-6a88-4489-ac94-67d1570aedf5
📒 Files selected for processing (11)
CHANGELOG.mddrizzle/0083_young_killraven.sqldrizzle/meta/0083_snapshot.jsondrizzle/meta/_journal.jsonsrc/app/[locale]/settings/providers/_components/batch-edit/build-patch-draft.tssrc/app/[locale]/settings/providers/_components/forms/provider-form.legacy.tsxsrc/app/[locale]/settings/providers/_components/forms/provider-form/sections/routing-section.tsxsrc/drizzle/schema.tssrc/lib/provider-patch-contract.tssrc/lib/validation/schemas.tstests/unit/settings/providers/build-patch-draft.test.ts
| @@ -0,0 +1 @@ | |||
| ALTER TABLE "providers" ALTER COLUMN "group_tag" SET DATA TYPE varchar(255); | |||
There was a problem hiding this comment.
请不要手写 Drizzle 迁移文件。
这条 SQL 本身可执行，但当前方式违背仓库迁移流程，后续容易出现 schema 与迁移元数据漂移。建议保留 src/drizzle/schema.ts 的变更后，重新执行 bun run db:generate 生成迁移与 journal 记录。
As per coding guidelines, "Modify database schema in src/drizzle/schema.ts, then run bun run db:generate to generate migrations. Never create SQL migration files manually".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@drizzle/0083_young_killraven.sql` at line 1, Do not hand-edit the SQL
migration file 0083_young_killraven.sql; instead update the providers table
definition in src/drizzle/schema.ts to change the group_tag column to
varchar(255) (or the equivalent Drizzle column type), then run the project
generator command bun run db:generate to produce the correct migration and
journal entries; finally remove the manually created 0083_young_killraven.sql so
the repo only contains generated migrations consistent with Drizzle metadata.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/app/[locale]/settings/providers/_components/forms/provider-form.legacy.tsx (1)
893-893: 建议提取单标签长度常量，避免后续漂移。Line 893 和 Line 899 里
50出现了重复字面量，建议抽成统一常量（例如GROUP_TAG_MAX_SINGLE_LENGTH），减少未来修改遗漏。可选重构示例
const GROUP_TAG_MAX_TOTAL_LENGTH = 255; +const GROUP_TAG_MAX_SINGLE_LENGTH = 50; @@ - maxTagLength={50} + maxTagLength={GROUP_TAG_MAX_SINGLE_LENGTH} @@ - too_long: tUI("tooLong", { max: 50 }), + too_long: tUI("tooLong", { max: GROUP_TAG_MAX_SINGLE_LENGTH }),Also applies to: 899-899
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/settings/providers/_components/forms/provider-form.legacy.tsx at line 893, Extract the repeated literal 50 used for the single tag length into a named constant (e.g., GROUP_TAG_MAX_SINGLE_LENGTH) and replace both occurrences of maxTagLength={50} in provider-form.legacy.tsx with maxTagLength={GROUP_TAG_MAX_SINGLE_LENGTH}; declare the constant near the top of the module (or next to other form constants) and export it if needed for reuse so future changes only require updating the single constant instead of multiple literals.
🤖 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/lib/validation/schemas.ts`:
- Line 444: The current zod field group_tag (in src/lib/validation/schemas.ts)
only enforces total length ≤255 and allows bypassing the business rule that each
individual tag must be ≤50 characters; update the schema to add a per-tag
validation (e.g., a .refine or custom validator on group_tag) that splits the
string into tags (by the same delimiter used by the app), trims each tag, and
returns false if any tag.length > 50, providing a clear error message; extract
this enhanced rule into a reusable schema/constant (e.g., groupTagSchema or
similar) and replace both occurrences of group_tag so the same single-tag
constraint is applied consistently (including the instances currently at the two
locations you found).
---
Nitpick comments:
In
`@src/app/`[locale]/settings/providers/_components/forms/provider-form.legacy.tsx:
- Line 893: Extract the repeated literal 50 used for the single tag length into
a named constant (e.g., GROUP_TAG_MAX_SINGLE_LENGTH) and replace both
occurrences of maxTagLength={50} in provider-form.legacy.tsx with
maxTagLength={GROUP_TAG_MAX_SINGLE_LENGTH}; declare the constant near the top of
the module (or next to other form constants) and export it if needed for reuse
so future changes only require updating the single constant instead of multiple
literals.
🪄 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: 67a929d9-0f52-4d89-8016-e68ebb8fce4c
📒 Files selected for processing (10)
drizzle/0084_needy_jackpot.sqldrizzle/meta/0084_snapshot.jsondrizzle/meta/_journal.jsonsrc/app/[locale]/settings/providers/_components/batch-edit/build-patch-draft.tssrc/app/[locale]/settings/providers/_components/forms/provider-form.legacy.tsxsrc/app/[locale]/settings/providers/_components/forms/provider-form/sections/routing-section.tsxsrc/drizzle/schema.tssrc/lib/provider-patch-contract.tssrc/lib/validation/schemas.tstests/unit/settings/providers/build-patch-draft.test.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- src/app/[locale]/settings/providers/_components/batch-edit/build-patch-draft.ts
- drizzle/meta/_journal.json
- src/drizzle/schema.ts
- src/app/[locale]/settings/providers/_components/forms/provider-form/sections/routing-section.tsx
- src/lib/provider-patch-contract.ts
| .default(null), | ||
| cost_multiplier: z.coerce.number().min(0, "成本倍率不能为负数").optional().default(1.0), | ||
| group_tag: z.string().max(50, "分组标签不能超过50个字符").nullable().optional(), | ||
| group_tag: z.string().max(255, "分组标签不能超过255个字符").nullable().optional(), |
There was a problem hiding this comment.
后端缺少“单个标签长度 ≤ 50”的强校验，规则可被绕过。
Line 444 和 Line 682 仅校验整串 group_tag 的总长度 <= 255。这会允许通过 API 直接写入超长单个标签（例如 120 字符），与“单标签上限 50”的业务约束不一致。
建议修复（统一复用 schema，增加单标签约束）
+const GROUP_TAG_MAX_TOTAL_LENGTH = 255;
+const GROUP_TAG_MAX_SINGLE_LENGTH = 50;
+
+const GROUP_TAG_SCHEMA = z
+ .string()
+ .max(GROUP_TAG_MAX_TOTAL_LENGTH, "分组标签不能超过255个字符")
+ .refine(
+ (value) =>
+ value
+ .split(",")
+ .every((tag) => {
+ const trimmed = tag.trim();
+ return trimmed.length > 0 && trimmed.length <= GROUP_TAG_MAX_SINGLE_LENGTH;
+ }),
+ "单个分组标签长度不能超过50个字符"
+ )
+ .nullable()
+ .optional();
+
export const CreateProviderSchema = z
.object({
@@
- group_tag: z.string().max(255, "分组标签不能超过255个字符").nullable().optional(),
+ group_tag: GROUP_TAG_SCHEMA,
@@
export const UpdateProviderSchema = z
.object({
@@
- group_tag: z.string().max(255, "分组标签不能超过255个字符").nullable().optional(),
+ group_tag: GROUP_TAG_SCHEMA,Also applies to: 682-682
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/validation/schemas.ts` at line 444, The current zod field group_tag
(in src/lib/validation/schemas.ts) only enforces total length ≤255 and allows
bypassing the business rule that each individual tag must be ≤50 characters;
update the schema to add a per-tag validation (e.g., a .refine or custom
validator on group_tag) that splits the string into tags (by the same delimiter
used by the app), trims each tag, and returns false if any tag.length > 50,
providing a clear error message; extract this enhanced rule into a reusable
schema/constant (e.g., groupTagSchema or similar) and replace both occurrences
of group_tag so the same single-tag constraint is applied consistently
(including the instances currently at the two locations you found).
Prevents adding tags that differ only in casing (e.g. 'openapiha' vs 'openapiHA'), which would cause confusion since the backend group matching uses exact string comparison.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/components/ui/tag-input.tsx`:
- Around line 209-212: filteredSuggestions filtering is case-sensitive while
duplicate detection in the add flow uses toLowerCase(), causing suggestions like
"Test" to show after "test" was added then be rejected; update the
filteredSuggestions computation (the symbol filteredSuggestions) to use the same
case-insensitive membership check as the duplicate check (compare lowercased
candidate s.value against existing currentTags and/or the typed value) so
suggestions that are case-insensitive duplicates are excluded, keeping
onInvalidTag, allowDuplicates and currentTags logic unchanged.
🪄 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: 3114f1c7-7de9-4398-900d-d6f44d2322c5
📒 Files selected for processing (1)
src/components/ui/tag-input.tsx
| if (!allowDuplicates && currentTags.some((t) => t.toLowerCase() === tag.toLowerCase())) { | ||
| onInvalidTag?.(tag, "duplicate"); | ||
| return false; | ||
| } |
There was a problem hiding this comment.
大小写不敏感的重复检测与建议列表过滤不一致
此处改为 toLowerCase() 比较，逻辑正确。但第 192 行的 filteredSuggestions 仍使用 !value.includes(s.value) 进行大小写敏感过滤。
这会导致用户体验问题：当已添加 "test" 时，建议列表仍会显示 "Test"，但用户选择后会被此处拒绝为重复项。
建议同步修改第 192 行的过滤逻辑：
建议修复
const filteredSuggestions = React.useMemo(() => {
if (!normalizedSuggestions.length) return [];
const search = inputValue.toLowerCase();
+ const lowerCaseTags = value.map((v) => v.toLowerCase());
return normalizedSuggestions.filter((s) => {
const keywords = s.keywords?.join(" ") || "";
const haystack = `${s.label} ${s.value} ${keywords}`.toLowerCase();
- return haystack.includes(search) && (allowDuplicates || !value.includes(s.value));
+ return haystack.includes(search) && (allowDuplicates || !lowerCaseTags.includes(s.value.toLowerCase()));
});
}, [normalizedSuggestions, inputValue, value, allowDuplicates]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ui/tag-input.tsx` around lines 209 - 212, filteredSuggestions
filtering is case-sensitive while duplicate detection in the add flow uses
toLowerCase(), causing suggestions like "Test" to show after "test" was added
then be rejected; update the filteredSuggestions computation (the symbol
filteredSuggestions) to use the same case-insensitive membership check as the
duplicate check (compare lowercased candidate s.value against existing
currentTags and/or the typed value) so suggestions that are case-insensitive
duplicates are excluded, keeping onInvalidTag, allowDuplicates and currentTags
logic unchanged.
github-project-automation
Bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
|
整体思路我觉得是对的，
顺手补充下我这边的验证：我在临时 worktree 上跑了 |
The suggestion dropdown still showed tags that only differed in casing from already-selected tags (e.g. showing 'openapiHA' when 'openapiha' was already selected). Selecting such a suggestion would then fail as a duplicate, creating a confusing UX. Follows up on the case-insensitive duplicate check added in ding113#940.
The suggestion dropdown still showed tags that only differed in casing from already-selected tags (e.g. showing 'openapiHA' when 'openapiha' was already selected). Selecting such a suggestion would then fail as a duplicate, creating a confusing UX. Follows up on the case-insensitive duplicate check added in #940.
3 participants
Summary
group_tagstores comma-separated tags in a singlevarcharcolumn. Users with 6+ tags easily exceeded the 50-char limit.varchar(50)tovarchar(255)across DB schema, Zod validation, frontend forms, and batch edit paths.join(", ")vsjoin(",")inconsistency between batch and single edit.group_tag <= 255validation in batch patch contract layer.maxTagLength/MAX_GROUP_NAME_LENGTH).Changes
src/drizzle/schema.tsvarchar(50)->varchar(255)drizzle/0083_young_killraven.sqlALTER COLUMN SET DATA TYPE varchar(255)src/lib/validation/schemas.tsmax(50)->max(255)(Create + Update)src/lib/provider-patch-contract.tsvalue.length <= 255forgroup_tagprovider-form.legacy.tsxGROUP_TAG_MAX_TOTAL_LENGTH = 255,maxTagLength = 50routing-section.tsxGROUP_TAG_MAX_TOTAL_LENGTH = 255,maxTagLength = 50build-patch-draft.tsjoin(", ")->join(",")build-patch-draft.test.tsMigration Safety
The
ALTER TABLEonly changes PostgreSQL catalog metadata (increasing varchar length) - no table rewrite, no lock, millisecond execution regardless of table size.Greptile Summary
This PR increases the
group_tagcolumn limit fromvarchar(50)tovarchar(255)to support providers with 6 or more tags, and consistently propagates that change across the DB schema, migration, Zod validation, batch patch contract, and both frontend form implementations. It also fixes a pre-existing inconsistency where the batch edit path joined tags with", "(space-padded) while the single-edit path used",", which could silently introduce leading-space artifacts in tags on round-trip.Key changes:
0084_needy_jackpot.sql) widens the column — a metadata-only operation in PostgreSQL with no table rewrite.CreateProviderSchemaandUpdateProviderSchemanow enforcemax(255)ongroup_tag.provider-patch-contract.ts) adds avalue.length <= 255guard.GROUP_TAG_MAX_TOTAL_LENGTH = 255while keeping individual tag length capped atmaxTagLength={50}.join(",")fix inbuild-patch-draft.tsensures all paths produce identically-formatted stored values.Confidence Score: 5/5
join(",")fix resolves a real inconsistency without any behavioral regression. The test is updated to match. No new logic paths are introduced.Important Files Changed
group_tagfromvarchar(50)tovarchar(255). In PostgreSQL, increasing a varchar length is a metadata-only operation — no table rewrite or lock.group_tagZod validation frommax(50)tomax(255)in bothCreateProviderSchemaandUpdateProviderSchema, aligned with the new DB column size.value.length <= 255check forgroup_tagin the batch patch contract. An empty-stringsetvalue is technically still accepted (length 0 passes the check), which was already flagged in a previous review thread.join(",")(no space) when building the batch patch draft — previously usedjoin(", "), which diverged from the single-edit path and could produce leading spaces when tags were reloaded.GROUP_TAG_MAX_TOTAL_LENGTHto 255 and validates total serialized length in both the onChange handler and the submit handler; per-tag limit correctly kept atmaxTagLength={50}.GROUP_TAG_MAX_TOTAL_LENGTH = 255with per-tagmaxTagLength={50}unchanged."tagA, tagB"to"tagA,tagB"matching the correctedjoin(",")format.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A["User adds/removes a tag in TagInput\n(maxTagLength = 50 per tag)"] --> B["handleGroupTagChange\ntags.join(',').length > 255?"] B -- "Yes" --> C["toast.error — reject change"] B -- "No" --> D["Update form state\n(groupTag: string[])"] D --> E{Submit path} E -- "Single edit\n(provider-form)" --> F["group_tag = tags.join(',') || null\nPOST/PATCH API"] E -- "Batch edit\n(build-patch-draft)" --> G["joined = tags.join(',')\nif empty → clear, else set"] F --> H["UpdateProviderSchema\nZod max(255)"] G --> I["normalizeProviderBatchPatchDraft\nvalue.length <= 255"] H --> J[("DB: providers.group_tag\nvarchar(255)")] I --> JLast reviewed commit: "chore: format code (..."