Skip to content

Improve DeepSeek detailed usage#2216

Closed
joeVenner wants to merge 4 commits into
steipete:mainfrom
joeVenner:codex/deepseek-detailed-usage
Closed

Improve DeepSeek detailed usage#2216
joeVenner wants to merge 4 commits into
steipete:mainfrom
joeVenner:codex/deepseek-detailed-usage

Conversation

@joeVenner

Copy link
Copy Markdown
Contributor

Summary

  • Adds DeepSeek Platform detailed usage support from Improve DeepSeek detailed usage #2135.
  • Supports browser-only DeepSeek usage and keeps detailed usage separate from API-key balance fetching.

Verification

  • swift test --filter DeepSeekUsageFetcherTests

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P2 Normal priority bug or improvement with limited blast radius. 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 16, 2026
@clawsweeper

clawsweeper Bot commented Jul 16, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed July 16, 2026, 9:15 AM ET / 13:15 UTC.

Summary
Adds DeepSeek Platform detailed usage, automatic Chrome-session/profile discovery, browser-only balance support, profile-scoped settings, refresh-state presentation, localization, documentation, and focused tests.

Reproducibility: not applicable. This PR adds and extends DeepSeek authentication and usage capabilities rather than fixing a clearly reproduced current-main bug. The relevant review path is fresh-install, upgrade, and real browser-session validation.

Review metrics: 2 noteworthy metrics.

  • Patch surface: 43 files; +3,593 / -187. The change crosses provider networking, persistent config, UI state, localization, docs, and tests, so focused tests alone do not settle integration and upgrade behavior.
  • Validation surface: 8 test files; 1 command reported. The branch adds substantial unit coverage, but the current PR body reports only one focused test command and no live browser-only run.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🦐 gold shrimp
Patch quality: 🐚 platinum hermit
Result: blocked until stronger 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:

  • [P1] Add redacted live output or a recording showing DeepSeek balance and detailed usage with no API key configured.
  • Demonstrate switching API credentials and Chrome profiles without stale or cross-account usage appearing.
  • Document whether legacy stored platform tokens remain supported and confirm newly imported Chrome tokens are never persisted.

Proof guidance:

  • [P1] Needs stronger real behavior proof before merge: Linked screenshots and UI notes from the predecessor PR show detailed usage and profile switching, but this head still needs redacted live proof of browser-only balance with no API key and cross-account/profile isolation; updating the PR body should trigger re-review, or a maintainer can request @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

  • [P1] A legacy DeepSeek Platform session token can remain stored in provider config and be projected into the fetch environment; maintainers must confirm that compatibility tradeoff is acceptable for a browser-auth credential.
  • [P1] The linked screenshots do not demonstrate the newly added browser-only balance path without an API key or prove that switching API credentials and Chrome profiles cannot surface another account's usage.
  • [P1] This PR and Improve DeepSeek detailed usage #2135 remain overlapping open landing candidates, so review fixes, authorship credit, and security decisions could diverge unless one branch is selected as canonical.

Maintainer options:

  1. Prove and document the compatibility path (recommended)
    Keep the legacy read path, add redacted fresh-install and upgrade proof for browser-only and account switching, and document that new Chrome tokens are never persisted.
  2. Remove legacy token persistence
    Normalize away the stored DeepSeek platform token and provide explicit recovery guidance for users who must sign in through Chrome again.
  3. Pause for credential-model review
    Do not merge either overlapping PR until maintainers choose the permanent DeepSeek session-storage and profile-scoping contract.

Next step before merge

  • [P1] A maintainer should select the canonical DeepSeek PR, decide the legacy session-token boundary, and require fresh browser-only/account-isolation proof before merge.

Maintainer decision needed

  • Question: Should DeepSeek browser-only support retain legacy platform tokens from provider config for upgrade compatibility, or enforce an ephemeral-only Chrome-session model?
  • Rationale: The branch intentionally preserves and rehydrates a sensitive legacy session token while automatic imports remain in memory; tests cannot decide whether that persistence boundary matches the repository's desired privacy and compatibility contract.
  • Likely owner: steipete — As repository owner and release integrator for the existing DeepSeek feature, steipete is the strongest available owner for the privacy-versus-upgrade compatibility choice.
  • Options:
    • Preserve legacy compatibility (recommended): Accept read-only legacy token support, prohibit writing newly imported tokens, document the storage boundary, and require live upgrade/account-isolation proof.
    • Migrate to ephemeral sessions: Remove stored legacy platform tokens during normalization and require signed-in Chrome discovery, with clear upgrade guidance for affected users.
    • Narrow the feature: Keep API-key balance plus optional detailed usage but defer browser-only balance until the credential model is settled.

