Skip to content

fix: clamp z.ai raw-percentage usedPercent fallback to 0...100#2342

Open
OfficialAbhinavSingh wants to merge 1 commit into
steipete:mainfrom
OfficialAbhinavSingh:fix-zai-percent-fallback-clamp
Open

fix: clamp z.ai raw-percentage usedPercent fallback to 0...100#2342
OfficialAbhinavSingh wants to merge 1 commit into
steipete:mainfrom
OfficialAbhinavSingh:fix-zai-percent-fallback-clamp

Conversation

@OfficialAbhinavSingh

Copy link
Copy Markdown
Contributor

Summary

ZaiLimitEntry.usedPercent returns computedUsedPercent (which already clamps min(100, max(0, …))) when z.ai reports quota fields. But when those fields are absent it falls back to return self.percentage — the raw API percentage, unclamped:

public var usedPercent: Double {
    if let computed = self.computedUsedPercent { return computed }
    return self.percentage            // raw, can be < 0 or > 100
}

z.ai is known to omit/misreport quota fields (see #1855), so an out-of-range percentage flows straight into RateWindow.usedPercent and the menu bar — every other provider, and this struct's own computed branch, clamp to 0…100.

Fix

- return self.percentage
+ return min(100, max(0, self.percentage))

Matches computedUsedPercent (line 150) and sibling providers. No change for in-range values.

Test

TestsLinux/ZaiUsedPercentLinuxTests.swift (usage == nil forces the fallback path):

  • 150 → 100, -5 → 0 (clamped)
  • 42 → 42 (in-range unchanged)
  • computed-path precedence: usage 100 / used 25 / percentage 999 → 25 (raw 999 does not leak)

Proof (real run)

Runs on CI in the build-linux-cli "Swift Test (Linux only)" step. Local red→green in swift:6.3.3:

Before the fix — fallback returns the raw value:

✘ "raw percentage fallback clamps above 100":  fallbackUsedPercent(150) → 150.0 == 100   FAILED
✘ "raw percentage fallback clamps below 0":     fallbackUsedPercent(-5)  → -5.0  == 0     FAILED
✔ in-range + computed-path tests pass
✘ Test run with 4 tests in 1 suite failed with 2 issues.

After the fix — all pass:

✔ Suite ZaiUsedPercentLinuxTests passed
✔ Test run with 4 tests in 1 suite passed

Build clean, swiftlint --strict 0 violations, swiftformat --lint clean.

Small and self-contained — happy to adjust or close if not wanted.

ZaiLimitEntry.usedPercent returns computedUsedPercent (already clamped)
when quota fields are present, but the fallback returned self.percentage
raw. z.ai omits/misreports quota fields, so an out-of-range API
percentage flowed unclamped into RateWindow.usedPercent. Clamp the
fallback like computedUsedPercent and sibling providers.
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. labels Jul 19, 2026
@clawsweeper

clawsweeper Bot commented Jul 19, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed July 19, 2026, 12:37 PM ET / 16:37 UTC.

Summary
The PR bounds Z.ai’s raw percentage fallback to 0–100 and adds four Linux tests covering out-of-range, in-range, and computed-value precedence cases.

Reproducibility: yes. at source level: a ZaiLimitEntry with missing quota fields takes the raw percentage fallback, so values below 0 or above 100 reach usedPercent without this patch.

Review metrics: 2 noteworthy metrics.

  • Patch surface: 2 files affected; 3 added source lines, 1 removed. The production behavior change is narrowly scoped to one provider fallback.
  • Regression cases: 4 Linux tests added. The suite exercises both bounds, normal input, and the existing computed-value precedence path.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🐚 platinum hermit
Result: blocked until real behavior proof from a real setup is added.

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

Rank-up moves:

  • [P2] Add a redacted terminal or runtime transcript showing the Z.ai CLI or menu path converts an out-of-range fallback percentage to 0 or 100; remove private tokens, endpoints, and account data.
  • [P1] After adding proof, update the PR body to trigger a fresh review; a maintainer can comment @clawsweeper re-review if needed.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body includes claimed local and CI unit-test results, but no inspectable redacted CLI, menu, log, or other after-fix runtime evidence from the Z.ai provider path; tests remain supplemental proof only. 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] The PR body shows focused test output only; it does not provide inspectable after-fix evidence from the actual Z.ai provider/CLI path, so malformed API values reaching user-visible output remain unverified before merge.

