Skip to content

feat(usage): user-configurable per-model cost overlay (providers.<name>.modelCosts) - #1010

Open
harryzhou2000 wants to merge 11 commits into
lidge-jun:devfrom
harryzhou2000:feat/provider-cost-overlay
Open

feat(usage): user-configurable per-model cost overlay (providers.<name>.modelCosts)#1010
harryzhou2000 wants to merge 11 commits into
lidge-jun:devfrom
harryzhou2000:feat/provider-cost-overlay

Conversation

@harryzhou2000

@harryzhou2000 harryzhou2000 commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Closes #1009

Summary

Adds providers.<name>.modelCosts — a per-model display price overlay (USD per
1M tokens) so operators can price internal/custom providers whose ids do not
match the compiled catalogs, or whose actual costs vary from list prices.

"providers": {
  "blsc": {
    "adapter": "openai-chat",
    "baseUrl": "https://llmapi.blsc.cn",
    "modelCosts": {
      "deepseek-v4-flash": { "input": 0.14, "output": 0.28, "cacheRead": 0.0028, "cacheWrite": 0 }
    }
  }
}

Resolution order: user modelCosts → jawcode catalog → expected-price overlay →
model-level vendor fallback. All-zero entries fall through. Display-time
estimation only; never billing.

Changes

  • src/usage/user-cost-overlays.ts (new): registry lifting
    providers.<name>.modelCosts into price-overlay rows; refreshed at
    loadConfig and every persist path; versioned so the estimator memo never
    serves stale rows and edits apply without a restart.
  • src/usage/cost.ts: user overlay wins in resolveMatchedPrice (exact
    provider/model match, source: "user"); memo key now includes the overlay
    version. Also replaces a stray NUL byte in the memo cache-key template
    literal with a space separator.
  • src/config.ts: providerModelCostsConfigError validation (non-negative
    finite 4-tuples) wired into the load-time schema refine.
  • src/server/auth-cors.ts: management API rejects malformed modelCosts;
    safeConfigDTO exposes the field.
  • src/server/management/shared.ts: new provider_cost_overlay estimate
    reason.
  • gui/: reason label + i18n strings (en/de/ja/ko/ru/zh).
  • Docs row in the providers reference.

UI change

Logs detail dialog showing the provider-configured price overlay

Tests

  • Usage-cost: user overlay beats jawcode/expected, custom providers resolve only
    via overlay, all-zero fall-through, combo fail-closed, registry refresh/memo
    invalidation.
  • Config: validation paths, load/save round-trip, registry refresh, management
    validation, DTO passthrough.

bun run typecheck, GUI eslint, and the touched suites pass (91 tests).

Summary by CodeRabbit

  • New Features

    • Added optional provider-specific pricing for exact models, covering input, output, cache reads, and cache writes.
    • Logs estimates prioritize configured prices and identify when overrides are applied.
    • Supports custom models; invalid entries are safely rejected or ignored, while all-zero entries fall back to other pricing sources.
    • Configured pricing is display-only and does not affect billing.
  • Documentation

    • Documented configuration, validation, precedence, and fallback behavior.
  • Localization

    • Added translated labels for provider-configured pricing indicators.

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

  • All CI tests are green on my local testing.

  • I pushed my PR to the latest dev commit.

  • I fixed all correct Codex and CodeRabbit findings.

  • My PR is ready for review.

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

PR quality gates passed

This pull request now targets dev with acceptable ancestry, description, and UI screenshot coverage. The review readiness checklist is complete.

The title was left unchanged. Its existing draft status has been preserved.

@github-actions github-actions Bot changed the title feat(usage): user-configurable per-model cost overlay (providers.<name>.modelCosts) [WRONG BRANCH] feat(usage): user-configurable per-model cost overlay (providers.<name>.modelCosts) Aug 4, 2026
@github-actions
github-actions Bot marked this pull request as draft August 4, 2026 16:31
@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds providers.<name>.modelCosts for exact model IDs. Validated prices populate a versioned runtime registry, override built-in cost sources, invalidate stale usage summaries, and update after configuration loads or persists. Logs reports provider-overlay estimates with localized labels.

Changes

Provider cost overlay

