fix: preserve extra pricing fields and expose semantic pricing details#1049
Conversation
📝 WalkthroughWalkthrough新增“高级字段”本地化条目、前后端对额外 JSON 字段的采集/校验/合并、详情对话框与抽屉编辑支持，扩展价格字段收集/验证工具并补充相应单元与集成测试。无公共 API 签名变更。 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)
⚔️ Resolve merge conflicts
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 |
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: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/actions/model-prices.ts (1)
661-694:⚠️ Potential issue | 🟠 Major校验额外计费字段，避免保存负价格。
基础价格字段都做了非负校验，但
extraFieldsJson中的*_cost_*/*_price_*等额外计费字段会直接写入priceData。这会允许input_cost_per_second: -1之类的数据进入价格表，后续计费或详情展示可能被污染。建议修改
if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) { return { ok: false, error: "高级字段必须是 JSON 对象" }; } + + for (const [key, value] of Object.entries(parsed)) { + const isPriceLikeField = /(?:^|_)(?:cost|price)(?:_|$)/.test(key); + if ( + isPriceLikeField && + (typeof value !== "number" || !Number.isFinite(value) || value < 0) + ) { + return { ok: false, error: `高级字段 ${key} 必须是非负数` }; + } + } + extraPriceData = parsed as Record<string, unknown>;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/actions/model-prices.ts` around lines 661 - 694, The parsed extraFieldsJson (extraPriceData) can contain arbitrary cost/price fields which are currently merged into priceData without validation; update the flow after JSON.parse and before merging by scanning extraPriceData for keys that indicate monetary/usage rates (e.g., keys matching /(cost|price|per_|per[A-Za-z0-9_]*|_cost_|_price_|per_second|per_token|per_image|per_request)/i), ensure each corresponding value is a number and clamp or reject negative values (either remove those keys, set them to undefined, or return a validation error), and coerce valid numeric strings to numbers; keep this validation localized to the extraFieldsJson parsing block that produces extraPriceData so priceData and ModelPriceData only receive non-negative numeric cost/price fields.
🧹 Nitpick comments (5)
tests/unit/actions/model-prices.test.ts (1)
177-206: 建议补充负路径用例。当前仅覆盖了合法 JSON 合并路径。建议再加至少两条：1)
extraFieldsJson为非法 JSON 时应返回ok=false且不调用仓库；2) 合法 JSON 但其中与表单字段（如input_cost_per_token）冲突时的优先级（表单 vs extra）行为，以锁定合并顺序。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/actions/model-prices.test.ts` around lines 177 - 206, Add two negative-path unit tests for upsertSingleModelPrice: 1) when extraFieldsJson is invalid JSON, assert the returned result.ok is false and that upsertModelPrice (upsertModelPriceMock) is not called; 2) when extraFieldsJson is valid but contains keys that conflict with form-provided fields (e.g., input_cost_per_token or mode), assert and lock the merge priority by testing the expected precedence (choose and assert either that form fields override extra JSON or vice versa) — reference upsertSingleModelPrice, extraFieldsJson, and upsertModelPriceMock in the new assertions to ensure behavior is deterministic.src/app/[locale]/settings/prices/_components/model-price-details-dialog.tsx (3)
92-109:providerGroups的useMemo实际上每次都会重算，缓存失效。
providerEntries在每次渲染时都由entries.filter(...)产生一个新的数组引用（Line 99），而providerGroups的useMemo依赖项就是这个新引用（Line 109），因此Object.is比较永远不等，memo 起不到缓存作用。同理，coreEntries/additionalBillableEntries/additionalMetadataEntries也在每次渲染时重新过滤，属于小的冗余计算。建议将所有分组逻辑合并到一个以
entries（或price.priceData）为依赖的useMemo中一次性完成：♻️ 建议的重构
- const entries = useMemo(() => collectModelPriceFieldEntries(price.priceData), [price.priceData]); - const coreEntries = entries.filter((entry) => entry.isCore && entry.source === "top_level"); - const additionalBillableEntries = entries.filter( - (entry) => !entry.isCore && entry.kind !== "display" && entry.source === "top_level" - ); - const additionalMetadataEntries = entries.filter( - (entry) => !entry.isCore && entry.kind === "display" && entry.source === "top_level" - ); - const providerEntries = entries.filter((entry) => entry.source === "provider_pricing"); - const providerGroups = useMemo(() => { - const groups = new Map<string, ModelPriceFieldEntry[]>(); - for (const entry of providerEntries) { - const key = entry.providerKey ?? "unknown"; - const bucket = groups.get(key) ?? []; - bucket.push(entry); - groups.set(key, bucket); - } - return Array.from(groups.entries()).sort((a, b) => a[0].localeCompare(b[0])); - }, [providerEntries]); + const { coreEntries, additionalBillableEntries, additionalMetadataEntries, providerGroups } = + useMemo(() => { + const entries = collectModelPriceFieldEntries(price.priceData); + const core: ModelPriceFieldEntry[] = []; + const addBillable: ModelPriceFieldEntry[] = []; + const addMetadata: ModelPriceFieldEntry[] = []; + const providerGroupsMap = new Map<string, ModelPriceFieldEntry[]>(); + for (const entry of entries) { + if (entry.source === "provider_pricing") { + const key = entry.providerKey ?? "unknown"; + const bucket = providerGroupsMap.get(key) ?? []; + bucket.push(entry); + providerGroupsMap.set(key, bucket); + } else if (entry.source === "top_level") { + if (entry.isCore) core.push(entry); + else if (entry.kind === "display") addMetadata.push(entry); + else addBillable.push(entry); + } + } + return { + coreEntries: core, + additionalBillableEntries: addBillable, + additionalMetadataEntries: addMetadata, + providerGroups: Array.from(providerGroupsMap.entries()).sort((a, b) => + a[0].localeCompare(b[0]) + ), + }; + }, [price.priceData]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/settings/prices/_components/model-price-details-dialog.tsx around lines 92 - 109, providerGroups is recomputed every render because providerEntries (and the other filtered arrays coreEntries, additionalBillableEntries, additionalMetadataEntries) are recreated by entries.filter(...) each render and used as dependencies in separate useMemo calls; consolidate the filtering into a single useMemo that depends on entries (or price.priceData) and inside it compute coreEntries, additionalBillableEntries, additionalMetadataEntries, providerEntries and providerGroups in one pass (grouping by providerKey and sorting as required) and return the computed values so subsequent renders use the stable memoized results instead of re-filtering on new array references.
120-120:trigger || defaultTrigger在传入合法 falsy 节点时会意外回退。
React.ReactNode允许null、false、0、""作为有效值。当前trigger || defaultTrigger的写法意味着若调用方显式传入null（例如想要隐藏默认触发器、使用外部控制弹窗时），组件仍会渲染默认 "View details" 按钮。建议使用trigger ?? defaultTrigger，仅在trigger为undefined时回退，更贴合语义。♻️ 建议修改
- <DialogTrigger asChild>{trigger || defaultTrigger}</DialogTrigger> + <DialogTrigger asChild>{trigger ?? defaultTrigger}</DialogTrigger>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/settings/prices/_components/model-price-details-dialog.tsx at line 120, The DialogTrigger fallback logic uses "trigger || defaultTrigger", which will incorrectly replace valid falsy React nodes (null/false/0/""); update the component (the DialogTrigger usage in ModelPriceDetailsDialog) to use the nullish coalescing operator "trigger ?? defaultTrigger" so it only falls back when trigger is undefined, preserving explicit falsy values passed by callers.
31-33: 数值格式化会在非常小/大的数值上显示为科学计数法。
String(value)会对例如0.0000025这样的定价值输出"0.0000025"（OK），但对更小的数（如1e-7）或更大的数会变为科学计数法，对定价展示可能不直观。考虑使用Number.prototype.toLocaleString或自定义的定价格式化（例如根据字段语义选择/M tokens、/img等）以与同模块中其他价格展示保持一致。如果当前业务场景下所有字段都已用原始精度存储且预期按原样显示，则可忽略此建议。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`[locale]/settings/prices/_components/model-price-details-dialog.tsx around lines 31 - 33, The numeric-to-string conversion currently uses String(value) in the number branch (where you check typeof value === "number" and Number.isFinite(value)), which can produce scientific notation for very small/large prices; replace that conversion with a consistent price formatter (e.g., Number.prototype.toLocaleString with appropriate minimumFractionDigits/maximumFractionDigits or a shared price-format helper used elsewhere in this module) so values like 1e-7 render as a readable decimal and follow the same `/M tokens` or `/img` semantics used by the rest of the component; update the code around the value check (the Number.isFinite(value) branch) to call the chosen formatter instead of String(value).tests/unit/settings/prices/model-price-details-dialog.test.tsx (1)
66-75: 断言依赖英文硬编码文案，建议改用更稳定的定位方式。测试通过
textContent?.includes("View details")以及后续的"Core fields"、"Additional billable fields"字符串查找触发器和验证渲染。这些字面量直接耦合到en语言包中的具体翻译文案，未来一旦文案措辞调整（例如改为 "Show details"），测试就会失败，即便组件行为完全正确。建议改用
data-testid、aria-label或通过messages对象读取相应的翻译键（如messages.settings.prices.actions.viewDetails）来定位元素与断言内容，降低与具体文案的耦合。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/settings/prices/model-price-details-dialog.test.tsx` around lines 66 - 75, The test is brittle because it finds the trigger by matching textContent?.includes("View details"); update the test to locate the button more stably (e.g., use a data-testid or aria-label on the trigger button) instead of literal English copy: replace the Array.from(document.querySelectorAll("button")).find(...) lookup (the trigger variable) with a query that uses getByTestId / queryByTestId or getByLabelText, or alternatively read the translation key from the messages object (messages.settings.prices.actions.viewDetails) and use that value for lookup; ensure the same approach is used for subsequent assertions that check "Core fields" and "Additional billable fields" so the test no longer depends on hard-coded English strings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@messages/en/settings/prices.json`:
- Around line 197-199: The placeholder for the advancedFields JSON is not valid
JSON (uses key = value); update the "advancedFieldsPlaceholder" value to a valid
JSON example (e.g. use an object with quoted keys and numeric values) so users
can copy/paste without JSON.parse errors; locate the keys "advancedFieldsJson",
"advancedFieldsPlaceholder", and "advancedFieldsHint" in the diff and replace
the placeholder string accordingly.
In `@messages/ja/settings/prices.json`:
- Around line 197-199: The placeholder string for advancedFieldsPlaceholder is
not valid JSON; update the value to a legal JSON example so users can copy/paste
successfully (change the placeholder key "advancedFieldsPlaceholder" and ensure
the example uses double-quoted keys, colon separators and proper JSON value
types, e.g. a quoted key like "input_cost_per_second" with a numeric value like
0.5), leaving advancedFieldsJson and advancedFieldsHint as-is.
In `@messages/ru/settings/prices.json`:
- Around line 197-199: The placeholder value for advancedFieldsPlaceholder is
not valid JSON (it uses "key = value"), so replace it with a legal JSON example
(e.g., an object with a quoted key and numeric value) so users copying it won't
trigger JSON.parse errors; update the string referenced by
"advancedFieldsPlaceholder" (and ensure consistency with
"advancedFieldsJson"/"advancedFieldsHint" wording) to something like a valid
JSON object with quoted keys and proper separators.
In `@messages/zh-CN/settings/prices.json`:
- Around line 197-199: The placeholder for advancedFieldsPlaceholder is not
valid JSON (it uses "input_cost_per_second = 0.5") so users copying it will
trigger JSON.parse errors; update the value of advancedFieldsPlaceholder to a
valid JSON example (e.g., an object with a numeric field or string) so it
matches the advancedFieldsJson/advancedFieldsHint context and will parse
correctly when saved, referring to the keys advancedFieldsJson,
advancedFieldsPlaceholder, and advancedFieldsHint to locate the text to change.
In `@messages/zh-TW/settings/prices.json`:
- Around line 197-199: The placeholder "advancedFieldsPlaceholder" currently
shows an invalid example using "key = value"; update that string to present a
valid JSON object example (a JSON object with a quoted key and colon-separated
value, e.g., a single-property object with the key in double quotes and a
numeric value) so users can copy/paste without JSON.parse failing; keep
"advancedFieldsJson" and "advancedFieldsHint" unchanged but ensure the
placeholder clearly indicates it's a JSON object.
In `@src/lib/utils/model-price-fields.ts`:
- Around line 115-119: The isPriceLikeFieldKey function's regex is too
permissive and causes false positives (e.g., "generate", "session_id",
"seconds_since_*"); tighten it by changing the matching strategy in
isPriceLikeFieldKey to require explicit suffixes/prefixes or boundaries — for
example match keys that end with _cost, _price, _rate, _multiplier (use $
anchors), or start with cost_per_, price_per_, price_per or contain explicit
patterns like cost_per_* / price_per_*; use word boundaries (\b) or anchors
instead of substring matches so collectAdditionalPriceLikeNumbers /
hasValidPriceData only treat clearly labeled price fields as price-like.
- Around line 91-113: The LABEL_OVERRIDES map and humanizeKey() currently
produce hardcoded English labels; move localization into the UI by removing or
ignoring English values from LABEL_OVERRIDES in rendering and instead use
useTranslations("settings.prices") inside ModelPriceDetailsDialog to resolve
labels for known keys (e.g., display_name, litellm_provider,
selected_pricing_provider, max_input_tokens, etc.), falling back to
humanizeKey(key) only when the translation lookup returns undefined; update
ModelPriceDetailsDialog to call the translation function for each field label
and reserve humanizeKey/ LABEL_OVERRIDES only as a last-resort fallback.
In `@src/lib/utils/price-data.ts`:
- Around line 55-62: 当前逻辑把 collectAdditionalPriceLikeNumbers 的所有结果并入
hasValidNumericPrice，导致上游
isPriceLikeFieldKey/正则误判时仅含元数据的记录也会被认定为“有效价格”；请修改该判断：仅当
collectNumericCosts(priceData) 返回至少一个数值时才直接返回 true，或者在将
collectAdditionalPriceLikeNumbers 的结果并入前用 isPriceLikeFieldKey（来自
model-price-fields.ts）对字段做一次严格过滤／验证；定位并修改使用
hasValidNumericPrice、collectNumericCosts、collectAdditionalPriceLikeNumbers
的代码路径以实现上述改动。
---
Outside diff comments:
In `@src/actions/model-prices.ts`:
- Around line 661-694: The parsed extraFieldsJson (extraPriceData) can contain
arbitrary cost/price fields which are currently merged into priceData without
validation; update the flow after JSON.parse and before merging by scanning
extraPriceData for keys that indicate monetary/usage rates (e.g., keys matching
/(cost|price|per_|per[A-Za-z0-9_]*|_cost_|_price_|per_second|per_token|per_image|per_request)/i),
ensure each corresponding value is a number and clamp or reject negative values
(either remove those keys, set them to undefined, or return a validation error),
and coerce valid numeric strings to numbers; keep this validation localized to
the extraFieldsJson parsing block that produces extraPriceData so priceData and
ModelPriceData only receive non-negative numeric cost/price fields.
---
Nitpick comments:
In `@src/app/`[locale]/settings/prices/_components/model-price-details-dialog.tsx:
- Around line 92-109: providerGroups is recomputed every render because
providerEntries (and the other filtered arrays coreEntries,
additionalBillableEntries, additionalMetadataEntries) are recreated by
entries.filter(...) each render and used as dependencies in separate useMemo
calls; consolidate the filtering into a single useMemo that depends on entries
(or price.priceData) and inside it compute coreEntries,
additionalBillableEntries, additionalMetadataEntries, providerEntries and
providerGroups in one pass (grouping by providerKey and sorting as required) and
return the computed values so subsequent renders use the stable memoized results
instead of re-filtering on new array references.
- Line 120: The DialogTrigger fallback logic uses "trigger || defaultTrigger",
which will incorrectly replace valid falsy React nodes (null/false/0/""); update
the component (the DialogTrigger usage in ModelPriceDetailsDialog) to use the
nullish coalescing operator "trigger ?? defaultTrigger" so it only falls back
when trigger is undefined, preserving explicit falsy values passed by callers.
- Around line 31-33: The numeric-to-string conversion currently uses
String(value) in the number branch (where you check typeof value === "number"
and Number.isFinite(value)), which can produce scientific notation for very
small/large prices; replace that conversion with a consistent price formatter
(e.g., Number.prototype.toLocaleString with appropriate
minimumFractionDigits/maximumFractionDigits or a shared price-format helper used
elsewhere in this module) so values like 1e-7 render as a readable decimal and
follow the same `/M tokens` or `/img` semantics used by the rest of the
component; update the code around the value check (the Number.isFinite(value)
branch) to call the chosen formatter instead of String(value).
In `@tests/unit/actions/model-prices.test.ts`:
- Around line 177-206: Add two negative-path unit tests for
upsertSingleModelPrice: 1) when extraFieldsJson is invalid JSON, assert the
returned result.ok is false and that upsertModelPrice (upsertModelPriceMock) is
not called; 2) when extraFieldsJson is valid but contains keys that conflict
with form-provided fields (e.g., input_cost_per_token or mode), assert and lock
the merge priority by testing the expected precedence (choose and assert either
that form fields override extra JSON or vice versa) — reference
upsertSingleModelPrice, extraFieldsJson, and upsertModelPriceMock in the new
assertions to ensure behavior is deterministic.
In `@tests/unit/settings/prices/model-price-details-dialog.test.tsx`:
- Around line 66-75: The test is brittle because it finds the trigger by
matching textContent?.includes("View details"); update the test to locate the
button more stably (e.g., use a data-testid or aria-label on the trigger button)
instead of literal English copy: replace the
Array.from(document.querySelectorAll("button")).find(...) lookup (the trigger
variable) with a query that uses getByTestId / queryByTestId or getByLabelText,
or alternatively read the translation key from the messages object
(messages.settings.prices.actions.viewDetails) and use that value for lookup;
ensure the same approach is used for subsequent assertions that check "Core
fields" and "Additional billable fields" so the test no longer depends on
hard-coded English strings.
🪄 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: 1fae5692-f47a-48cb-9e24-1729c014fb68
📒 Files selected for processing (18)
messages/en/settings/prices.jsonmessages/ja/settings/prices.jsonmessages/ru/settings/prices.jsonmessages/zh-CN/settings/prices.jsonmessages/zh-TW/settings/prices.jsonsrc/actions/model-prices.tssrc/app/[locale]/settings/prices/_components/model-price-details-dialog.tsxsrc/app/[locale]/settings/prices/_components/model-price-drawer.tsxsrc/app/[locale]/settings/prices/_components/price-list.tsxsrc/lib/utils/model-price-fields.tssrc/lib/utils/price-data.tstests/integration/billing-model-source.test.tstests/unit/actions/model-prices.test.tstests/unit/lib/model-price-fields.test.tstests/unit/lib/price-data-price-like-fields.test.tstests/unit/price-sync/cloud-price-table.test.tstests/unit/settings/prices/model-price-details-dialog.test.tsxtests/unit/settings/prices/model-price-drawer-prefill-and-submit-ui.test.tsx
| const LABEL_OVERRIDES: Record<string, string> = { | ||
| display_name: "Display name", | ||
| litellm_provider: "LiteLLM provider", | ||
| selected_pricing_provider: "Selected pricing provider", | ||
| selected_pricing_source_model: "Selected source model", | ||
| selected_pricing_resolution: "Selected resolution", | ||
| max_input_tokens: "Max input tokens", | ||
| max_output_tokens: "Max output tokens", | ||
| max_tokens: "Max tokens", | ||
| output_vector_size: "Output vector size", | ||
| }; | ||
|
|
||
| function isPlainObject(value: unknown): value is Record<string, unknown> { | ||
| return typeof value === "object" && value !== null && !Array.isArray(value); | ||
| } | ||
|
|
||
| function humanizeKey(key: string): string { | ||
| if (LABEL_OVERRIDES[key]) { | ||
| return LABEL_OVERRIDES[key]; | ||
| } | ||
|
|
||
| return key.replace(/_/g, " ").replace(/\b([a-z])/g, (match) => match.toUpperCase()); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 确认 label 是否直接渲染到 UI
fd -e tsx 'model-price-details-dialog' | xargs rg -n -C2 '\.label|humanizeKey|LABEL_OVERRIDES'Repository: ding113/claude-code-hub
Length of output: 751
🏁 Script executed:
rg -n 'entry\.|label' src/app/\[locale\]/settings/prices/_components/model-price-details-dialog.tsx -B5 -A5Repository: ding113/claude-code-hub
Length of output: 3935
🏁 Script executed:
rg -n 'import.*humanizeKey|from.*model-price-fields' src/app/\[locale\]/settings/prices/_components/model-price-details-dialog.tsxRepository: ding113/claude-code-hub
Length of output: 110
🏁 Script executed:
cat -n src/app/\[locale\]/settings/prices/_components/model-price-details-dialog.tsx | head -100Repository: ding113/claude-code-hub
Length of output: 3772
🏁 Script executed:
cat -n src/lib/utils/model-price-fields.ts | head -150Repository: ding113/claude-code-hub
Length of output: 5583
🏁 Script executed:
rg -n 'collectModelPriceFieldEntries|entry\.label|humanizeKey' src/lib/utils/model-price-fields.ts -B3 -A3Repository: ding113/claude-code-hub
Length of output: 673
修复硬编码字段标签，应用 i18n。
humanizeKey() 生成的字段标签在 ModelPriceDetailsDialog 中直接渲染给用户（line 74），但这些标签为硬编码英文：
LABEL_OVERRIDES包含英文字符串（"Display name", "LiteLLM provider" 等）- 缺失覆盖的字段由函数生成英文格式标签（例如 "Input Cost Per Token"）
应将这些标签的本地化处理移至对话框组件内：在 ModelPriceDetailsDialog 中使用 useTranslations("settings.prices") 为已知字段提供翻译查询，仅在无译文时才降级使用 humanizeKey()。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/utils/model-price-fields.ts` around lines 91 - 113, The
LABEL_OVERRIDES map and humanizeKey() currently produce hardcoded English
labels; move localization into the UI by removing or ignoring English values
from LABEL_OVERRIDES in rendering and instead use
useTranslations("settings.prices") inside ModelPriceDetailsDialog to resolve
labels for known keys (e.g., display_name, litellm_provider,
selected_pricing_provider, max_input_tokens, etc.), falling back to
humanizeKey(key) only when the translation lookup returns undefined; update
ModelPriceDetailsDialog to call the translation function for each field label
and reserve humanizeKey/ LABEL_OVERRIDES only as a last-resort fallback.
| if ( | ||
| hasValidNumericPrice([ | ||
| ...collectNumericCosts(priceData), | ||
| ...collectAdditionalPriceLikeNumbers(priceData), | ||
| ]) | ||
| ) { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
逻辑变更依赖 isPriceLikeFieldKey 的准确性。
此处把 collectAdditionalPriceLikeNumbers 的结果并入 hasValidNumericPrice 输入。一旦上游正则误判（见 model-price-fields.ts 的建议），仅含元数据的记录也可能被判定为“有效价格”，从而影响订阅/计费前置校验。修正上游正则后本处即可收敛。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/utils/price-data.ts` around lines 55 - 62, 当前逻辑把
collectAdditionalPriceLikeNumbers 的所有结果并入 hasValidNumericPrice，导致上游
isPriceLikeFieldKey/正则误判时仅含元数据的记录也会被认定为“有效价格”；请修改该判断：仅当
collectNumericCosts(priceData) 返回至少一个数值时才直接返回 true，或者在将
collectAdditionalPriceLikeNumbers 的结果并入前用 isPriceLikeFieldKey（来自
model-price-fields.ts）对字段做一次严格过滤／验证；定位并修改使用
hasValidNumericPrice、collectNumericCosts、collectAdditionalPriceLikeNumbers
的代码路径以实现上述改动。
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| export function isPriceLikeFieldKey(key: string): boolean { | ||
| return /(cost|price|rate|multiplier|per_|second|session|query|page|pixel|character|dbu)/i.test( | ||
| key | ||
| ); | ||
| } |
There was a problem hiding this comment.
The pattern tests whether any of those tokens appear anywhere in the key. Fields like response_time_in_seconds, session_timeout, max_queries, total_pages, or character_limit all satisfy the test, so hasValidPriceData can return true for models that only carry metadata numerics, and the details dialog will badge them as "unsupported billing field". Consider anchoring the ambiguous tokens to their pricing-specific compound forms (e.g., per_second, per_session, per_query, per_page) rather than accepting them as bare substrings.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/utils/model-price-fields.ts
Line: 115-119
Comment:
**Regex matches "second", "session", "query", "page", "character" as bare substrings — false positives**
The pattern tests whether any of those tokens appear *anywhere* in the key. Fields like `response_time_in_seconds`, `session_timeout`, `max_queries`, `total_pages`, or `character_limit` all satisfy the test, so `hasValidPriceData` can return `true` for models that only carry metadata numerics, and the details dialog will badge them as "unsupported billing field". Consider anchoring the ambiguous tokens to their pricing-specific compound forms (e.g., `per_second`, `per_session`, `per_query`, `per_page`) rather than accepting them as bare substrings.
How can I resolve this? If you propose a fix, please make it concise.| let extraPriceData: Record<string, unknown> = {}; | ||
| if (input.extraFieldsJson?.trim()) { | ||
| try { | ||
| const parsed = JSON.parse(input.extraFieldsJson); | ||
| if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) { | ||
| return { ok: false, error: "高级字段必须是 JSON 对象" }; | ||
| } | ||
| extraPriceData = parsed as Record<string, unknown>; | ||
| } catch (error) { | ||
| return { | ||
| ok: false, | ||
| error: | ||
| error instanceof Error | ||
| ? `高级字段 JSON 解析失败: ${error.message}` | ||
| : "高级字段 JSON 解析失败", | ||
| }; | ||
| } | ||
| } |
There was a problem hiding this comment.
extraFieldsJson bypass non-negative validation
The action validates each explicit price field (inputCostPerToken, outputCostPerToken, etc.) for non-negative finiteness, but the parsed extraPriceData receives no equivalent check. A well-formed JSON object like {"output_cost_per_token_priority": -0.5} passes the type guard and gets written to the DB. Applying a shallow check over the parsed object's numeric values, or at least documenting the intentional gap, would align it with the rest of the validation logic.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/actions/model-prices.ts
Line: 661-678
Comment:
**Numeric values inside `extraFieldsJson` bypass non-negative validation**
The action validates each explicit price field (`inputCostPerToken`, `outputCostPerToken`, etc.) for non-negative finiteness, but the parsed `extraPriceData` receives no equivalent check. A well-formed JSON object like `{"output_cost_per_token_priority": -0.5}` passes the type guard and gets written to the DB. Applying a shallow check over the parsed object's numeric values, or at least documenting the intentional gap, would align it with the rest of the validation logic.
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 introduces an "Advanced fields" feature for model pricing, enabling the storage and editing of additional price-related fields through a JSON object. Key additions include a detailed model price information dialog, a JSON editor within the price management drawer, and utility logic for classifying and extracting dynamic pricing fields. Feedback identifies a critical issue where existing provider-specific pricing data is lost during manual edits because it is excluded from the editable fields. Additionally, a refinement to the regex for identifying price-like fields was suggested to prevent false positives on metadata like timeouts.
| "cache_read_input_token_cost", | ||
| "cache_creation_input_token_cost", | ||
| "cache_creation_input_token_cost_above_1hr", | ||
| "pricing", |
There was a problem hiding this comment.
Including pricing in NON_EDITABLE_MANAGED_FIELDS causes it to be stripped from the editable JSON in the UI. Since the upsertSingleModelPrice server action builds the new price data object from scratch using only the form fields and the JSON input, any existing provider-specific pricing (the pricing object) will be permanently lost when a model is manually edited. If the intention is to preserve all extra fields, pricing should be allowed in the JSON editor or handled explicitly on the server to merge with existing data.
| return /(cost|price|rate|multiplier|per_|second|session|query|page|pixel|character|dbu)/i.test( | ||
| key | ||
| ); |
There was a problem hiding this comment.
The regex for identifying price-like fields is too broad and will flag common metadata fields for review. For example, timeout_seconds or max_seconds will match second, and max_characters will match character. Consider using word boundaries or more specific patterns to reduce false positives.
| return /(cost|price|rate|multiplier|per_|second|session|query|page|pixel|character|dbu)/i.test( | |
| key | |
| ); | |
| return /(cost|price|rate|multiplier|per_|(_|^)second$|(_|^)session$|(_|^)query$|(_|^)page$|(_|^)pixel$|(_|^)character$|(_|^)dbu$)/i.test( | |
| key | |
| ); |
![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: 128c0c3b5e
ℹ️ 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".
| hasValidNumericPrice([ | ||
| ...collectNumericCosts(priceData), | ||
| ...collectAdditionalPriceLikeNumbers(priceData), | ||
| ]) |
There was a problem hiding this comment.
Including collectAdditionalPriceLikeNumbers in hasValidPriceData makes fields like input_cost_per_second/file_search_cost_per_1k_calls count as “valid pricing”, but calculateRequestCost still ignores those keys and only bills token/request/image/cache fields. Because resolvePricingForModelRecords prioritizes manual records whenever hasValidPriceData is true, a manual override that only has these unsupported fields can now suppress a valid cloud pricing record and produce $0 charges instead of the expected billed amount.
Useful? React with 👍 / 👎.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| "cachePricingTitle": "Cache Pricing", | ||
| "advancedFieldsTitle": "Advanced fields", | ||
| "advancedFieldsJson": "Additional price fields JSON", | ||
| "advancedFieldsPlaceholder": "Example: input_cost_per_second = 0.5", |
There was a problem hiding this comment.
[HIGH] [COMMENT-INACCURATE] The placeholder shows invalid syntax for a JSON-only field
Why this is a problem: upsertSingleModelPrice() only accepts a JSON object in extraFieldsJson, but this example uses assignment syntax (input_cost_per_second = 0.5). If a user follows the placeholder literally, save fails with a JSON parse error.
Suggested fix:
"advancedFieldsPlaceholder": "{\"input_cost_per_second\": 0.5}"Please mirror the same fix in the other locale files because the same placeholder was added there too.
| return typeof value === "object" && value !== null && !Array.isArray(value); | ||
| } | ||
|
|
||
| function humanizeKey(key: string): string { |
There was a problem hiding this comment.
[HIGH] [STANDARD-VIOLATION] The new details dialog hardcodes English labels instead of using i18n
Why this is a problem: CLAUDE.md says "All user-facing strings must use i18n (5 languages supported). Never hardcode display text". LABEL_OVERRIDES and humanizeKey() always produce English ("Display name", "Max input tokens", title-cased snake_case, etc.), so the new dialog is partially untranslated in every non-English locale.
Suggested fix:
// model-price-fields.ts
entries.push({
key,
label: key,
// ...
});
// model-price-details-dialog.tsx
const fieldLabelMap: Partial<Record<string, string>> = {
display_name: t("details.fieldLabels.display_name"),
litellm_provider: t("details.fieldLabels.litellm_provider"),
};
const label = fieldLabelMap[entry.key] ?? entry.path;That keeps the fallback technical path, while moving user-facing labels into messages/*/settings/prices.json.
| return "supported"; | ||
| } | ||
|
|
||
| if (isPriceLikeFieldKey(key)) { |
There was a problem hiding this comment.
[HIGH] [LOGIC-BUG] Nested price objects are misclassified as metadata because only the leaf key is inspected
Why this is a problem: classifyField() checks isPriceLikeFieldKey(key), but some billable structures only expose the pricing hint in the parent path. For example, search_context_cost_per_query.search_context_size_high ends up with key === "search_context_size_high", so it is tagged as display and rendered under Additional metadata even though it is a pricing field.
Suggested fix:
function classifyField(path: string, key: string): ModelPriceFieldKind {
if (
SUPPORTED_TOP_LEVEL_BILLING_KEYS.has(key) ||
(path.startsWith("long_context_pricing.") && SUPPORTED_LONG_CONTEXT_KEYS.has(key))
) {
return "supported";
}
if (isPriceLikeFieldKey(key) || isPriceLikeFieldKey(path)) {
return "unsupported";
}
return "display";
}Using the full path here keeps nested pricing nodes in the billable section instead of hiding them as metadata.
There was a problem hiding this comment.
Code Review Summary
I found 3 high-priority issues in the new pricing details/editor work. The main problems are a broken JSON example in the new advanced-fields UI, hardcoded English labels in a localized dialog, and classification logic that hides some nested pricing fields under metadata.
PR Size: L
- Lines changed: 952
- Files changed: 18
- Split suggestion: Separate the server/data-shape changes (
src/actions,src/lib/utils, billing tests) from the UI/i18n additions (messages/*, drawer/details dialog) so review and rollback stay smaller.
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 2 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 1 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Critical Issues (Must Fix)
- None.
High Priority Issues (Should Fix)
messages/*/settings/prices.json: the new advanced-fields placeholder shows assignment syntax instead of a JSON object, so users following the example will hit a parse error on save.src/lib/utils/model-price-fields.ts: the new details dialog hardcodes English field labels, which violates the repo i18n rule and leaves non-English locales partially untranslated.src/lib/utils/model-price-fields.ts: nested pricing paths likesearch_context_cost_per_query.*are classified as metadata because only the leaf key is inspected.
Review Coverage
- Logic and correctness
- Security (OWASP Top 10)
- Error handling
- Type safety
- Documentation accuracy
- Test coverage
- Code clarity
Automated review by Codex AI
There was a problem hiding this comment.
Review Complete
- Reviewed PR
#1049, applied thesize/Llabel, and submitted the required summary review. - Posted 3 inline comments on validated new-code issues:
- Invalid JSON placeholder in
messages/en/settings/prices.json:198(same problem exists in the other locale files). - Hardcoded English field labels bypassing i18n in
src/lib/utils/model-price-fields.ts:107. - Nested pricing paths being classified as metadata in
src/lib/utils/model-price-fields.ts:129.
- Invalid JSON placeholder in
- Summary also includes an
L-PR split suggestion: separate server/data-shape changes from UI/i18n work next time.
Want me to do a follow-up review after the author pushes fixes?
🧪 测试结果
总体结果: ✅ 所有测试通过 |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
messages/zh-TW/settings/prices.json (1)
198-198:⚠️ Potential issue | 🟡 Minor将 JSON 占位示例改为可复制的合法 JSON。
这里标注为 JSON，但示例使用
key = value，用户照抄后会被JSON.parse拒绝。建议修改
- "advancedFieldsPlaceholder": "範例：input_cost_per_second = 0.5", + "advancedFieldsPlaceholder": "範例：{\"input_cost_per_second\": 0.5}",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@messages/zh-TW/settings/prices.json` at line 198, 字段 advancedFieldsPlaceholder 当前给出的示例使用了 key = value 语法，会导致 JSON.parse 失败；将该占位示例改成一个合法的 JSON 对象格式：用双引号包住键名、用冒号分隔键和值、数值不加引号，并用大括号包裹整个对象（例如键名为 input_cost_per_second 且值为 0.5 的合法 JSON 表示），以确保用户复制粘贴后能被 JSON.parse 正常解析。
🧹 Nitpick comments (1)
src/actions/model-prices.ts (1)
717-731:extraPriceData会被表单字段的undefined覆盖，确认行为符合预期。
priceData使用{ ...extraPriceData, ...formFields }展开；当用户未在抽屉表单里填写displayName/inputCostPerToken等字段时，对应键会以undefined值被显式写入，从而覆盖extraFieldsJson中的同名键（例如用户在 JSON 里写了display_name: "Foo"，但 UI 文本框清空，最终display_name会变为undefined）。鉴于
NON_EDITABLE_MANAGED_FIELDS（src/lib/utils/model-price-fields.ts:76-88）已把这些字段从"可编辑 extras"中剔除，正常流程下 JSON 编辑器不会再提交这些键，这个覆盖策略是可接受的；但在用户手工在 JSON 里补这些键、又清空对应 UI 字段的边缘路径下会出现"静默丢失"。请确认这是期望行为，或考虑用条件展开避免undefined覆盖：♻️ 可选：避免 undefined 覆盖 extras
- const priceData: ModelPriceData = { - ...extraPriceData, - mode: input.mode, - display_name: input.displayName?.trim() || undefined, - litellm_provider: input.litellmProvider || undefined, - supports_prompt_caching: input.supportsPromptCaching, - input_cost_per_token: input.inputCostPerToken, - ... - }; + const priceData: ModelPriceData = { + ...extraPriceData, + mode: input.mode, + ...(input.displayName?.trim() ? { display_name: input.displayName.trim() } : {}), + ...(input.litellmProvider ? { litellm_provider: input.litellmProvider } : {}), + ...(input.supportsPromptCaching !== undefined + ? { supports_prompt_caching: input.supportsPromptCaching } + : {}), + ...(input.inputCostPerToken !== undefined + ? { input_cost_per_token: input.inputCostPerToken } + : {}), + // ...同理处理剩余字段 + };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/actions/model-prices.ts` around lines 717 - 731, The current priceData construction spreads extraPriceData then writes form fields directly, so undefined form values (e.g., input.displayName, input.inputCostPerToken) will overwrite extras and silently drop JSON-provided values; update the construction of priceData (the block that creates priceData using ...extraPriceData and the listed input.* fields) to only assign each form-derived key when its input value is not undefined (or empty after trim for strings), e.g. build priceData from ...extraPriceData and conditionally spread each field only if input.<field> !== undefined/empty; reference NON_EDITABLE_MANAGED_FIELDS to confirm which keys must remain editable via extras vs UI, or keep current behavior if silent overwrites are acceptable.
🤖 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/actions/model-prices.ts`:
- Around line 598-633: The sanitizeExtraPriceData function duplicates a
price-like field regex that is inconsistent with the exported utility; replace
the inline regex check in sanitizeExtraPriceData with a call to the shared
predicate (isPriceLikeFieldPath or isPriceLikeFieldKey as exported from
src/lib/utils/model-price-fields) by importing it, so price-like detection
(including "second") is centralized; keep the existing numeric/string conversion
and non-negative validation logic intact and only swap the regex condition to
use the imported function (ensure the same path argument you pass into
sanitizeExtraPriceData is fed to the utility).
- Around line 602-633: sanitizeExtraPriceData currently iterates
Object.entries(value) and will copy dangerous keys like "__proto__",
"constructor", and "prototype" into the returned object (and later merged into
priceData in upsertSingleModelPrice), opening the door to prototype pollution;
update sanitizeExtraPriceData to explicitly skip those three keys when mapping
object entries (i.e., if key is "__proto__" || key === "constructor" || key ===
"prototype" then omit it) so they are not recursed into or returned, preserving
the same defensive behavior as isPriceDataEqual and preventing unsafe merging in
upsertSingleModelPrice.
---
Duplicate comments:
In `@messages/zh-TW/settings/prices.json`:
- Line 198: 字段 advancedFieldsPlaceholder 当前给出的示例使用了 key = value 语法，会导致
JSON.parse 失败；将该占位示例改成一个合法的 JSON 对象格式：用双引号包住键名、用冒号分隔键和值、数值不加引号，并用大括号包裹整个对象（例如键名为
input_cost_per_second 且值为 0.5 的合法 JSON 表示），以确保用户复制粘贴后能被 JSON.parse 正常解析。
---
Nitpick comments:
In `@src/actions/model-prices.ts`:
- Around line 717-731: The current priceData construction spreads extraPriceData
then writes form fields directly, so undefined form values (e.g.,
input.displayName, input.inputCostPerToken) will overwrite extras and silently
drop JSON-provided values; update the construction of priceData (the block that
creates priceData using ...extraPriceData and the listed input.* fields) to only
assign each form-derived key when its input value is not undefined (or empty
after trim for strings), e.g. build priceData from ...extraPriceData and
conditionally spread each field only if input.<field> !== undefined/empty;
reference NON_EDITABLE_MANAGED_FIELDS to confirm which keys must remain editable
via extras vs UI, or keep current behavior if silent overwrites are acceptable.
🪄 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: 2237dbcd-138c-4109-a99c-49dbbbc999a3
📒 Files selected for processing (11)
messages/en/settings/prices.jsonmessages/ja/settings/prices.jsonmessages/ru/settings/prices.jsonmessages/zh-CN/settings/prices.jsonmessages/zh-TW/settings/prices.jsonsrc/actions/model-prices.tssrc/app/[locale]/settings/prices/_components/model-price-details-dialog.tsxsrc/app/[locale]/settings/prices/_components/model-price-drawer.tsxsrc/lib/utils/model-price-fields.tstests/unit/actions/model-prices.test.tstests/unit/settings/prices/model-price-details-dialog.test.tsx
✅ Files skipped from review due to trivial changes (3)
- messages/zh-CN/settings/prices.json
- messages/en/settings/prices.json
- tests/unit/settings/prices/model-price-details-dialog.test.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- tests/unit/actions/model-prices.test.ts
- messages/ja/settings/prices.json
- messages/ru/settings/prices.json
- src/app/[locale]/settings/prices/_components/model-price-details-dialog.tsx
- src/app/[locale]/settings/prices/_components/model-price-drawer.tsx
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e6a4333a44
ℹ️ 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".
| if (NON_EDITABLE_MANAGED_FIELDS.has(key) || value === undefined) { | ||
| continue; | ||
| } | ||
| result[key] = value; |
There was a problem hiding this comment.
getEditableExtraPriceData currently copies every non-managed top-level key, which includes pricing, and upsertSingleModelPrice persists that blob back into a new manual record. For models whose actual rates are only under priceData.pricing (the resolver already supports this shape), this produces a manual record with no top-level billable fields; resolveManualPricing still prefers it because hasValidPriceData sees nested numbers, but calculateRequestCost does not bill from nested pricing nodes, so requests can be recorded as $0.
Useful? React with 👍 / 👎.
| return /(cost|price|rate|multiplier|per_|second|session|query|page|pixel|character|dbu)/i.test( | ||
| key |
There was a problem hiding this comment.
The matcher treats bare words such as rate, session, query, and page as price indicators, so non-billing metadata keys (for example request_rate_limit or session_timeout) are classified as price-like. Because these keys now feed collectAdditionalPriceLikeNumbers, a metadata-only manual record can pass hasValidPriceData and override cloud pricing even though the billing path cannot charge from those fields.
Useful? React with 👍 / 👎.
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 `@messages/zh-TW/settings/prices.json`:
- Line 198: The placeholder string advancedFieldsPlaceholder currently contains
outer single quotes that will cause JSON.parse to fail when users paste it;
update the value for "advancedFieldsPlaceholder" to remove the surrounding
single quotes so it contains a plain JSON object example like
{"input_cost_per_second": 0.5} (ensure the file remains valid JSON string value
and escape quotes as needed in the JSON file).
🪄 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: 92d8860a-5ff6-4244-9062-351f37ccfdc2
📒 Files selected for processing (6)
messages/en/settings/prices.jsonmessages/ja/settings/prices.jsonmessages/ru/settings/prices.jsonmessages/zh-CN/settings/prices.jsonmessages/zh-TW/settings/prices.jsonsrc/actions/model-prices.ts
✅ Files skipped from review due to trivial changes (2)
- messages/zh-CN/settings/prices.json
- messages/en/settings/prices.json
🚧 Files skipped from review as they are similar to previous changes (3)
- messages/ja/settings/prices.json
- messages/ru/settings/prices.json
- src/actions/model-prices.ts
| "cachePricingTitle": "快取價格", | ||
| "advancedFieldsTitle": "進階欄位", | ||
| "advancedFieldsJson": "額外價格欄位 JSON", | ||
| "advancedFieldsPlaceholder": "範例：'{\"input_cost_per_second\": 0.5}'", |
There was a problem hiding this comment.
占位示例外层的单引号会导致复制即失败。
当前 placeholder 为 '{"input_cost_per_second": 0.5}'，用户按示例原样粘贴到 textarea 时，外层单引号会使 JSON.parse 失败。建议去掉外层单引号，仅保留纯 JSON 对象。
建议修改
- "advancedFieldsPlaceholder": "範例：'{\"input_cost_per_second\": 0.5}'",
+ "advancedFieldsPlaceholder": "範例：{\"input_cost_per_second\": 0.5}",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@messages/zh-TW/settings/prices.json` at line 198, The placeholder string
advancedFieldsPlaceholder currently contains outer single quotes that will cause
JSON.parse to fail when users paste it; update the value for
"advancedFieldsPlaceholder" to remove the surrounding single quotes so it
contains a plain JSON object example like {"input_cost_per_second": 0.5} (ensure
the file remains valid JSON string value and escape quotes as needed in the JSON
file).
🧪 测试结果
总体结果: ✅ 所有测试通过 |
github-project-automation
Bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 19aa55e3e4
ℹ️ 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".
| if (path && isPriceLikeFieldPath(path)) { | ||
| if (typeof value === "string" && value.trim()) { |
There was a problem hiding this comment.
The new sanitizeExtraPriceData path check uses isPriceLikeFieldPath for validation, so any extra field whose key contains broad tokens like session, rate, or page is forced to be a non-negative number. This causes legitimate metadata (for example {"session_timeout":"30s"} or {"request_rate_limit":"1000rpm"}) to be rejected with a validation error, which breaks the stated goal of preserving arbitrary extra fields during manual edits. Validation should be limited to explicit billable key patterns (or a maintained allowlist) rather than all loosely “price-like” names.
Useful? React with 👍 / 👎.
1 participant
Summary
Preserve extra price-like fields during manual price edits instead of dropping them to the drawer-managed subset, align billing-model-source integration tests with breakdown-backed DB cost writes, add a semantic pricing details dialog and advanced extra-fields JSON editing for local model prices, and recognize generic price-like fields as valid pricing data.
Problem
When editing model prices through the drawer UI, only the explicitly managed fields (input/output/cache costs) were preserved. Any extra pricing fields from cloud price tables (e.g.
input_cost_per_second,file_search_cost_per_1k_calls,code_interpreter_cost_per_session) were silently dropped on save. Additionally, there was no way to inspect the full pricing record to see what fields exist, and the integration test suite still referenced the oldupdateMessageRequestCostfunction that was renamed toupdateMessageRequestCostWithBreakdownin PR #1025.Related PRs:
updateMessageRequestCostmocks toupdateMessageRequestCostWithBreakdownto match the repository function renamed in that PR.Solution
model-price-fieldsutility (src/lib/utils/model-price-fields.ts) — Classifies every field in a price record as "supported" (billing keys the app knows), "unsupported" (generic price-like keys by regex), or "display" (metadata). ProvidesgetEditableExtraPriceData()to extract non-managed fields for round-tripping, andcollectAdditionalPriceLikeNumbers()for price validity checks.extraFieldsJsontextarea so users can view/edit arbitrary extra pricing fields that aren't covered by the basic form. These are spread into thepriceDataobject on upsert.model-price-details-dialog.tsx) — A new "View details" action in the price list dropdown opens a dialog showing all fields organized into Core fields, Additional billable fields, Additional metadata, and Provider pricing nodes (grouped by provider key).price-data.ts) —hasValidPriceDatanow also considers generic price-like numeric fields (matchingcost|price|rate|multiplier|per_|second|session|query|page|pixel|character|dbu) as valid pricing data, so models priced by non-token metrics are recognized as having valid prices.updateMessageRequestCostmock references toupdateMessageRequestCostWithBreakdowninbilling-model-source.test.tsto match the current repository API.Changes
Core Changes
src/lib/utils/model-price-fields.ts(new) — Field classification, editable extras extraction, and price-like key detection utilitiessrc/actions/model-prices.ts— AcceptextraFieldsJson, parse and spread intopriceDataon upsertsrc/lib/utils/price-data.ts— Include generic price-like numbers inhasValidPriceDatachecksrc/app/[locale]/settings/prices/_components/model-price-details-dialog.tsx(new) — Semantic details dialog with field categoriessrc/app/[locale]/settings/prices/_components/model-price-drawer.tsx— Extra fields JSON textarea with pre-fill on editsrc/app/[locale]/settings/prices/_components/price-list.tsx— "View details" menu itemSupporting Changes
messages/*/settings/prices.json(all 5 locales) — i18n strings for advanced fields and details dialogtests/unit/lib/model-price-fields.test.ts(new) — Tests for field classification, editable extras, and price-like detectiontests/unit/lib/price-data-price-like-fields.test.ts(new) — Tests for generic price-like field recognition inhasValidPriceDatatests/unit/settings/prices/model-price-details-dialog.test.tsx(new) — Dialog rendering teststests/unit/settings/prices/model-price-drawer-prefill-and-submit-ui.test.tsx— Tests for extra fields pre-fill and submit round-triptests/unit/actions/model-prices.test.ts— Test for extra JSON fields merge on upserttests/unit/price-sync/cloud-price-table.test.ts— Test for preserving generic pricing fields from TOMLtests/integration/billing-model-source.test.ts— Align mocks with renamedupdateMessageRequestCostWithBreakdownBreaking Changes
None. All changes are additive — existing price records and workflows continue to work unchanged.
Verification
bunx vitest run tests/unit/price-sync/cloud-price-table.test.ts tests/unit/actions/model-prices.test.ts tests/unit/settings/prices/model-price-drawer-prefill-and-submit-ui.test.tsx tests/unit/settings/prices/model-price-details-dialog.test.tsx tests/unit/lib/model-price-fields.test.ts tests/unit/lib/price-data-price-like-fields.test.ts tests/integration/billing-model-source.test.ts --reporter=dot bun run typecheck bun run build bun run test bun run lint -- --max-diagnostics 200Screenshot
Description enhanced by Claude AI
Greptile Summary
This PR preserves extra pricing fields during manual edits by introducing a
model-price-fieldsutility for field classification, adds anextraFieldsJsontextarea to the price drawer for round-tripping non-managed fields, exposes a semantic "View details" dialog for inspecting all price record fields, broadenshasValidPriceDatato recognise generic price-like numerics, and aligns integration test mocks with theupdateMessageRequestCostWithBreakdownrename from PR #1025.Confidence Score: 5/5
Safe to merge; all remaining findings are minor style/consistency P2s that do not affect correctness or data integrity.
Prior P0/P1 concerns (negative-value bypass, server-side managed-field stripping, regex false positives) were surfaced in earlier review rounds and the current state either addresses them or intentionally defers them. The two new findings — duplicate
classifyFieldcall and unconditional core-section rendering — are cosmetic and do not impact runtime behaviour or data correctness.No files require special attention;
src/lib/utils/model-price-fields.tshas the minor duplicate-call nit andmodel-price-details-dialog.tsxhas the inconsistent section guard.Important Files Changed
classifyFieldcall per entry inpushEntriesextraFieldsJson, sanitizes viasanitizeExtraPriceData(prototype-pollution-safe, validates price-like leaf values), spreads before managed fields; server-side does not stripNON_EDITABLE_MANAGED_FIELDSkeys such aspricing(noted in prior review)hasValidPriceDatato delegate tocollectAdditionalPriceLikeNumbers; known cost fields are double-counted withcollectNumericCostsbut harmlessly so due to.some()semanticsextraFieldsJsontextarea with pre-fill viagetEditableExtraPriceDataon edit/prefill; correctly round-trips extra fields through the actionModelPriceDetailsDialogvia standardonSelect-prevents-close +asChildpatternupdateMessageRequestCost→updateMessageRequestCostWithBreakdown; straightforward rename alignmentComments Outside Diff (1)
src/actions/model-prices.ts, line 661-694 (link)NON_EDITABLE_MANAGED_FIELDSfromextraPriceDatabefore the spreadgetEditableExtraPriceDatafilters outNON_EDITABLE_MANAGED_FIELDS(includingpricing) only on the client to pre-populate the textarea. The server never applies the same filter. A user who manually types{"pricing": {"openai": {...}}}in the advanced JSON textarea will persist apricingnode even though the form treatspricingas a managed field it doesn't control. Thepricingkey is not set explicitly later in the spread, so it won't be overridden. Consider stripping managed fields server-side before the merge, or reusinggetEditableExtraPriceDataon the server path.Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (3): Last reviewed commit: "fix(pricing): address second-round revie..." | Re-trigger Greptile