Skip to content

Show DeepSeek Platform usage with cost summary#2270

Merged
steipete merged 3 commits into
steipete:mainfrom
Zihao-Qi:codex/deepseek-cost-summary-display
Jul 18, 2026
Merged

Show DeepSeek Platform usage with cost summary#2270
steipete merged 3 commits into
steipete:mainfrom
Zihao-Qi:codex/deepseek-cost-summary-display

Conversation

@Zihao-Qi

@Zihao-Qi Zihao-Qi commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Summary

  • show DeepSeek Platform cost and token history with Cost summary
  • show DeepSeek details inline for Inline, Submenu, and Both because DeepSeek has no cost submenu
  • preserve Show credits & extra usage as the existing opt-in for Chrome-backed Platform probes
  • keep balance and browser sign-in guidance independent, while limiting detailed usage to the active token account

Why

DeepSeek detailed Platform data is cost and token usage, but its rendering did not follow the Cost summary setting. Users could authenticate successfully and see their balance while the available usage chart remained hidden.

This is a follow-up to #2135.

Consent boundary

DeepSeek detailed usage continues to use the existing selected Chrome session. Both a non-Off Cost summary option and Show credits & extra usage are required to run that Platform usage probe and show its cost and token data inline. Turning either setting off disables detailed usage. Balance and sign-in guidance remain independent.

Validation

  • swift test --filter MenuCardDeepSeekTests (9 tests passed)
  • swift test --filter ProviderSettingsDescriptorTests (36 tests passed)
  • make check (format and lint clean)
  • maintainer autoreview: no findings

Live DeepSeek account verification was unavailable for the maintainer review. Endpoint and source audit confirmed the existing official api.deepseek.com and platform.deepseek.com surfaces remain unchanged.

@Zihao-Qi
Zihao-Qi marked this pull request as ready for review July 17, 2026 19:47
@clawsweeper

clawsweeper Bot commented Jul 17, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed July 17, 2026, 4:41 PM ET / 20:41 UTC.

Summary
Moves DeepSeek Platform token and cost details from the credits/extras gate to Cost summary, rendering the details inline for every enabled DeepSeek cost-summary style.

Reproducibility: no. live current-main reproduction was established here; source inspection gives a high-confidence configuration path where Cost summary is enabled and the former explicit extras opt-in is disabled.

Review metrics: 1 noteworthy metric.

  • Change surface: 3 app files changed; 2 focused test files changed. The patch is narrowly scoped, but all production edits participate in the persisted setting-to-private-session behavior change.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🦪 silver shellfish
Result: blocked until real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Resolve the browser-session consent contract with a maintainer and update the gate or migration accordingly.
  • Attach redacted real-app proof for Cost summary enabled and disabled states; updating the PR body should trigger review, or ask a maintainer to comment @clawsweeper re-review.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR describes local live checks but provides no inspectable redacted after-fix artifact; attach enabled and disabled Cost summary proof from a real app run, then update the PR body for re-review.

Risk before merge

  • [P1] Existing users with Cost summary enabled but the prior credits/extras opt-in disabled may begin a private Chrome-session-backed DeepSeek usage probe after upgrade.
  • [P1] The PR changes the meaning of a persisted setting without an explicit migration, dedicated DeepSeek consent surface, or demonstrated enabled/disabled upgrade behavior.

Maintainer options:

  1. Preserve the current explicit opt-in (recommended)
    Restore the existing browser-session gate for DeepSeek and keep this display change separate from any future consent redesign.
  2. Approve and prove a new consent model
    If Cost summary is intentionally the new consent control, document that migration, preserve an explicit user choice, and provide fresh-install and upgrade proof before merging.

Next step before merge

  • [P1] A maintainer must choose the persisted Cost summary and browser-session consent semantics; contributor proof is also required before merge.

Maintainer decision needed

  • Question: Should an existing generic Cost summary preference count as consent to access the selected Chrome DeepSeek Platform session, or must DeepSeek retain separate explicit browser-session consent?
  • Rationale: This is a persisted privacy and user-expectation contract; tests can verify either implementation but cannot determine which consent model the product intends.
  • Likely owner: steipete — Repository ownership is the strongest available signal for deciding the intended persisted-setting and privacy contract.
  • Options:
    • Retain explicit browser-session consent (recommended): Keep the existing DeepSeek detailed-usage opt-in as the probe gate and defer the Cost summary-only experience until a separate consent design is approved.
    • Approve a dedicated DeepSeek consent migration: Add clearly worded DeepSeek session consent, migration behavior, and enabled/disabled upgrade proof before allowing Cost summary to control rendering.

Security
Needs attention: The patch broadens the persisted setting that authorizes access to selected Chrome DeepSeek Platform-session data without demonstrating an explicit consent migration.

Review findings

  • [P1] Keep browser-session access explicitly opted in — Sources/CodexBar/Providers/Shared/ProviderTokenAccountSelection.swift:27-28
Review details

Best possible solution:

Keep DeepSeek Platform usage behind explicit browser-session consent; after product approval, either preserve the existing opt-in or add a clearly worded DeepSeek-specific consent path with migration and fresh-install and upgrade coverage.

Do we have a high-confidence way to reproduce the issue?

No live current-main reproduction was established here; source inspection gives a high-confidence configuration path where Cost summary is enabled and the former explicit extras opt-in is disabled.

Is this the best way to solve the issue?

No. The display goal is reasonable, but repurposing a generic persisted Cost summary setting as consent for a private Chrome-session probe is not the narrowest safe solution without approved migration semantics.

Full review comments:

  • [P1] Keep browser-session access explicitly opted in — Sources/CodexBar/Providers/Shared/ProviderTokenAccountSelection.swift:27-28
    When showOptionalCreditsAndExtraUsage is false but a Cost summary option is saved, this returns true for DeepSeek and authorizes the selected Chrome-session usage probe. That changes a generic display preference into private browser-session consent for existing setups that intentionally left the explicit extras opt-in disabled. Retain explicit DeepSeek/session consent, or land an approved dedicated consent migration before enabling this path.
    Confidence: 0.96

Overall correctness: patch is incorrect
Overall confidence: 0.96

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 7f8d33f8451c.

Label changes

Label justifications:

  • P1: The change can alter real existing users' browser-session access behavior after upgrade.
  • merge-risk: 🚨 compatibility: A persisted Cost summary preference gains a new side effect for existing DeepSeek configurations.
  • merge-risk: 🚨 auth-provider: The modified gate controls provider-specific detailed usage for the active DeepSeek token account.
  • merge-risk: 🚨 security-boundary: The new setting semantics can trigger access to selected Chrome Platform-session data without the previous explicit gate.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦪 silver shellfish.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR describes local live checks but provides no inspectable redacted after-fix artifact; attach enabled and disabled Cost summary proof from a real app run, then update the PR body for re-review.
Evidence reviewed

Security concerns:

  • [medium] Generic cost preference authorizes Chrome-session access — Sources/CodexBar/Providers/Shared/ProviderTokenAccountSelection.swift:28
    The DeepSeek-specific gate changes from the explicit credits/extras opt-in to costUsageEnabled, allowing a saved generic display preference to start a private browser-backed usage probe after upgrade.
    Confidence: 0.96

What I checked:

  • Changed browser-session gate: The proposed DeepSeek path no longer requires showOptionalCreditsAndExtraUsage; any enabled Cost summary setting now authorizes optional usage handling for DeepSeek. (Sources/CodexBar/Providers/Shared/ProviderTokenAccountSelection.swift:27, 9b3ebfdf79d8)
  • Display setting becomes the detailed-usage switch: DeepSeek treats every enabled Cost summary style as inline dashboard consent because it has no submenu, including the submenu-only setting. (Sources/CodexBar/SettingsStore+TokenCost.swift:6, 9b3ebfdf79d8)
  • Feature provenance: The underlying Chrome-profile-scoped DeepSeek detailed-usage feature was merged through Improve DeepSeek detailed usage #2135 on July 17, 2026; this PR is a follow-up that changes its display and consent gate. (d9cae08c7e69)
  • Real behavior proof remains unavailable: The PR body reports local live validation, but it contains no inspectable redacted screenshot, recording, terminal output, artifact, or runtime log showing the enabled and disabled Cost summary states after this change. (9b3ebfdf79d8)