Security
Needs attention: No malicious or third-party supply-chain change is evident, but the legacy DeepSeek session-token persistence boundary needs explicit maintainer approval and upgrade proof.

Review details

Best possible solution:

Land one canonical DeepSeek branch that keeps newly imported Chrome tokens ephemeral, explicitly documents or migrates legacy stored tokens, and includes redacted live proof for no-API-key balance plus multi-profile account isolation before closing the overlapping PR.

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

Not applicable: this PR adds and extends DeepSeek authentication and usage capabilities rather than fixing a clearly reproduced current-main bug. The relevant review path is fresh-install, upgrade, and real browser-session validation.

Is this the best way to solve the issue?

Unclear: the defensive fetch design and profile-scoping tests are strong, but the permanent legacy-token persistence boundary and which overlapping PR should land require maintainer confirmation.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 6d71af30b84d.

Label changes

Label changes:

  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: Linked screenshots and UI notes from the predecessor PR show detailed usage and profile switching, but this head still needs redacted live proof of browser-only balance with no API key and cross-account/profile isolation; updating the PR body should trigger re-review, or a maintainer can request @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • remove proof: sufficient: Current real behavior proof status is insufficient, not sufficient.
  • remove status: ⏳ waiting on author: Current PR status label is status: 📣 needs proof.

Label justifications:

  • P2: This is a substantial but bounded DeepSeek provider improvement with limited provider-specific blast radius and no demonstrated urgent regression.
  • merge-risk: 🚨 compatibility: The branch adds persisted profile fields and preserves a legacy platform-token path whose upgrade behavior must remain intentional.
  • merge-risk: 🚨 auth-provider: Automatic Chrome-session selection and API-credential-scoped profiles can misassociate or break DeepSeek authentication if the runtime assumptions are wrong.
  • merge-risk: 🚨 security-boundary: The diff reads sensitive Chrome local-storage sessions and retains a legacy session-token configuration path that requires explicit privacy approval.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: Linked screenshots and UI notes from the predecessor PR show detailed usage and profile switching, but this head still needs redacted live proof of browser-only balance with no API key and cross-account/profile isolation; updating the PR body should trigger re-review, or a maintainer can request @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

Security concerns:

  • [medium] Confirm legacy platform-token persistence — Sources/CodexBarCore/Config/ProviderConfigEnvironment.swift:117
    This branch preserves provider cookieHeader as a DeepSeek bearer session token and projects it into the fetch environment. Because that credential can outlive the browser session in config, maintainers should explicitly approve the compatibility boundary and verify restrictive storage, redaction, and non-persistence of newly imported tokens.
    Confidence: 0.92

What I checked:

Likely related people:

  • steipete: The repository and v0.30.0 release containing the existing DeepSeek detailed-usage feature are owned and released by steipete, making this the strongest decision owner for the credential and canonical-PR boundary. (github.com) (role: repository owner and release integrator; confidence: high; commits: 81cf30a; files: Sources/CodexBarCore/Providers/DeepSeek/DeepSeekProviderDescriptor.swift, Sources/CodexBarCore/Providers/DeepSeek/DeepSeekUsageFetcher.swift, docs/deepseek.md)
  • Yuxin-Qiao: Release history credits Yuxin-Qiao for the shipped DeepSeek optional web-session usage and cost summaries in DeepSeek: add web-session usage and cost summary #1166. (github.com) (role: existing detailed-usage feature introducer; confidence: high; files: Sources/CodexBarCore/Providers/DeepSeek/DeepSeekUsageFetcher.swift, Sources/CodexBarCore/Providers/DeepSeek/DeepSeekUsageCostParser.swift, Tests/CodexBarTests/DeepSeekUsageFetcherTests.swift)
  • willytop8: Release history credits willytop8 for the initial DeepSeek provider and token-account balance support in Add DeepSeek provider support #811. (github.com) (role: original DeepSeek provider contributor; confidence: medium; files: Sources/CodexBarCore/Providers/DeepSeek/DeepSeekProviderDescriptor.swift, Sources/CodexBarCore/Providers/DeepSeek/DeepSeekUsageFetcher.swift, docs/deepseek.md)
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 (1 earlier review cycle)
  • reviewed 2026-07-16T12:32:45.108Z sha 40f0f1d :: needs maintainer review before merge. :: none

@clawsweeper clawsweeper Bot added status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed proof: sufficient Contributor real behavior proof is sufficient. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jul 16, 2026
@steipete

Copy link
Copy Markdown
Owner

Thanks @joeVenner for carrying this work forward and validating it. PR #2135 contains Zihao-Qi’s original DeepSeek commits and is being merged as the canonical change so first-author credit stays intact. The additional UsageFetcher+Testing.swift split is useful; please send that focused refactor as its own small PR.

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. P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. 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.

3 participants