Layer / File(s) Summary
Overlay contracts and validation
src/types.ts, src/config.ts, src/server/auth-cors.ts, docs-site/src/content/docs/.../providers.md, tests/provider-cost-overlay-config.test.ts
Defines four per-million-token rates, validates non-negative finite values, sanitizes management DTOs, and documents display-only behavior. Tests cover invalid structures, persistence, management validation, and DTO serialization.
Registry synchronization
src/usage/user-cost-overlays.ts, src/config.ts, tests/provider-cost-overlay-config.test.ts
Builds active overlay rows from provider configuration and refreshes them after configuration loads, repairs, fallbacks, diagnostics, and persistence.
Cost resolution and caching
src/usage/cost.ts, src/server/management/usage-summary-cache.ts, src/server/management/logs-usage-routes.ts, tests/usage-cost.test.ts, tests/api-usage.test.ts
Uses user overlays before built-in sources, includes registry versioning in memoization and usage-summary cache validation, propagates overlays through estimators, and preserves zero-price fallback.
Logs attribution and localization
src/server/management/shared.ts, gui/src/pages/Logs.tsx, gui/src/i18n/*.ts
Records provider_cost_overlay, maps the user price source, and adds localized Logs detail messages.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ConfigLoader
  participant UserCostOverlays
  participant CostEstimator
  participant UsageSummaryCache
  participant Logs
  ConfigLoader->>UserCostOverlays: refreshUserCostOverlays(config)
  UserCostOverlays-->>CostEstimator: active overlays and version
  CostEstimator->>CostEstimator: resolve user price before built-in prices
  CostEstimator-->>UsageSummaryCache: store estimate with overlay version
  UsageSummaryCache->>UsageSummaryCache: reject stale overlay versions
  CostEstimator-->>Logs: estimate with source "user"
  Logs-->>Logs: map provider_cost_overlay to localized detail
Loading

Possibly related PRs

Suggested labels: intake: hygiene-blocked

Suggested reviewers: ingwannu, lidge-jun, wibias

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The implementation covers issue #1009 requirements, including validation, precedence, fall-through, registry refresh, API exposure, localization, documentation, and tests.
Out of Scope Changes check ✅ Passed The configuration, API, estimation, caching, GUI, documentation, and test changes directly support the linked issue objectives.
Docstring Coverage ✅ Passed Docstring coverage is 86.21% which is sufficient. The required threshold is 80.00%.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely identifies the main change: user-configurable per-model cost overlays through providers..modelCosts.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs-site/src/content/docs/reference/configuration/providers.md`:
- Line 69: Update the `modelCosts` documentation on line 69 to explicitly
clarify that custom or internal provider model IDs—even those absent from
built-in catalogs—are valid overlay keys for cost configuration. Add a note
stating that users can define custom prices for any model identifier regardless
of catalog presence, making clear that the feature supports providers beyond the
standard built-in offerings.
🪄 Autofix

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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: d0d55d83-385c-4beb-b349-119980acd24b

📥 Commits

Reviewing files that changed from the base of the PR and between f9b9440 and afc131d.

📒 Files selected for processing (16)
  • docs-site/src/content/docs/reference/configuration/providers.md
  • gui/src/i18n/de.ts
  • gui/src/i18n/en.ts
  • gui/src/i18n/ja.ts
  • gui/src/i18n/ko.ts
  • gui/src/i18n/ru.ts
  • gui/src/i18n/zh.ts
  • gui/src/pages/Logs.tsx
  • src/config.ts
  • src/server/auth-cors.ts
  • src/server/management/shared.ts
  • src/types.ts
  • src/usage/cost.ts
  • src/usage/user-cost-overlays.ts
  • tests/provider-cost-overlay-config.test.ts
  • tests/usage-cost.test.ts

Add providers.<name>.modelCosts (model -> input/output/cacheRead/
cacheWrite in USD per 1M tokens) so operators can price internal/custom
providers whose ids do not match the compiled catalogs, or whose actual
costs vary from list prices.

- modelCosts rows win over the jawcode catalog and the expected-price
  overlay in resolveMatchedPrice (exact provider/model match; all-zero
  entries fall through to the catalogs). Follows ocx's flat per-model
  config convention (models, modelContextWindows, ...).
- Rows are lifted from config at loadConfig and every persist path into
  a versioned registry; the estimator memo keys on that version so edits
  apply immediately and stale rows are never served.
- New CostResult reason provider_cost_overlay is surfaced in the GUI
  logs detail with i18n strings.
- config.json validation accepts only non-negative finite 4-tuples; the
  management API rejects malformed overlays; safeConfigDTO exposes the
  field to the dashboard.
- Also replace a stray NUL byte in the price-memo cache-key template
  literal with a space separator.
- Tests cover precedence, fall-through, registry refresh/memo
  invalidation, config round-trip, management validation, and DTO
  passthrough.
@harryzhou2000
harryzhou2000 force-pushed the feat/provider-cost-overlay branch from afc131d to 180b8b4 Compare August 4, 2026 16:41
@harryzhou2000
harryzhou2000 changed the base branch from main to dev August 4, 2026 16:42
@github-actions github-actions Bot changed the title [WRONG BRANCH] feat(usage): user-configurable per-model cost overlay (providers.<name>.modelCosts) feat(usage): user-configurable per-model cost overlay (providers.<name>.modelCosts) Aug 4, 2026
@github-actions
github-actions Bot marked this pull request as ready for review August 4, 2026 16:42
@github-actions github-actions Bot added the enhancement New feature or request label Aug 4, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs-site/src/content/docs/reference/configuration/providers.md`:
- Line 75: Update the modelCosts documentation to state the complete
display-price fallback order: user configuration, jawcode catalog,
expected-price overlay, then model-level vendor fallback. Clarify that an
all-zero user entry falls through to the next source in that sequence, while
preserving the existing display-only and non-billing behavior.
- Line 75: Add the modelCosts configuration row after modelMaxOutputTokens in
the Japanese, Korean, Russian, and Simplified Chinese provider pages,
documenting exact model-ID keys, all-zero entries falling back to built-in
catalogs, and display-only estimates that never affect billing.

In `@gui/src/i18n/de.ts`:
- Line 574: Update the German translation value for
logs.detail.estimate.provider_cost_overlay to “Ein vom Anbieter konfiguriertes
Preis-Overlay wurde verwendet.”, preserving alignment with the English catalog
entry.

In `@gui/src/pages/Logs.tsx`:
- Around line 45-49: Extend the GUI price-source contract used by
MatchedPriceInfo to include "user", then add the logs.detail.source.user
translation key to every locale alongside the existing source labels so
provider-configured overlay-priced rows display a localized source.
🪄 Autofix

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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: a4b0f2de-a70c-4109-a5d9-f87dcbe73c8d

📥 Commits

Reviewing files that changed from the base of the PR and between afc131d and 180b8b4.

📒 Files selected for processing (8)
  • docs-site/src/content/docs/reference/configuration/providers.md
  • gui/src/i18n/de.ts
  • gui/src/i18n/en.ts
  • gui/src/i18n/ja.ts
  • gui/src/i18n/ko.ts
  • gui/src/i18n/ru.ts
  • gui/src/i18n/zh.ts
  • gui/src/pages/Logs.tsx

Comment thread docs-site/src/content/docs/reference/configuration/providers.md Outdated
Comment thread gui/src/i18n/de.ts Outdated
Comment thread gui/src/pages/Logs.tsx

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 180b8b4326

ℹ️ 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".

Comment thread gui/src/pages/Logs.tsx
Comment thread src/config.ts Outdated
- Add JSDoc to the functions touched by this PR (loadConfig,
  persistConfigUnlocked, saveConfig, withRefreshedCostOverlays,
  safeConfigDTO, costResult, resolveMatchedPriceInner/Exact,
  estimateAttemptCost, validCost4, Logs key helpers) to satisfy the
  docstring-coverage gate.
- GUI: MatchedPriceInfo.source accepts "user"; add
  logs.detail.source.user to all six locales so user-priced rows render a
  label instead of a raw key.
- de.ts: fix provider_cost_overlay grammar (Ein vom Anbieter
  konfiguriertes Preis-Overlay ...).
- safeConfigDTO: serialize only the four rate fields of modelCosts rows so
  extra hand-edited fields cannot leak to the dashboard; regression test
  added.
- Docs: document the complete fallback order (user modelCosts -> jawcode
  catalog -> expected-price overlay -> vendor fallback) with all-zero
  fall-through in the providers reference, and add the modelCosts row to
  the ja/ko/ru/zh-cn provider pages.
@harryzhou2000

Copy link
Copy Markdown
Contributor Author

Thanks for the review — addressed in 7a4ce62:

  • Docstring coverage: JSDoc added to every function touched by this PR (loadConfig, persistConfigUnlocked, saveConfig, withRefreshedCostOverlays, safeConfigDTO, costResult, resolveMatchedPriceInner/Exact, estimateAttemptCost, validCost4, the Logs key helpers, plus the new DTO sanitizers). A local AST replica now reports 19/19 = 100% on the PR's new/modified functions.
  • Inline findings: GUI price-source contract extended with user and logs.detail.source.user in all six locales; German grammar fix; complete fallback order documented with the modelCosts row added to ja/ko/ru/zh-cn pages; safeConfigDTO serializes only the four rate fields per cost row (regression test added).
  • Verified: root typecheck clean, GUI eslint clean, 66 tests pass in the touched suites (usage-cost + model-costs config + management metrics + routing).

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs-site/src/content/docs/ja/reference/configuration/providers.md`:
- Line 68: Update the Cost4 documentation row in
docs-site/src/content/docs/ja/reference/configuration/providers.md:68-68 to list
the supported input, output, cacheRead, and cacheWrite rate fields. Apply the
equivalent field list or example to
docs-site/src/content/docs/ko/reference/configuration/providers.md:68-68,
docs-site/src/content/docs/ru/reference/configuration/providers.md:78-78, and
docs-site/src/content/docs/zh-cn/reference/configuration/providers.md:67-67,
preserving each locale’s language and existing pricing guidance.

In `@gui/src/i18n/ru.ts`:
- Line 566: Update the Russian translation for
logs.detail.estimate.provider_cost_overlay at line 592 to reuse the same
terminology from logs.detail.source.user at line 566. Replace the current
"настроенная пользователем цена провайдера" phrasing with a sentence form that
incorporates "Ценовой оверлей провайдера" (keeping the "оверлей" term
consistent). This aligns the Russian translations with every other locale in the
PR, which reuses an identical term across both the price-source label and the
estimate-reason explanation.
🪄 Autofix

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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: a3d12d7c-bcdf-4344-b9a0-61f4e65190d7

📥 Commits

Reviewing files that changed from the base of the PR and between 180b8b4 and 7a4ce62.

📒 Files selected for processing (18)
  • docs-site/src/content/docs/ja/reference/configuration/providers.md
  • docs-site/src/content/docs/ko/reference/configuration/providers.md
  • docs-site/src/content/docs/reference/configuration/providers.md
  • docs-site/src/content/docs/ru/reference/configuration/providers.md
  • docs-site/src/content/docs/zh-cn/reference/configuration/providers.md
  • gui/src/i18n/de.ts
  • gui/src/i18n/en.ts
  • gui/src/i18n/ja.ts
  • gui/src/i18n/ko.ts
  • gui/src/i18n/ru.ts
  • gui/src/i18n/zh.ts
  • gui/src/pages/Logs.tsx
  • src/config.ts
  • src/server/auth-cors.ts
  • src/server/management/shared.ts
  • src/usage/cost.ts
  • src/usage/user-cost-overlays.ts
  • tests/provider-cost-overlay-config.test.ts

Comment thread docs-site/src/content/docs/ja/reference/configuration/providers.md Outdated
Comment thread gui/src/i18n/ru.ts
…ology

- ja/ko/ru/zh-cn provider references now list the four Cost4 rate fields
  (input, output, cacheRead, cacheWrite) with a JSON example.
- ru.ts: logs.detail.estimate.provider_cost_overlay reuses the same
  "ценовой оверлей провайдера" term as logs.detail.source.user.
@github-actions github-actions Bot added the intake: hygiene-blocked Deterministic PR hygiene checks failed label Aug 5, 2026
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown

⚠️ Deterministic hygiene checks failed.

  • unsponsored_surface — This changes an authentication, workflow, release-automation, or dependency surface. MAINTAINERS.md requires security review for these; ask a maintainer to apply maintainer-sponsored once they have reviewed it. Paths: src/server/auth-cors.ts.

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown

Review readiness checklist

This PR is kept in draft until every requirement below is fulfilled. The tickable checklist has been added to your PR description — tick all four boxes there.

  • ✅ All CI tests are green on my local testing.
  • ✅ I pushed my PR to the latest dev commit.
  • ✅ I fixed all correct Codex and CodeRabbit findings.
  • ✅ My PR is ready for review.

4/4 boxes ticked.

All four boxes are ticked.
Completed against head 7dda694; new commits after this will reset the checklist.
This pull request has been marked Ready for Review.
Maintainers notified: @lidge-jun @Ingwannu @Wibias

@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@github-actions
github-actions Bot marked this pull request as draft August 5, 2026 10:27

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/server/auth-cors.ts`:
- Around line 525-539: Update sanitizeModelCosts to create out with a null
prototype so assigning the "__proto__" model ID creates an own property and
Object.keys includes it. Add a regression test using JSON.parse() to construct
the "__proto__" key and verify safeConfigDTO exposes its valid cost row.

In `@tests/usage-cost.test.ts`:
- Around line 769-812: Move the empty-provider cleanup for user cost overlays
out of the test body and into an afterEach hook scoped to the surrounding
describe block. Ensure the hook calls refreshUserCostOverlays with an empty
providers object so the blsc overlay is reset even when assertions in “registry
refresh replaces rows, bumps the version, and invalidates the memo” fail, and
update the bun:test imports to include afterEach.
🪄 Autofix

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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: d8697a9c-f003-4c14-ae6a-70292ae50aef

📥 Commits

Reviewing files that changed from the base of the PR and between 8034cd7 and f620f7f.

📒 Files selected for processing (20)
  • docs-site/src/content/docs/ja/reference/configuration/providers.md
  • docs-site/src/content/docs/ko/reference/configuration/providers.md
  • docs-site/src/content/docs/reference/configuration/providers.md
  • docs-site/src/content/docs/ru/reference/configuration/providers.md
  • docs-site/src/content/docs/zh-cn/reference/configuration/providers.md
  • gui/src/i18n/de.ts
  • gui/src/i18n/en.ts
  • gui/src/i18n/ja.ts
  • gui/src/i18n/ko.ts
  • gui/src/i18n/ru.ts
  • gui/src/i18n/zh.ts
  • gui/src/pages/Logs.tsx
  • src/config.ts
  • src/server/auth-cors.ts
  • src/server/management/shared.ts
  • src/types.ts
  • src/usage/cost.ts
  • src/usage/user-cost-overlays.ts
  • tests/provider-cost-overlay-config.test.ts
  • tests/usage-cost.test.ts

Comment thread src/server/auth-cors.ts
Comment thread tests/usage-cost.test.ts
@harryzhou2000

Copy link
Copy Markdown
Contributor Author

@coderabbitai resume — the dev merge (f620f7f) resolved the config.ts conflict (persistConfigUnlocked now returns boolean for generation bumping while still refreshing the cost-overlay registry). Please run the full review on the merged state.

@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

@harryzhou2000 I will resume review and perform a full review of the merged state, including the persistConfigUnlocked generation and cost-overlay refresh behavior.

✅ Action performed

Reviews resumed.


Full review finished.

@harryzhou2000

Copy link
Copy Markdown
Contributor Author

Heads-up for review: the hygiene gate flags src/server/auth-cors.ts as an auth surface (unsponsored_surface) because this PR adds management-API validation for modelCosts and exposes the field in safeConfigDTO. The change is read/validation-only — no credential handling changes — but per MAINTAINERS.md it needs the maintainer-sponsored label once reviewed. Could you apply it when you take a look?

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
src/server/auth-cors.ts (1)

525-539: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Preserve the valid __proto__ model ID.

Line 527 creates a normal object. providerModelCostsConfigError accepts "__proto__" because it is a nonblank model ID. For a row parsed from JSON, Line 536 changes out's prototype instead of adding an own property. Object.keys(out) then omits the row, so safeConfigDTO does not expose a valid configured price.

Create out with a null prototype. Add a regression test that builds the key through JSON.parse().

Proposed fix
-  const out: Record<string, ProviderCostOverlay> = {};
+  const out: Record<string, ProviderCostOverlay> = Object.create(null);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/server/auth-cors.ts` around lines 525 - 539, Update sanitizeModelCosts to
create out with a null prototype so assigning the valid "__proto__" model ID
produces an own property and remains included in the returned configuration. Add
a regression test that obtains this key via JSON.parse() and verifies the
sanitized costs preserve the valid entry.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/provider-cost-overlay-config.test.ts`:
- Around line 26-30: Reset the user overlay registry in both affected test
sites: in tests/provider-cost-overlay-config.test.ts lines 26-30, call
refreshUserCostOverlays({ providers: {} } as OcxConfig) in the existing
afterEach before deleting OPENCODEX_HOME; in tests/usage-cost.test.ts lines
769-812, add a scoped afterEach that performs the same reset and remove the
test-body cleanup near line 807.

---

Duplicate comments:
In `@src/server/auth-cors.ts`:
- Around line 525-539: Update sanitizeModelCosts to create out with a null
prototype so assigning the valid "__proto__" model ID produces an own property
and remains included in the returned configuration. Add a regression test that
obtains this key via JSON.parse() and verifies the sanitized costs preserve the
valid entry.
🪄 Autofix

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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: a262235e-fcc4-491e-a174-d1c80bdad0fb

📥 Commits

Reviewing files that changed from the base of the PR and between 8034cd7 and f620f7f.

📒 Files selected for processing (20)
  • docs-site/src/content/docs/ja/reference/configuration/providers.md
  • docs-site/src/content/docs/ko/reference/configuration/providers.md
  • docs-site/src/content/docs/reference/configuration/providers.md
  • docs-site/src/content/docs/ru/reference/configuration/providers.md
  • docs-site/src/content/docs/zh-cn/reference/configuration/providers.md
  • gui/src/i18n/de.ts
  • gui/src/i18n/en.ts
  • gui/src/i18n/ja.ts
  • gui/src/i18n/ko.ts
  • gui/src/i18n/ru.ts
  • gui/src/i18n/zh.ts
  • gui/src/pages/Logs.tsx
  • src/config.ts
  • src/server/auth-cors.ts
  • src/server/management/shared.ts
  • src/types.ts
  • src/usage/cost.ts
  • src/usage/user-cost-overlays.ts
  • tests/provider-cost-overlay-config.test.ts
  • tests/usage-cost.test.ts

Comment thread tests/provider-cost-overlay-config.test.ts
… isolation

- sanitizeModelCosts builds the DTO map with a null prototype so a model id
  literally named __proto__ stays an own row instead of mutating the map's
  prototype and vanishing from Object.keys (CodeRabbit minor).
- usage-cost and provider-cost-overlay-config tests reset the module-level
  overlay registry in afterEach so rows cannot leak across test files in a
  shared-process run (CodeRabbit stability).
- Regression test: safeConfigDTO keeps a __proto__ model id as an own row.
@github-actions
github-actions Bot marked this pull request as ready for review August 5, 2026 11:46

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c5db9f883f

ℹ️ 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".

Comment thread src/usage/user-cost-overlays.ts Outdated
Comment thread src/config.ts
Codex review findings:

- refreshUserCostOverlays now copies ONLY the four validated rate fields
  into the overlay row, so a hand-edited row carrying extra properties
  (e.g. a misplaced apiKey) can no longer leak through /api/logs display
  estimates (P1).
- loadConfig/configDiagnosticsFromRaw run sanitizeModelCostsForLoad before
  schema validation: malformed display-price rows are dropped with a warning
  instead of failing the whole parse and falling back to defaults, which
  previously discarded otherwise valid providers. Strict rejection stays at
  the management/write boundary (P2).

Regression tests: malformed row degradation, non-object modelCosts drop, and
registry rows containing only the four rate fields.
@github-actions
github-actions Bot marked this pull request as draft August 5, 2026 13:06
@github-actions
github-actions Bot marked this pull request as ready for review August 5, 2026 13:09

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

const userOverlay = findExpectedPriceOverlay(provider, modelId, userOverlays);

P2 Badge Try exact provider names before collapsing user overlays

When a valid custom provider name happens to end with the Codex account-label suffix pattern, such as blsc-pabcdef, resolveMatchedPrice() has already collapsed it to blsc before this new user-overlay lookup runs, while refreshUserCostOverlays() stores the configured row under the exact provider name. In that scenario a valid providers.blsc-pabcdef.modelCosts.custom-model entry is never matched, so Logs/usage estimates stay unpriced despite the explicit override; try the exact provider against userOverlays before applying baseProviderLabel, or register both exact and collapsed keys.

ℹ️ 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".

Comment thread src/config.ts Outdated
Comment thread src/config.ts
A custom provider whose name ends with the Codex account-log-label suffix
pattern (e.g. blsc-pabcdef) had its provider collapsed by baseProviderLabel
before the user-overlay lookup, so providers.<name>.modelCosts rows stored
under the exact name never matched and estimates stayed unpriced. resolveMatchedPrice now checks the exact provider against the user overlay first, then collapses for the compiled catalogs.
@harryzhou2000

Copy link
Copy Markdown
Contributor Author

Addressed in 0e37a89: resolveMatchedPrice now tries the exact provider name against the user overlay BEFORE applying baseProviderLabel, so a custom provider whose name legitimately ends with the account-label suffix pattern (e.g. blsc-pabcdef) matches its own providers.<name>.modelCosts rows. The collapse still applies for the compiled jawcode/expected catalogs. Regression test: user overlay matches an exact provider name ending with an account-label suffix (73/73 tests pass, typecheck clean).

@github-actions
github-actions Bot marked this pull request as draft August 5, 2026 13:39
@github-actions
github-actions Bot marked this pull request as ready for review August 5, 2026 13:41
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex review is not enabled for this repo. Please contact the admins of this repo to enable Codex.

…y changes

- providerModelCostsConfigError now JSON-quotes and redacts secret-shaped
  model ids so a malformed write cannot echo a pasted key/secret back
  through the management API.
- /api/usage summaries are cached against userCostOverlayVersion(): an
  overlay save invalidates the entry even when the usage log is unchanged.
- refreshUserCostOverlays is a no-op when the extracted rows are
  byte-identical, so reloads of an unchanged config (server start,
  migrations, persist paths) no longer churn the version, invalidate the
  summary cache, or thash the cost memo.
- tests: redaction + quoted-key assertions, cache-invalidation test, and
  an identical-refresh no-op regression test.
@github-actions
github-actions Bot marked this pull request as draft August 5, 2026 14:44

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/api-usage.test.ts`:
- Around line 171-185: Update the test’s finally block to clear the module-level
cost overlay registry before calling server.stop(true), using the existing
overlay-reset mechanism. Keep this cleanup inside finally so it executes even
when assertions or server shutdown fail, and target the registry installed by
refreshUserCostOverlays.
🪄 Autofix

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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: f7f99109-da20-4a5b-aaba-4eb52efe10ba

📥 Commits

Reviewing files that changed from the base of the PR and between 0e37a89 and 724a439.

📒 Files selected for processing (7)
  • src/config.ts
  • src/server/management/logs-usage-routes.ts
  • src/server/management/usage-summary-cache.ts
  • src/usage/user-cost-overlays.ts
  • tests/api-usage.test.ts
  • tests/provider-cost-overlay-config.test.ts
  • tests/usage-cost.test.ts

Comment thread tests/api-usage.test.ts
…t finally block

The test installs a module-level blsc overlay; reset it to empty before
stopping the server so a later test (or an assertion/shutdown failure)
cannot resolve user-configured prices unexpectedly.
@github-actions
github-actions Bot marked this pull request as ready for review August 5, 2026 14:54

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 66fc6a3d4f

ℹ️ 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".

Comment thread src/server/auth-cors.ts Outdated
providerManagementConfigError echoed the caller-controlled provider name
verbatim in the modelCosts error path even though the route has not yet
validated/sanitized it. JSON-quote and redact the name (same rule as the
retryOn429 branch) so a token-shaped provider name cannot serialize back
through the management API. Regression test added.
@github-actions
github-actions Bot marked this pull request as draft August 5, 2026 15:15
@github-actions
github-actions Bot marked this pull request as ready for review August 5, 2026 16:00
@harryzhou2000

Copy link
Copy Markdown
Contributor Author

Hi @lidge-jun — just a gentle follow-up whenever you have a moment. The PR is marked ready and everything on our side is green: all review threads (CodeRabbit + Codex connector) are resolved, and CodeRabbit, enforce-target, and label checks pass.

The one remaining gate is the hygiene check's unsponsored_surface on the small auth-surface change in src/server/auth-cors.ts (management-API validation for modelCosts — provider names and model ids in validation errors are now redacted). Per MAINTAINERS.md that needs your security review; if you're comfortable with it, applying maintainer-sponsored would clear the check. Happy to adjust anything you'd like. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request intake: hygiene-blocked Deterministic PR hygiene checks failed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(usage): user-configurable per-model cost overlay (providers.<name>.modelCosts)

1 participant