Likely related people:

  • Zihao-Qi: Authored the merged DeepSeek detailed-usage implementation and this follow-up, so they have direct history on the profile-scoped Platform usage path. (role: introduced merged DeepSeek detailed-usage behavior; confidence: high; commits: d9cae08c7e69; files: Sources/CodexBar/Providers/Shared/ProviderTokenAccountSelection.swift, Sources/CodexBar/SettingsStore+TokenCost.swift)
  • steipete: The unresolved question is the product and privacy meaning of a persisted setting in this repository, rather than a mechanical implementation detail. (role: repository owner and likely product decision owner; confidence: medium; files: Sources/CodexBar/Providers/Shared/ProviderTokenAccountSelection.swift, Sources/CodexBar/SettingsStore+TokenCost.swift)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.
Review history (3 earlier review cycles)
  • reviewed 2026-07-17T19:46:04.767Z sha f307892 :: needs real behavior proof before merge. :: [P3] Remove the release-owned changelog entry
  • reviewed 2026-07-17T19:50:17.424Z sha f307892 :: needs real behavior proof before merge. :: [P1] Preserve the existing browser-session opt-in | [P3] Remove the release-owned changelog entry
  • reviewed 2026-07-17T19:59:11.334Z sha 9b3ebfd :: needs real behavior proof before merge. :: [P1] Preserve the existing browser-session opt-in

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P1 Urgent regression or broken agent/channel workflow affecting real users now. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 auth-provider 🚨 Merging this PR could break OAuth, tokens, provider routing, model choice, or credentials. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. labels Jul 17, 2026

@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: 482bc03c02

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

override.provider == provider
else { return true }
guard provider == .deepseek else { return settings.showOptionalCreditsAndExtraUsage }
guard settings.costUsageEnabled, settings.showOptionalCreditsAndExtraUsage else { return false }

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Stop gating DeepSeek cost usage on extras

When DeepSeek's Cost summary is set to Inline/Submenu/Both but "Show credits & extra usage" is off, this guard still returns false, so ProviderFetchContext.includeOptionalUsage is false and DeepSeekProviderDescriptor.loadUsage skips importing/fetching the Platform usage summary. That leaves the new cost-summary chart and notes unavailable even though Cost summary is enabled, which preserves the user-facing failure this change is meant to move away from; the DeepSeek branch should key off the Cost summary setting and active-account check, not the extras toggle.

Useful? React with 👍 / 👎.

@steipete
steipete merged commit 92362b1 into steipete:main Jul 18, 2026
8 checks passed
@steipete

Copy link
Copy Markdown
Owner

Maintainer verification complete.

  • Reviewed the full untrusted diff and audited the DeepSeek request surfaces. No endpoint changes: detailed usage remains limited to the existing official api.deepseek.com and platform.deepseek.com paths.
  • Added commit 482bc03 to preserve the existing Show credits & extra usage consent boundary alongside Cost summary, including active-account and rendering coverage.
  • swift test --filter MenuCardDeepSeekTests: 9/9 passed.
  • swift test --filter ProviderSettingsDescriptorTests: 36/36 passed.
  • make check: SwiftFormat clean; SwiftLint 0 violations; repository checks passed.
  • Two autoreview passes reported no findings (uncommitted fix, then full branch against origin/main).
  • Exact-head CI: 8/8 checks passed, including both macOS test shards, Linux x64/arm64 builds, lint, aggregate test, and security check.

Live DeepSeek account verification was unavailable; no real session or credentials were used.

Merged as 92362b1.

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

Labels

merge-risk: 🚨 auth-provider 🚨 Merging this PR could break OAuth, tokens, provider routing, model choice, or credentials. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P1 Urgent regression or broken agent/channel workflow affecting real users now. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants