Skip to content

feat(web): /calc expression evaluator -- pipeline-ab card 2 Arm B (STREAMLINED)#12

Open
eksno wants to merge 2 commits into
alphafrom
feat/calc-arm-b
Open

feat(web): /calc expression evaluator -- pipeline-ab card 2 Arm B (STREAMLINED)#12
eksno wants to merge 2 commits into
alphafrom
feat/calc-arm-b

Conversation

@eksno

@eksno eksno commented Jun 12, 2026

Copy link
Copy Markdown
Member

EXPERIMENT PR (card 2: harder, separating) -- DO NOT MERGE. pipeline-ab Arm B (STREAMLINED).

Streamlined lean layout: Composer 2.5 hands + review-reconcile only.

Correctness-deep card (expression parser/evaluator) chosen to separate Composer-hands-vs-Opus-hands. Both arms build from ONE shared plan. Stays OPEN for eyeball evaluation against its live Railway preview.


View with Codesmith Autofix with Codesmith
Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.

@railway-app railway-app Bot temporarily deployed to testing / testing-pr-12 June 12, 2026 20:09 Destroyed
@railway-app

railway-app Bot commented Jun 12, 2026

Copy link
Copy Markdown

🚅 Deployed to the testing-pr-12 environment in testing

Service Status Web Updated (UTC)
testing ✅ Success (View Logs) Web Jun 12, 2026 at 8:31 pm

…ed core, Opus-gated)

Hand-rolled precedence-climbing parser for + - * / ^ with right-assoc ^,
unary minus binding looser than ^, parens, decimals, scientific notation.
Pure core (tokenize/parse/evaluate/formatNumber/calculate) with a CalcError
boundary; graceful inline errors (no crashes), div-by-zero gated before
Infinity, 12-sig-digit float formatting (0.1+0.2 -> 0.3). Live-result /calc
page with a capped (10) most-recent-first history, per-row copy-result button
+ auto-dismiss toast (timer-leak cleanup on unmount), theme-aware, nav-wired,
prerender-safe. Adds vitest+jsdom and a 7-file/89-case test floor.

Core code authored by Cursor Composer 2.5 (ADR 0042) against an Opus-written
test floor it never saw; reviewed, applied, and gated by the Opus seat. All
89 tests pass; check + build + format green.

Station-Trailer-Version: 1
Station-Run-Id: k57f4ntnpwy60x26dyfqnx6tsd88h2va
@coderabbitai

coderabbitai Bot commented Jun 12, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@eksno, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 1 minute and 31 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more credits in the billing tab to continue.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 77647d6d-7eec-4135-bb5f-9f46d1bac72c

📥 Commits

Reviewing files that changed from the base of the PR and between c20b967 and 6ea4095.

⛔ Files ignored due to path filters (1)
  • web/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (24)
  • web/package.json
  • web/src/lib/calc/calculate.test.ts
  • web/src/lib/calc/calculate.ts
  • web/src/lib/calc/evaluate.test.ts
  • web/src/lib/calc/evaluate.ts
  • web/src/lib/calc/format.test.ts
  • web/src/lib/calc/format.ts
  • web/src/lib/calc/index.ts
  • web/src/lib/calc/parse.test.ts
  • web/src/lib/calc/parse.ts
  • web/src/lib/calc/tokenize.test.ts
  • web/src/lib/calc/tokenize.ts
  • web/src/lib/calc/types.ts
  • web/src/lib/clipboard.test.ts
  • web/src/lib/clipboard.ts
  • web/src/lib/components/site-header.svelte
  • web/src/lib/components/toaster.svelte
  • web/src/lib/toast-core.test.ts
  • web/src/lib/toast-core.ts
  • web/src/lib/toast.svelte.ts
  • web/src/routes/+layout.svelte
  • web/src/routes/calc/+page.svelte
  • web/src/routes/calc/+page.ts
  • web/vitest.config.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/calc-arm-b

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@eksno

