Skip to content

fix: show day form at exactly 24h in reset countdowns#2343

Open
OfficialAbhinavSingh wants to merge 1 commit into
steipete:mainfrom
OfficialAbhinavSingh:fix-reset-countdown-day-rollover
Open

fix: show day form at exactly 24h in reset countdowns#2343
OfficialAbhinavSingh wants to merge 1 commit into
steipete:mainfrom
OfficialAbhinavSingh:fix-reset-countdown-day-rollover

Conversation

@OfficialAbhinavSingh

Copy link
Copy Markdown
Contributor

Summary

Four providers format their reset countdown with a private copy of the same helper, each using if hours > 24:

  • WindsurfGetPlanStatusResponse.formatResetDescription (WindsurfWebFetcher.swift)
  • WindsurfCachedPlanInfo.formatResetDescription (WindsurfStatusProbe.swift)
  • ZedUsageSnapshot.formatResetDescription (ZedStatusProbe.swift)
  • JetBrainsStatusSnapshot.formatResetDescription (JetBrainsStatusProbe.swift)

Because the day branch is gated on hours > 24, a reset exactly 24h out (hours == 24) falls through to the hours branch and renders "Resets in 24h 0m" / "Cycle ends in 24h 0m". The "1d 0h" day form is unreachable; the smallest day output is "1d 1h" (at 25h).

The app's canonical formatter, UsageFormatter.resetCountdownDescription, rolls over correctly at 24h (days = totalMinutes / (24*60)). These four provider copies are the outliers.

Fix

hours > 24hours >= 24 in all four copies. At 24h → "1d 0h"; 25h → "1d 1h" (unchanged); 23h → "23h 0m" (unchanged). The [24h, 25h) band now uses the day form (dropping minutes, exactly as [25h, ∞) already did).

Each formatResetDescription also gains an injectable now: Date = Date() parameter (replacing an internal let now = Date()), matching UsageFormatter.resetCountdownDescription(from:now:) so the pure formatting is unit-testable. All existing callers pass only date and are unaffected.

Test

TestsLinux/ResetCountdownDayRolloverLinuxTests.swift — for each of the four formatters: 24h → day form; plus Windsurf 25h → "1d 1h" and 23h → "23h 0m" (no regression).

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 (> 24) — the 24h cases render the hours form:

✘ Windsurf web  : formatResetDescription(now+24h) → "Resets in 24h 0m"      == "Resets in 1d 0h"      FAILED
✘ Windsurf cached: ...                             → "Resets in 24h 0m"      == "Resets in 1d 0h"      FAILED
✘ Zed           : ...                              → "Cycle ends in 24h 0m"  == "Cycle ends in 1d 0h"  FAILED
✘ JetBrains     : ...                              → "Resets in 24h 0m"      == "Resets in 1d 0h"      FAILED
✔ 23h stays "23h 0m", 25h stays "1d 1h"
✘ Test run with 6 tests failed with 4 issues.

After the fix (>= 24) — all pass:

✔ Test run with 6 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.

Windsurf (web + cached), Zed, and JetBrains each format their reset
countdown with if hours > 24, so a reset exactly 24h out fell through to
the hours branch and rendered '24h 0m' / 'Cycle ends in 24h 0m' - the
'1d 0h' day form was unreachable. Change to hours >= 24 in all four,
matching UsageFormatter.resetCountdownDescription. Inject now (default
Date()) so the formatters are unit-testable, mirroring the canonical
formatter's signature.
@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. P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. 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:52 PM ET / 16:52 UTC.

Summary
The PR changes four reset countdown formatters to render exactly 24 hours as one day and adds Linux-only boundary tests.

Reproducibility: yes. from source: with hours == 24, the base condition hours > 24 necessarily selects the hours branch. The supplied patch and deterministic now parameter define a focused boundary reproduction, though I did not independently run it against current main.

