Skip to content

Reject non-finite Kimi K2 usage values#1915

Merged
steipete merged 2 commits into
steipete:mainfrom
joeVenner:fix/kimi-k2-nonfinite-usage
Jul 6, 2026
Merged

Reject non-finite Kimi K2 usage values#1915
steipete merged 2 commits into
steipete:mainfrom
joeVenner:fix/kimi-k2-nonfinite-usage

Conversation

@joeVenner

@joeVenner joeVenner commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Summary

  • reject non-finite Kimi K2 credit and average-token values from JSON or headers
  • keep missing/malformed values on the existing zero-or-nil fallback path
  • allow an injected HTTP transport for deterministic production-path validation
  • cover NaN, infinity, and overflow numeric strings

Production-path proof

The regression test calls the public fetcher with an in-memory HTTP response containing non-finite JSON values and a non-finite X-Credits-Remaining fallback. It verifies the request authorization header and the resulting zero-or-nil fallback. No live credentials are used.

Testing

  • swift test --disable-sandbox --filter KimiK2UsageFetcherTests (7 passed)
  • make test (all 48 shards passed)
  • make check (SwiftFormat clean; SwiftLint 0 violations)
  • autoreview (clean, 0.98 confidence)
  • git diff --check
  • exact head: 909b415a751eb7e34af141a8a8db80e989442219

@clawsweeper

clawsweeper Bot commented Jul 5, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed July 6, 2026, 4:33 AM ET / 08:33 UTC.

Summary
The branch filters non-finite Kimi K2 numeric usage values, injects ProviderHTTPTransport into fetchUsage for deterministic validation, and adds regression coverage plus a changelog entry.

Reproducibility: yes. from source inspection: current main's Kimi K2 double(from:) accepts Double and Double(String) values without finite checks, so NaN or Infinity can propagate into the usage summary. I did not run local tests because this review was read-only.

Review metrics: none identified.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

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

Rank-up moves:

  • none.

Next step before merge

  • [P2] No repair lane is needed because the PR has no actionable findings; the remaining action is ordinary maintainer review and required CI completion.

Security
Cleared: The diff only changes Kimi K2 numeric parsing, a narrow injectable HTTP transport seam, tests, and a changelog entry; I found no concrete security or supply-chain regression.

Review details

Best possible solution:

Land the focused finite-value guard after required CI and maintainer review, keeping malformed or non-finite Kimi K2 numeric inputs on the existing zero/nil fallback path.

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

Yes from source inspection: current main's Kimi K2 double(from:) accepts Double and Double(String) values without finite checks, so NaN or Infinity can propagate into the usage summary. I did not run local tests because this review was read-only.

Is this the best way to solve the issue?

Yes; putting the finite check in the shared double(from:) parser covers JSON values and the header fallback with one narrow change, and the transport seam matches existing provider-test patterns.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 76a63dc55939.

Label changes

Label justifications:

  • P2: This is a bounded provider-parser correctness fix for Kimi K2 usage values with limited blast radius.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR carries a proof-sufficient label and the prior ClawSweeper review records copied live output from a temporary CodexBarCore executable exercising the public fetcher; the current body also lists focused and full validation commands.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR carries a proof-sufficient label and the prior ClawSweeper review records copied live output from a temporary CodexBarCore executable exercising the public fetcher; the current body also lists focused and full validation commands.
Evidence reviewed

