Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueWarning Your free Security trial is over. An organization admin can upgrade to Advanced for continuous pull request security review or dismiss this notice. Comment |
closest('.field') on #calc-imgs resolved to the Output-tokens wrapper, so
selecting Pro hid the whole output field and selecting Vision never revealed
the image input (it carried its own hidden attr). Give the image field its
own #img-field wrapper with a max and a unit hint; add a 'Cache saves' card;
widen the token input pattern to match the new parser.
- 'You save … · 50%' sat under the cache slider but the % was a constant from PEAK_FACTOR. Add a Cache-saves value (input cost delta vs 0% hit), label the 50% as 'vs peak', include output/image tokens in the breakdown with /1M units, set aria-valuetext on the slider. - parseTokens returned 0 for '50 M', '1.5m', '50_000_000', so the bill collapsed to output-only and the slider had nothing to move. Accept decimals/spaces/underscores/b, return NaN otherwise and flag the field with aria-invalid; blur no longer overwrites invalid text. - fmtTotal: 6 decimals below $0.01 so 8k/500 presets don't round to 0. - imgField now targets #img-field.
…, img clamp - readTokens: Number.isFinite (400-digit input overflowed to Infinity past the NaN check) and returns NaN so recalc shows '—' on every card instead of pricing a 0-token bill; breakdown tells the user which field to fix. - Per-field hint <small id="calc-*-err"> wired via aria-describedby, shown only while invalid. - Drop inputmode=numeric on token inputs: mobile numeric keyboards have no k/M/B or '.' keys the pattern accepts. - Invalid:focus keeps the 4px focus ring (amber-tinted) instead of a 1px ring. - Image count clamped to the input's max (10000); type=number does not enforce min/max/step on typed values.
… smoke test
- parseTokens('1.5') was rounded to 2 tokens and accepted; a user who forgot
the M got a near-zero bill with no flag. Decimals now require a k/M/B suffix.
- Image count: out-of-range/fractional values now use the input's own
validity state → aria-invalid + hint + blanked bill, same contract as the
token fields, so the visible value and the billed value never disagree
(previously clamped silently to 0 or 10000).
- test.mjs: id-memoised stub DOM; drives readTokens/recalc end to end and
asserts card text, aria-invalid, hint visibility and image validity paths.
…281cf4) Rebase onto main dropped a281cf4's calculator edits as superseded: - direct cache-ratio listener: redundant, the form-level input listener already fires for range inputs; aria-valuetext is set in recalc(). - savePct = save/peak: always 50%, same constant the branch labels 'vs peak'; the slider-driven number is the new Cache-saves card. - chip el.blur(): blur() on an unfocused element fires no event, so chips stopped recalculating on main. Chips call recalc() directly here. Only the image-count reset was worth keeping.
tomkabel
force-pushed
the
fix/calculator-audit
branch
from
September 6, 2026 21:02
2f1381d to
f57c316
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Audit of the cost calculator at
d12c4d6. The per-token formula was correct; the bugs were in display, parsing and DOM wiring.closest('.field')resolved to the output wrapper, so selecting Pro removed the output input entirely and Vision never showed the image input. Own#img-fieldwrapper now.1 − 1/PEAK_FACTORrendered50%under the cache slider regardless of input. New Cache saves card shows input-cost delta vs 0% hit ratio; the 50% is now labelledvs peak.50 M,1.5m,50_000_000→ 0 input tokens, bill collapsed to output-only, slider had nothing to move. Parser accepts decimals/spaces/underscores/b; anything else setsaria-invalidwith an amber ring instead of billing 0./1Munits and output share of bill.fmtTotaluses 6 decimals below $0.01 so the 8k/500 presets no longer round to$0.0007.aria-valuetext.Verification
node test.mjspasses (10 new assertions).50 Mnow equals50M.fiftyflags the field. Pro keeps the Output field visible; Vision shows the image field and 10 images add $0.0008.Review notes
Four atomic commits, one concern each, reviewable independently. Branch is off
d12c4d6, notmainHEAD;a281cf4on main touches the same code, so expect a merge conflict inapp.jsto resolve on rebase.