Classify StepFun Coding vs Token Plan by payload shape, not a hard-coded plan_family - #2492
Conversation
clawsweeper
Bot
added
rating: 🦐 gold shrimp
|
Codex review: needs real behavior proof before merge. Reviewed July 29, 2026, 12:29 PM ET / 16:29 UTC. ClawSweeper reviewWhat this changesThis PR makes StepFun choose rolling-window or credit usage rendering from the response payload shape, using Merge readiness⛔ Blocked until stronger real behavior proof is added - 3 items remain Keep this PR open: the focused shape-first classifier appears to address the linked StepFun rendering bug, but the changed grandfathered Coding Plan branch still lacks after-fix runtime proof. Likely related people: Priority: P2 Review scores
Verification
How this fits togetherCodexBar’s StepFun provider decodes a rate-limit response into a normalized usage snapshot. The resulting snapshot chooses whether the menu bar and widget show five-hour/weekly rolling windows or a monthly credit balance. flowchart LR
A[StepFun rate-limit response] --> B[Decode usage fields]
B --> C{Live reset window?}
C -->|Yes| D[Rolling-window usage]
C -->|No| E{Credit pool present?}
E -->|Yes| F[Credit-balance usage]
E -->|No| G[Family-id fallback]
D --> H[Menu and widget rows]
F --> H
G --> H
Before merge
Agent review detailsSecurityNone. Review metrics
Root-cause clusterRelationship: Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Retain the payload-shape classifier and add redacted after-fix Coding Plan output that demonstrates nonzero reset times render as five-hour and weekly usage before merging. Do we have a high-confidence way to reproduce the issue? Unclear: the fixture and focused tests establish the intended conflicting-payload path, and the PR reports a live Token Plan check, but no after-fix runtime evidence exists for the changed grandfathered Coding Plan window path. Is this the best way to solve the issue? Yes, conditionally: preferring observed payload shape over a hard-coded family id is the narrower and more resilient repair, provided the exhausted-credit-pool handling remains structurally detected and live behavior is confirmed. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against c52b0efa13b0. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (1 earlier review cycle)
|
…ded plan_family StepFun runs two Step Plan billing models side by side after the 2026-06-18 upgrade (docs/zh/step-plan/upgrade-notice): the grandfathered Coding Plan meters rolling 5-hour/weekly windows, while the current Token Plan meters a monthly Credit pool via plan_credit_rate_limit (its rate windows come back as 0 with reset_time "0"). isCreditPlan trusted plan_family == 2 first, so a windowed plan that ever reported plan_family=2 (or any future family-id change) would route to the credit renderer — dropping the real 5h/weekly windows and showing a bogus credit balance. Classify by the shape the payload actually carries (a live window reset time means Coding Plan; no window plus a Credit pool means Token Plan) and use plan_family only as a tie-breaker for an otherwise-ambiguous payload. Adds tests for the window-wins-over-family-id case and the ambiguous fallback. Verified live against a Token Plan account (still renders the credit balance).
1401d62 to
4f6b23f
Compare
|
Merged after full verification. StepFun plans are now classified by payload shape — live reset windows take precedence over Fixed during review: |
2 participants
Summary
Closes #2491.
StepFun's
isCreditPlantrustedplan_family == 2ahead of the payload shape. After the 2026-06-18 Step Plan upgrade the grandfathered Coding Plan (rolling 5-hour/weekly windows) and the current Token Plan (monthly Credit pool) both run in the wild, so a windowed payload that ever carriedplan_family == 2— or any future family-id change — would route a rolling-window plan into the credit renderer, dropping the real windows and showing a credit balance the plan doesn't have.This classifies by the shape the response actually carries and keeps
plan_familyonly as a tie-breaker:*_reset_time > 0) → Coding Plan (windows);plan_family == 2.The credit renderer already only fires when a plan has a Credit pool and no live windows, so the current Token Plan path is unchanged; this just stops the family id from overriding a live window.
Verification
swift test --filter StepFun— 39 passed, including two new cases: a live-window payload that also reportsplan_family == 2stays on the rolling-window renderer, and the ambiguous-payload fallback to the family id.Scripts/lint.sh(strict) — 0 violations; SwiftFormat — no changes.I don't have a grandfathered Coding Plan account to exercise the window branch live, so that path is covered by a fixture built from the documented shape + the fields observed on
QueryStepPlanRateLimit, mirroring the fixture-based approach used for the Alibaba Personal variants in #2487.