Maintainer options:

  1. Decide the mitigation before merge
    Retain the narrow clamp and regression tests, then land it after redacted runtime evidence shows the Z.ai CLI or menu path converts an out-of-range fallback response into a bounded usage value.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P1] Request real, redacted runtime proof before merge; this proof requirement is contributor-specific and cannot be completed by an automated repair patch.

Security
Cleared: The diff changes only local numeric normalization and a test suite; it adds no dependency, credential, permission, network, or build-supply-chain surface.

Review details

Best possible solution:

Retain the narrow clamp and regression tests, then land it after redacted runtime evidence shows the Z.ai CLI or menu path converts an out-of-range fallback response into a bounded usage value.

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

Yes, at source level: a ZaiLimitEntry with missing quota fields takes the raw percentage fallback, so values below 0 or above 100 reach usedPercent without this patch.

Is this the best way to solve the issue?

Yes. Clamping at the fallback is the narrowest maintainable fix because the computed branch already bounds the same user-facing percentage and the tests preserve its precedence.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 3b4d9a85cbec.

Label changes

Label changes:

  • add P2: Malformed Z.ai quota data can produce misleading provider usage values, but the impact is limited to one provider’s display path.
  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body includes claimed local and CI unit-test results, but no inspectable redacted CLI, menu, log, or other after-fix runtime evidence from the Z.ai provider path; tests remain supplemental proof only. 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.

Label justifications:

  • P2: Malformed Z.ai quota data can produce misleading provider usage values, but the impact is limited to one provider’s display path.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body includes claimed local and CI unit-test results, but no inspectable redacted CLI, menu, log, or other after-fix runtime evidence from the Z.ai provider path; tests remain supplemental proof only. 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

What I checked:

  • Focused implementation: The proposed source change only affects the fallback after computedUsedPercent is unavailable, preserving the existing computed path while bounding the raw API percentage. (Sources/CodexBarCore/Providers/Zai/ZaiUsageStats.swift:84, 39d86dae22e2)
  • Focused regression coverage: The added Linux test suite covers upper and lower bounds, an in-range value, and precedence of computed quota data over the raw percentage. (TestsLinux/ZaiUsedPercentLinuxTests.swift:1, 39d86dae22e2)
  • Related provider history: The related closed report documented Z.ai responses without quota fields, and the repository release history records earlier hardening for missing token-limit fields; this supports the fallback scenario but does not itself prove the new out-of-range case in a live run.

Likely related people:

  • takumi3488: Release history credits this contributor’s investigation and fix for preserving Z.ai weekly and 5-hour quota windows, an adjacent quota-shaping path. (role: prior Z.ai quota repair contributor; confidence: medium; files: Sources/CodexBarCore/Providers/Zai/ZaiUsageStats.swift)
  • MohamedMohana: Release history credits this contributor for handling Z.ai quota responses with missing token-limit fields, directly adjacent to this fallback behavior. (role: prior Z.ai missing-field hardening contributor; confidence: medium; files: Sources/CodexBarCore/Providers/Zai/ZaiUsageStats.swift)
  • steipete: Repository owner and the most likely person to confirm whether the intended UI contract is to bound malformed provider percentages at this parser boundary. (role: likely follow-up owner; confidence: low; commits: 3b4d9a85cbec; files: Sources/CodexBarCore/Providers/Zai/ZaiUsageStats.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.

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. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. 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.

1 participant