Review metrics: 2 noteworthy metrics.

  • Provider helpers changed: 4 formatter implementations changed. The same boundary behavior is corrected across Windsurf web, Windsurf cached, Zed, and JetBrains.
  • Regression coverage: 1 Linux-only test file added with 6 cases. The test suite covers the boundary but is outside the repository's standard requested test location.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🦐 gold shrimp
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:

  • Move or duplicate the boundary tests under Tests/CodexBarTests and run the standard suite.
  • [P1] Add redacted after-fix runtime output from the built CLI or app; after updating the PR body, request @clawsweeper re-review if a fresh review does not start automatically.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR includes red-to-green unit-test output, but no after-fix runtime evidence from the compiled CLI or app; please add a redacted terminal transcript or other live output that directly shows the changed countdown behavior. 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] Before merge, the only supplied after-fix evidence is unit-test output; obtain a redacted runtime transcript or equivalent live output from the built CLI/app.
  • [P1] Linux-only regression coverage can miss a macOS-specific integration or compilation regression in the app's standard test target.

Maintainer options:

  1. Decide the mitigation before merge
    Keep the shared 24-hour rollover behavior, add or mirror focused tests under Tests/CodexBarTests, and attach redacted after-fix runtime output from the built product before merging.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P1] The remaining blockers require contributor-provided runtime proof and a normal review update; there is no safe autonomous repair task to dispatch.

Security
Cleared: The diff only adjusts local time-formatting logic and test coverage; it introduces no dependency, credential, permission, download, or supply-chain surface.

Review findings

  • [P2] Place regression coverage in the standard test target — TestsLinux/ResetCountdownDayRolloverLinuxTests.swift:1-5
Review details

Best possible solution:

Keep the shared 24-hour rollover behavior, add or mirror focused tests under Tests/CodexBarTests, and attach redacted after-fix runtime output from the built product before merging.

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

Yes, from source: with hours == 24, the base condition hours > 24 necessarily selects the hours branch. The supplied patch and deterministic now parameter define a focused boundary reproduction, though I did not independently run it against current main.

Is this the best way to solve the issue?

Yes, changing the outlying conditions to >= 24 is the narrowest fix because it matches the stated canonical formatter behavior. The regression coverage should also be placed in the repository's standard test target.

Full review comments:

  • [P2] Place regression coverage in the standard test target — TestsLinux/ResetCountdownDayRolloverLinuxTests.swift:1-5
    Move or mirror these boundary tests under Tests/CodexBarTests as repository policy requires. Keeping them only in TestsLinux means the normal macOS suite used for the menu-bar app does not protect this behavior.
    Confidence: 0.92

Overall correctness: patch is incorrect
Overall confidence: 0.84

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add P3: This is a narrow formatting-boundary correction with limited user impact and a straightforward repair path.
  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
  • 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 includes red-to-green unit-test output, but no after-fix runtime evidence from the compiled CLI or app; please add a redacted terminal transcript or other live output that directly shows the changed countdown behavior. 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:

  • P3: This is a narrow formatting-boundary correction with limited user impact and a straightforward repair path.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR includes red-to-green unit-test output, but no after-fix runtime evidence from the compiled CLI or app; please add a redacted terminal transcript or other live output that directly shows the changed countdown behavior. 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:

  • Proposed boundary correction: All four provider-specific helpers change the day condition from hours > 24 to hours >= 24, which makes the documented 24-hour boundary render as 1d 0h. (Sources/CodexBarCore/Providers/Windsurf/WindsurfWebFetcher.swift:90, 0bc246819c45)
  • Coverage location conflicts with repository policy: Repository guidance asks new focused coverage to live under Tests/CodexBarTests; the proposed regression suite is instead isolated under TestsLinux, so the normal macOS test path does not exercise it. (TestsLinux/ResetCountdownDayRolloverLinuxTests.swift:1, 0bc246819c45)
  • Proof supplied is test-only: The PR body supplies local red-to-green Swift Testing output, but no runtime output from the compiled CLI or app demonstrates the user-visible provider countdown after the change. (0bc246819c45)
  • Current check context: Lint and security checks are successful, while the Linux build/test and macOS test jobs were still running in the supplied review context. (0bc246819c45)

Likely related people:

  • steipete: Repository ownership identifies this account as the best available routing candidate, but current-main file history could not be independently established from the supplied review context. (role: likely follow-up owner; confidence: low; files: Sources/CodexBarCore/Providers/Windsurf/WindsurfWebFetcher.swift, Sources/CodexBarCore/Providers/Windsurf/WindsurfStatusProbe.swift, Sources/CodexBarCore/Providers/Zed/ZedStatusProbe.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

P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. 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