What I checked:

  • Current main accepts non-finite usage numbers: Current main's Kimi K2 double(from:) returns raw Double values and Double(String) results without an isFinite guard, so NaN or Infinity can reach consumed, remaining, or averageTokens. (Sources/CodexBarCore/Providers/KimiK2/KimiK2UsageFetcher.swift:242, 76a63dc55939)
  • Patch adds the finite guard at the shared parser seam: The PR head centralizes conversion in double(from:) and rejects any nil or non-finite Double before returning a usage value. (Sources/CodexBarCore/Providers/KimiK2/KimiK2UsageFetcher.swift:245, 909b415a751e)
  • Regression test exercises the public fetcher path: The added async test injects an in-memory HTTP transport, verifies the Authorization header, returns non-finite JSON and header values, and asserts zero-or-nil fallback results. (Tests/CodexBarTests/KimiK2UsageFetcherTests.swift:43, 909b415a751e)
  • Transport injection matches an existing shared seam: ProviderHTTPTransport and ProviderHTTPTransportHandler are already public CodexBarCore test seams used for deterministic provider fetcher validation. (Sources/CodexBarCore/ProviderHTTPClient.swift:6, 76a63dc55939)
  • Kimi K2 docs confirm the affected source and fallback: The provider docs describe the legacy Kimi K2 credits endpoint, X-Credits-Remaining fallback, and common JSON usage fields that this parser consumes. (docs/kimi-k2.md:15, 76a63dc55939)
  • Re-review continuity and PR state: The prior ClawSweeper cycle had no findings; the current PR head is 909b415, remains open, has proof sufficient/ready labels, and non-macOS checks shown by GitHub were successful while macOS shards were still running. (909b415a751e)

Likely related people:

  • steipete: Peter Steinberger authored multiple Kimi K2 usage-parser follow-ups in the relevant history and also authored the latest hardening commit on this branch. (role: feature history owner and recent area contributor; confidence: high; commits: a5fb02b7d0f2, fc3670b5363e, 762f60b382b3; files: Sources/CodexBarCore/Providers/KimiK2/KimiK2UsageFetcher.swift, Tests/CodexBarTests/KimiK2UsageFetcherTests.swift, docs/kimi-k2.md)
  • 0-CYBERDYNE-SYSTEMS-0: The Kimi K2 provider support appears in history under this author before later parser follow-ups refined the fetcher. (role: introduced provider support; confidence: medium; commits: eade57bfde35; files: Sources/CodexBarCore/Providers/KimiK2/KimiK2UsageFetcher.swift, Sources/CodexBarCore/Providers/KimiK2/KimiK2ProviderDescriptor.swift)
  • William: Recent balance-metric work touched API-balance provider display paths that would consume malformed Kimi K2 credit values. (role: adjacent balance display contributor; confidence: medium; commits: c7f38669a655; files: Sources/CodexBarCore/Providers/KimiK2/KimiK2UsageFetcher.swift, Sources/CodexBar/StatusItemController+Animation.swift, Tests/CodexBarTests/StatusItemBalanceDisplayTests.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 (1 earlier review cycle)
  • reviewed 2026-07-05T16:12:52.208Z sha 750db3f :: needs maintainer review before merge. :: none

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal priority bug or improvement with limited blast radius. labels Jul 5, 2026
@steipete steipete force-pushed the fix/kimi-k2-nonfinite-usage branch from 750db3f to 909b415 Compare July 6, 2026 08:13
@steipete steipete changed the title Reject nonfinite Kimi K2 usage values Reject non-finite Kimi K2 usage values Jul 6, 2026
@steipete steipete merged commit b007975 into steipete:main Jul 6, 2026
10 checks passed
@steipete

steipete commented Jul 6, 2026

Copy link
Copy Markdown
Owner

Landed as b00797537dbffe0e0bd469544003fd5769b75542. Thanks @joeVenner!

Maintainer verification:

  • swift test --disable-sandbox --filter KimiK2UsageFetcherTests — 7 passed
  • make test — all 48 shards passed
  • make check — SwiftFormat and SwiftLint clean
  • autoreview — no actionable findings; patch-correct confidence 0.98
  • git diff --check — clean
  • exact-head CI for 909b415a751eb7e34af141a8a8db80e989442219 — all 10 checks passed: https://github.com/steipete/CodexBar/actions/runs/28777491318

The regression uses an in-memory transport through the public fetch path; no live credentials or Keychain access were used.

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

Labels

P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants