-
Notifications
You must be signed in to change notification settings - Fork 7
fix(reset): stop refusing eligible credits on wham applicable_available_count #133
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1241,7 +1241,6 @@ async function buildResetPreviewRow( | |
| const precondition = evaluateResetPrecondition( | ||
| quota, | ||
| ctx.quotaManager.isRateLimited(accountKey), | ||
| applicableAvailableCount, | ||
| ctx.now(), | ||
| ) | ||
| let reason: string | undefined | ||
|
|
@@ -1313,11 +1312,17 @@ function renderResetConfirm(row: ResetPreviewRow): string { | |
| '', | ||
| `Account: **${row.label}** (\`${row.accountKey}\`)`, | ||
| `Current quota: **${row.usedPercent ?? 'unknown'}% used**`, | ||
| `Credit: **Spend 1 of ${row.applicableAvailableCount ?? 0}**`, | ||
| `Credit: **Spend 1 of ${row.availableCount ?? 0}**`, | ||
| `Credit expires: **${row.selectedCreditExpiresAt ?? 'unavailable'}**`, | ||
| `Quota resets: **${row.resetTime ?? 'unavailable'}**`, | ||
| '', | ||
| ] | ||
| if ((row.availableCount ?? 0) > 0 && row.applicableAvailableCount === 0) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: When WHAM omits Prompt for AI agents
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Valid. The |
||
| lines.push( | ||
| 'The server does not currently count this credit as applicable; redemption may return a no-op, and a no-op does not spend the credit.', | ||
| ) | ||
| lines.push('') | ||
| } | ||
| if (row.eligible && row.chatgptAccountId) { | ||
| lines.push( | ||
| `Confirm: \`/openai-reset confirm ${encodeURIComponent(row.accountKey)} ${encodeURIComponent(row.chatgptAccountId)}\``, | ||
|
|
@@ -1360,8 +1365,6 @@ function resetErrorPayload( | |
| 'There is no active reset redemption to retry. Reopen the account list.', | ||
| not_exhausted: | ||
| 'No credit was spent: the fresh account state is not exhausted.', | ||
| no_applicable_credits: | ||
| 'No credit was spent: no applicable credits are available.', | ||
| no_eligible_credit: | ||
| 'No credit was spent: no eligible credit was returned.', | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -448,20 +448,20 @@ describe('reset redemption precondition', () => { | |
|
|
||
| it('accepts an exhausted live window with an applicable credit', () => { | ||
| expect( | ||
| evaluateResetPrecondition({ primary: quotaWindow(100) }, false, 1, now), | ||
| evaluateResetPrecondition({ primary: quotaWindow(100) }, false, now), | ||
| ).toEqual({ ok: true }) | ||
| }) | ||
|
|
||
| it('refuses healthy quota', () => { | ||
| expect( | ||
| evaluateResetPrecondition({ primary: quotaWindow(20) }, false, 1, now), | ||
| evaluateResetPrecondition({ primary: quotaWindow(20) }, false, now), | ||
| ).toEqual({ ok: false, reason: 'not exhausted' }) | ||
| }) | ||
|
|
||
| it('refuses exhausted quota without applicable credits', () => { | ||
| it('accepts exhausted quota when the server reports no applicable credits', () => { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P3: The renamed test 'accepts exhausted quota when the server reports no applicable credits' is now identical to the test above it ('accepts an exhausted live window with an applicable credit'): both call evaluateResetPrecondition({ primary: quotaWindow(100) }, false, now) and expect { ok: true }. Because applicableAvailableCount was removed from the function signature, this test no longer tests anything about applicable credits and only duplicates the prior case. Rename it to reflect what it actually asserts (e.g. 'accepts exhausted quota regardless of applicable credits') or drop it. Prompt for AI agents
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct, it was a duplicate after the parameter went away. Dropped in |
||
| expect( | ||
| evaluateResetPrecondition({ primary: quotaWindow(100) }, false, 0, now), | ||
| ).toEqual({ ok: false, reason: 'no applicable credits' }) | ||
| evaluateResetPrecondition({ primary: quotaWindow(100) }, false, now), | ||
| ).toEqual({ ok: true }) | ||
| }) | ||
|
|
||
| it('treats a 100%-used expired window as stale rather than exhausted', () => { | ||
|
|
@@ -471,7 +471,6 @@ describe('reset redemption precondition', () => { | |
| primary: quotaWindow(100, '2026-07-17T11:59:59.999Z'), | ||
| }, | ||
| false, | ||
| 1, | ||
| now, | ||
| ), | ||
| ).toEqual({ ok: false, reason: 'not exhausted' }) | ||
|
|
@@ -485,7 +484,6 @@ describe('reset redemption precondition', () => { | |
| secondary: quotaWindow(100), | ||
| }, | ||
| false, | ||
| 1, | ||
| now, | ||
| ), | ||
| ).toEqual({ ok: true }) | ||
|
|
@@ -499,7 +497,6 @@ describe('reset redemption precondition', () => { | |
| secondary: quotaWindow(100, '2026-07-17T11:59:59.999Z'), | ||
| }, | ||
| false, | ||
| 1, | ||
| now, | ||
| ), | ||
| ).toEqual({ ok: false, reason: 'not exhausted' }) | ||
|
|
@@ -510,26 +507,20 @@ describe('reset redemption precondition', () => { | |
| evaluateResetPrecondition( | ||
| { primary: quotaWindow(100, undefined) }, | ||
| false, | ||
| 1, | ||
| now, | ||
| ), | ||
| ).toEqual({ ok: true }) | ||
| expect( | ||
| evaluateResetPrecondition( | ||
| { primary: quotaWindow(100, 'not-a-date') }, | ||
| false, | ||
| 1, | ||
| now, | ||
| ), | ||
| ).toEqual({ ok: true }) | ||
| }) | ||
|
|
||
| it('lets a live rate-limit mark satisfy only exhaustion', () => { | ||
| expect(evaluateResetPrecondition({}, true, 1, now)).toEqual({ ok: true }) | ||
| expect(evaluateResetPrecondition({}, true, 0, now)).toEqual({ | ||
| ok: false, | ||
| reason: 'no applicable credits', | ||
| }) | ||
| expect(evaluateResetPrecondition({}, true, now)).toEqual({ ok: true }) | ||
| }) | ||
| }) | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: When the credit list contains an eligible credit but both available-count fields are omitted, this confirmation still says
Spend 1 of 0. Preserve the unknown count or render an explicit unknown/at-least-one value instead of defaulting it to zero.Prompt for AI agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Valid.
225c437: when bothavailable_countfields are absent,availableCountfalls back to the number of credits the eligibility filter accepts (factored intoisResetCreditEligible/countEligibleResetCreditssoselectCreditToSpendand the count share one definition); if that is also 0 the confirmation rendersunknown, never0. The TUIDialogConfirmreads the same row value, so both surfaces agree. Test: both counts omitted, one eligible credit →Spend 1 of 1; removing the list fallback makes it fail.