Skip to content

Populate windowMinutes for MiMo and StepFun monthly windows (#2431) - #2526

Open
LeoLin990405 wants to merge 1 commit into
steipete:mainfrom
LeoLin990405:feat/mimo-stepfun-windowminutes
Open

Populate windowMinutes for MiMo and StepFun monthly windows (#2431)#2526
LeoLin990405 wants to merge 1 commit into
steipete:mainfrom
LeoLin990405:feat/mimo-stepfun-windowminutes

Conversation

@LeoLin990405

Copy link
Copy Markdown
Contributor

Summary

Companion to #2514 (#2431). MiMo's monthly token quota and StepFun's monthly credit pool build their primary RateWindow with windowMinutes = nil. planUtilizationSeriesSamples requires a window with windowMinutes, so those two cards get neither the plan-utilization history chart nor the predictive pace forecast that the Codex/Claude cards show — a real info-density gap.

Changes

  • MiMo (MiMoUsageSnapshot): the monthly token-quota window carries ProviderPaceCapability.monthlyWindowSentinelMinutes when planPeriodEnd is present (a real reset to project against), and nil otherwise.
  • StepFun (StepFunUsageFetcher): the credit-pool window carries it when creditResetTime is present (otherwise the pace projection would be meaningless).

Both are gated on a real reset date, so no window is fabricated for an account without one.

Verification

  • swift test --filter "MiMo|StepFun" — 115 passed, including new cases: StepFun's credit window now asserts windowMinutes == 43200, and MiMo asserts 43200 with a period end and nil without one.
  • swiftlint --strict — 0 violations; SwiftFormat — clean.
  • Live: a StepFun credit-plan account now reports the credit window with windowMinutes: 43200 (reset ≈ 1 month out) — it now feeds the history + pace forecast. (MiMo verified by construction; the account under test has no active plan period to exercise it live.)

Refs #2431.

…#2431)

MiMo's monthly token quota and StepFun's monthly credit pool built their
primary RateWindow with windowMinutes = nil, so `planUtilizationSeriesSamples`
skipped them — meaning those cards get neither the plan-utilization history
chart nor the predictive pace forecast that Codex/Claude cards show. This
brings them to parity, mirroring steipete#2514 (which did the same for Zai/Kimi/Grok/
Antigravity).

- MiMo: token window carries `monthlyWindowSentinelMinutes` when `planPeriodEnd`
  is present (a real reset to project against); nil otherwise.
- StepFun: the credit window carries it when `creditResetTime` is present.

Verified live: a StepFun credit-plan account now reports the credit window with
windowMinutes 43200 (reset ~1 month out). Tests cover both providers, including
the no-reset case that keeps windowMinutes nil.

Refs steipete#2431.
@clawsweeper clawsweeper Bot added the rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. label Jul 31, 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: c1928af5a7

ℹ️ 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".

// Codex/Claude windows. Only when there's a real period end to reset against.
let windowMinutes = self.planPeriodEnd == nil
? nil
: ProviderPaceCapability.monthlyWindowSentinelMinutes

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Route monthly windows through history and card pace

Setting the sentinel here (and in the analogous StepFun change) still does not enable either advertised card feature: UsageStore.planUtilizationSeriesSamples routes generic providers only through exact 300/10080-minute session-equivalent windows, so a lone 43200-minute primary produces no history samples, while both provider descriptors retain the default .unsupported pace capability, causing resetWindowPaceDetail to return nil. Add explicit monthly sampling and .calendarMonthResetWindow capabilities for MiMo and StepFun, with a model-seam test that verifies the history and pace output rather than only the snapshot field.

AGENTS.md reference: AGENTS.md:L25-L27

Useful? React with 👍 / 👎.

Comment on lines +269 to +271
let creditWindowMinutes = self.creditResetTime == nil
? nil
: ProviderPaceCapability.monthlyWindowSentinelMinutes

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Reject zero StepFun reset timestamps

When a credit-plan response contains "subscription_credit_reset_time": "0" (the API's existing representation for an unconfigured reset), parsing produces a non-nil Date(timeIntervalSince1970: 0), so this nil check incorrectly marks it as a real monthly window. Validate that the timestamp/reset date is positive and meaningful before assigning the monthly sentinel; otherwise downstream consumers receive a fabricated 30-day window anchored in 1970.

Useful? React with 👍 / 👎.

@LeoLin990405
LeoLin990405 force-pushed the feat/mimo-stepfun-windowminutes branch from c1928af to 117e855 Compare July 31, 2026 05:28
@clawsweeper clawsweeper Bot added 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. merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. labels Jul 31, 2026
@clawsweeper

clawsweeper Bot commented Jul 31, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed July 31, 2026, 1:31 AM ET / 05:31 UTC.

ClawSweeper review

What this changes

Adds monthly windowMinutes metadata to MiMo token-quota and StepFun credit-pool usage windows, plus focused snapshot parsing tests.

Merge readiness

Blocked until stronger real behavior proof is added - 8 items remain

Keep this PR open: its current changes set the monthly sentinel on two snapshots, but they do not route either provider through the downstream history and pace features the PR promises, and StepFun can treat an API reset value of 0 as a valid monthly reset.

Priority: P2
Reviewed head: 117e85593cf335dbbb2cb4b7eafaf4a35368a206

Review scores

Measure Result What it means
Overall readiness 🦪 silver shellfish (2/6) The patch has a focused intent and field-level tests, but it currently misses the downstream behavior it claims to unlock and lacks inspectable real-behavior proof.
Proof confidence 🦪 silver shellfish (2/6) Needs stronger real behavior proof before merge: The PR describes a live StepFun result, but provides no inspectable terminal output, redacted log, screenshot, recording, or linked artifact that demonstrates the after-fix history and pace behavior; add redacted real-output evidence after completing the downstream fix. 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.
Patch quality 🦪 silver shellfish (2/6) 2 actionable review findings remain.

Verification

Check Result Evidence
Real behavior Needs proof Needs stronger real behavior proof before merge: The PR describes a live StepFun result, but provides no inspectable terminal output, redacted log, screenshot, recording, or linked artifact that demonstrates the after-fix history and pace behavior; add redacted real-output evidence after completing the downstream fix. 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 5 items PR diff only populates snapshot metadata: The proposed MiMo and StepFun changes assign the 43,200-minute sentinel to their primary RateWindow, but do not modify the downstream history-sampling or pace-capability paths.
Downstream behavior remains disabled: The prior review correctly identifies that generic history sampling accepts only the existing session-equivalent durations and both provider descriptors remain .unsupported; therefore the new sentinel alone cannot enable the advertised chart or forecast.
Invalid reset timestamp remains accepted: The StepFun change checks only whether creditResetTime is non-nil. A parsed Unix timestamp of zero is non-nil and would receive a fabricated monthly duration despite being an unconfigured reset.
Findings 2 actionable findings [P2] Enable the downstream monthly history and pace paths
[P2] Reject non-positive StepFun reset timestamps
Security None None.

How this fits together

Provider adapters normalize vendor quota responses into RateWindow values for CodexBar usage cards. The usage store and provider pace capability then decide whether those windows produce historical utilization samples and a pace forecast for the menu-bar UI.

flowchart LR
    A[MiMo and StepFun APIs] --> B[Provider usage snapshots]
    B --> C[Rate window metadata]
    C --> D[Usage history sampling]
    C --> E[Pace capability decision]
    D --> F[Utilization chart]
    E --> G[Pace forecast]
    F --> H[Usage card]
    G --> H
Loading

Before merge

  • Add real behavior proof - Needs stronger real behavior proof before merge: The PR describes a live StepFun result, but provides no inspectable terminal output, redacted log, screenshot, recording, or linked artifact that demonstrates the after-fix history and pace behavior; add redacted real-output evidence after completing the downstream fix. 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.
  • Enable the downstream monthly history and pace paths (P2) - The sentinel does not itself make either advertised feature available: generic history sampling still recognizes only the existing session-equivalent durations, and both provider descriptors remain pace-unsupported. Add explicit monthly sampling and calendar-month pace capability for MiMo and StepFun, then test the resulting history and pace outputs through the model seam.
  • Reject non-positive StepFun reset timestamps (P2) - creditResetTime == Date(timeIntervalSince1970: 0) is non-nil, so an API response using "0" for an unconfigured reset receives a fictitious 30-day window. Require a positive, meaningful reset timestamp before assigning the monthly sentinel.
  • Resolve merge risk (P1) - Merging as written would leave MiMo and StepFun without the history chart and pace forecast described in the PR, while making the snapshot metadata imply that they are supported.
  • Resolve merge risk (P1) - A StepFun response with reset timestamp 0 could display a monthly pace projection anchored to an invalid 1970 reset boundary.
  • Complete next step (P2) - Both blockers are concrete, local code/test repairs; real after-fix evidence must still be supplied by the contributor before merge.
  • Improve patch quality - Implement the two concrete review findings and add model-seam tests for actual history and pace output.
  • Improve patch quality - Post redacted after-fix terminal output, logs, or a recording that shows the StepFun or MiMo card now receives the history and forecast behavior; updating the PR body should trigger re-review, or a maintainer can request @clawsweeper re-review.

Findings

  • [P2] Enable the downstream monthly history and pace paths — Sources/CodexBarCore/Providers/MiMo/MiMoUsageSnapshot.swift:65-67
  • [P2] Reject non-positive StepFun reset timestamps — Sources/CodexBarCore/Providers/StepFun/StepFunUsageFetcher.swift:269-271
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Changed surface 4 files affected: 2 provider files and 2 test files The patch is narrow, so the missing downstream capability wiring is a bounded repair rather than a broad redesign.
Snapshot assertions 3 assertions added for monthly or nil window metadata They validate field assignment but do not establish the chart and forecast behavior claimed in the PR description.

Merge-risk options

Maintainer options:

  1. Complete the downstream monthly-window support (recommended)
    Add explicit monthly sampling and calendar-month pace capabilities for both providers, reject invalid StepFun reset values, and cover the model-level history and pace outputs before merging.
  2. Pause the companion change
    Hold this PR if the product does not intend to support monthly pace forecasts for these providers, because snapshot-only metadata is misleading and has no user-visible benefit.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Add valid monthly sampling and calendar-month pace support for MiMo and StepFun, reject non-positive StepFun reset timestamps, and add focused model-seam coverage for history samples and pace output.

Technical review

Best possible solution:

Wire valid monthly MiMo and StepFun windows through the existing history and calendar-month pace paths, reject absent or non-positive reset dates, and verify the resulting chart samples and pace output at the model seam.

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

Yes, by source inspection: construct a valid monthly MiMo or StepFun snapshot and pass it through the existing usage-history and pace model seams; the current descriptor and sampling rules leave both outputs absent. A StepFun fixture containing reset timestamp 0 also exercises the invalid-reset path.

Is this the best way to solve the issue?

No. Setting windowMinutes alone is not the narrowest complete fix because the downstream eligibility rules still reject it; the fix should include valid-reset validation, explicit monthly sampling, provider pace capability, and model-seam regression coverage.

Full review comments:

  • [P2] Enable the downstream monthly history and pace paths — Sources/CodexBarCore/Providers/MiMo/MiMoUsageSnapshot.swift:65-67
    The sentinel does not itself make either advertised feature available: generic history sampling still recognizes only the existing session-equivalent durations, and both provider descriptors remain pace-unsupported. Add explicit monthly sampling and calendar-month pace capability for MiMo and StepFun, then test the resulting history and pace outputs through the model seam.
    Confidence: 0.98
  • [P2] Reject non-positive StepFun reset timestamps — Sources/CodexBarCore/Providers/StepFun/StepFunUsageFetcher.swift:269-271
    creditResetTime == Date(timeIntervalSince1970: 0) is non-nil, so an API response using "0" for an unconfigured reset receives a fictitious 30-day window. Require a positive, meaningful reset timestamp before assigning the monthly sentinel.
    Confidence: 0.97

Overall correctness: patch is incorrect
Overall confidence: 0.98

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 8ef86077e70a.

Labels

Label justifications:

  • P2: This is a bounded provider-usage regression in the proposed patch: the advertised UI capability remains inactive and one invalid reset representation can yield a misleading forecast.
  • merge-risk: 🚨 other: The PR changes user-visible usage forecasting semantics, but invalid or unsupported provider windows can surface nonsensical or absent forecast data despite the new metadata.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦪 silver shellfish.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR describes a live StepFun result, but provides no inspectable terminal output, redacted log, screenshot, recording, or linked artifact that demonstrates the after-fix history and pace behavior; add redacted real-output evidence after completing the downstream fix. 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

Acceptance criteria:

  • [P1] swift test --filter "MiMo|StepFun".
  • [P1] make check.

What I checked:

  • PR diff only populates snapshot metadata: The proposed MiMo and StepFun changes assign the 43,200-minute sentinel to their primary RateWindow, but do not modify the downstream history-sampling or pace-capability paths. (Sources/CodexBarCore/Providers/MiMo/MiMoUsageSnapshot.swift:65, 117e85593cf3)
  • Downstream behavior remains disabled: The prior review correctly identifies that generic history sampling accepts only the existing session-equivalent durations and both provider descriptors remain .unsupported; therefore the new sentinel alone cannot enable the advertised chart or forecast. (117e85593cf3)
  • Invalid reset timestamp remains accepted: The StepFun change checks only whether creditResetTime is non-nil. A parsed Unix timestamp of zero is non-nil and would receive a fabricated monthly duration despite being an unconfigured reset. (Sources/CodexBarCore/Providers/StepFun/StepFunUsageFetcher.swift:269, 117e85593cf3)
  • Tests do not cover the user-visible path: The added and updated tests assert the snapshot field, but do not verify that a MiMo or StepFun window yields plan-utilization samples and a non-nil pace result through the stable model seam. (Tests/CodexBarTests/MiMoWindowMinutesTests.swift:19, 117e85593cf3)
  • Repository policy applied: The repository guidance asks for focused provider/parser tests and stable model seams rather than live AppKit flows; the missing downstream model-seam coverage is material here. (AGENTS.md:25, 8ef86077e70a)

Likely related people:

  • steipete: The repository owner is the only concrete current-main routing signal available in the supplied review context; local history inspection was unavailable in this read-only environment. (role: likely follow-up owner; confidence: low; files: Sources/CodexBarCore/Providers/MiMo/MiMoUsageSnapshot.swift, Sources/CodexBarCore/Providers/StepFun/StepFunUsageFetcher.swift)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

  • 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

merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. 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