eksno commented Jun 12, 2026

Copy link
Copy Markdown
Member Author

Final review (advisory -- not a merge gate)

/calc Arm B (lean, Composer-authored / Opus-gated) -- review report

ONLY review gate. Advisory; never blocks ship. Whole-diff red-team across three
lenses (spec / standards / quality). Change = git diff origin/alpha...HEAD
(commit 6ea4095). 25 files, +1833. Tests/check/build reported green by the
upstream gate; independently re-confirmed npm run test (89 passed / 7 files).

Verdict

Core is correct and spec-faithful. Every grammar/precedence/associativity rule,
the full error taxonomy, the divide-by-zero gate, the non-finite gate, and the
12-sig-digit float formatting match the plan verbatim. The required 7-file test
floor exists and every Section-6 case is present and passing. SSR/prerender
safety, theme-token-only styling, and the toast timer-leak cleanup are all in
place. ONE advisory finding: a pre-existing repo eslint config gap makes
npm run lint fail on the first .svelte.ts file. Out of allowed scope to fix
(eslint.config.js is DO-NOT-TOUCH); does not affect check/build/test.

Lens 1 -- spec gaps

No correctness gaps.

  • Grammar (parse.ts): precedence-climbing exactly per plan. ^ right-assoc
    (RHS recurses into parseUnary) -> 2^3^2 AST is ^(2, ^(3,2)). Unary minus
    looser than ^ (parseUnary wraps parsePower) -> -2^2 AST is
    unary(-, ^(2,2)). 2^-2 valid via RHS-through-unary. All asserted.
  • Error taxonomy: traced every disambiguation case by hand and all match --
    2+/2* -> Incomplete; (2+3 -> Mismatched; 2)+3/2 3/2(3)/2**3
    -> Unexpected token; () -> Incomplete (not Mismatched); 1/0/0/0/5/(2-2)
    -> Division by zero; 1e308*10 -> Result is not a finite number; bare ./1e
    -> Unexpected character with the exact offending char. Strings copied verbatim.
  • format.ts: -0/0 -> '0'; round via Number(n.toPrecision(12)); exp branch on
    abs >= 1e21 || abs < 1e-6 with mantissa trailing-zero trim. 1e21->'1e+21',
    0.0000005->'5e-7', 0.1+0.2->'0.3', 1/3->'0.333333333333' all verified.
  • calculate.ts: empty/whitespace pre-check; single try/catch; CalcError ->
    message, anything else -> 'Invalid expression'; non-finite gate post-eval.
  • UI (+page.svelte): live $derived(calculate(...)), empty state muted (not
    red), error in text-destructive + aria-invalid, Enter/"=" commit, history
    most-recent-first capped at 10, per-row Copy -> toast. Matches Section 4.
    Minor: empty-state copy reuses "Enter an expression" rather than the plan's
    suggested "Type an expression above" -- but Section 9 item 9 explicitly says
    render the Enter an expression state as the neutral empty state, so this is
    compliant, not a gap.
  • Nav + layout: /calc added to nav array; <Toaster/> mounted once in
    +layout.svelte after footer. Per manifest.
  • Deps: only vitest + jsdom added; test/test:watch scripts + vitest.config.ts
    present. No parser library. tsconfig/eslint/prettier/svelte.config untouched.

Lens 2 -- standards

  • Svelte 5 runes throughout: $state, $derived, $props, $effect. Toast
    store correctly carries .svelte.ts (module-scope $state); timer logic
    split into plain toast-core.ts for node-testability, per plan Section 5.2.
  • Intra-$lib imports use .js/index.js extensions as the repo does. $lib
    alias used in source and re-aliased in vitest.config.ts.
  • Prettier: npm run prettier --check passes for all files (tabs, single
    quotes, semis, es5 trailing comma).
  • ASCII: -- used as the em-dash convention in copy and comments. No unicode.
  • cn(): page uses inline class strings / array class syntax; cn() not required
    where no conditional class merge is needed -- consistent with existing pages.

Lens 3 -- quality

  • PARSER CORRECTNESS: clean. Precedence, right-assoc ^, unary-minus binding,
    unary-in-exponent, no-implicit-mult, div-by-zero in evaluate, non-finite gate
    in calculate, 12-sig-digit formatting -- all correct and test-covered (89
    assertions). No dead branches; the parsePrimary op-vs-rparen split produces
    the exact taxonomy strings.
  • Theme: toaster + page use only theme tokens (bg-card, text-destructive,
    text-muted-foreground, border-border, etc.). No hardcoded hex/rgb.
  • Timer/subscription leaks: toaster $effect returns () => clearAllToastTimers();
    toast-core clearAll clearTimeouts every handle and empties the map; proven
    by toast-core.test.ts (clearAll -> cb never fires; pending() returns to 0).
  • SSR/prerender: calculate('') is pure; no window/document/navigator at module
    scope or during render; clipboard.ts guards typeof navigator === 'undefined'
    and missing .clipboard. +page.ts restates prerender = true. Safe.
  • Dead code: none. evaluate's nested switch is exhaustive; outer switch returns
    on every ast.type so check is satisfied.
  • ESLint .svelte.ts gap (the flagged probe): npm run lint FAILS with
    Parsing error: Unexpected token interface on src/lib/toast.svelte.ts:3.
    Root cause: eslint.config.js applies tseslint.parser only via a
    files: ['**/*.svelte'] block; there is NO **/*.svelte.ts block, so the
    first .svelte.ts file in the repo falls through to espree (plain JS) which
    cannot parse interface. This is a PRE-EXISTING config limitation (the config
    pre-dates this branch and is in the plan's DO-NOT-TOUCH list), surfaced -- not
    introduced -- by this arm. The arm's file is correct: the plan MANDATES the
    .svelte.ts extension for the runes store, and interface Toast {} is normal
    TS. Fix belongs in eslint.config.js (add a files: ['**/*.svelte.ts'] block
    with parserOptions.parser = tseslint.parser), which is out of this arm's
    allowed scope. check/build/test are unaffected and green.

Gate evidence

  • npm run test: 7 files / 89 tests passed.
  • npm run lint: prettier clean; eslint fails ONLY on the pre-existing
    .svelte.ts parser gap (advisory).
  • check/build: reported green by upstream gate; not re-run (no fix applied).

@eksno

eksno commented Jun 12, 2026

Copy link
Copy Markdown
Member Author

/calc Arm B -- triage disposition

Findings: 1. Acted-on: 0. Left: 1. Advisory gate -- never blocks ship.

ACTED-ON (applied in worktree)

(none -- the single finding is out of allowed scope to fix)

LEFT (judgement calls / out-of-scope)

  1. npm run lint fails: Parsing error: Unexpected token interface at
    web/src/lib/toast.svelte.ts:3.
    • Reason left: PRE-EXISTING repo eslint config gap, not introduced by this
      arm. eslint.config.js has a files: ['**/*.svelte'] tseslint.parser block
      but none for **/*.svelte.ts, so the first .svelte.ts file falls through
      to espree and cannot parse TS interface. The proper fix edits
      eslint.config.js -- explicitly in the plan's DO-NOT-TOUCH list and outside
      this arm's allowed scope. The arm's file is correct (plan mandates the
      .svelte.ts extension; interface Toast is valid TS). check/build/test
      are green and unaffected; advisory only.

No-finding lenses

  • Spec: no correctness gaps (one cosmetic copy note -- empty-state text reuses
    "Enter an expression" -- is plan-compliant per Section 9 item 9, NOT a finding).
  • Standards: clean.
  • Quality parser-correctness: clean.

Fix commits

none (no fix applied; nothing committed; nothing pushed).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant