Skip to content

feat(schema): add phase 22 calculation engine - #24

Merged
II-ricky-bobby-II merged 3 commits into
canaryfrom
codex/phase-22-calculation-engine-clean
May 15, 2026
Merged

feat(schema): add phase 22 calculation engine#24
II-ricky-bobby-II merged 3 commits into
canaryfrom
codex/phase-22-calculation-engine-clean

Conversation

@cobmojo

@cobmojo cobmojo commented May 2, 2026

Copy link
Copy Markdown

Summary

  • Adds Phase 22 deterministic calculation primitives in @asym/pdf-template-schema.
  • Exports structured helpers for numeric aggregates, table totals, grouped subtotals, invoice totals, financial totals, and tax-deductible amounts.
  • Includes Greptile follow-up fixes for grouped diagnostic duplication, grouped source indexes, grouped grand total reconciliation, and exponential decimal parsing.
  • Documents the Phase 22 contract, roadmap/OpenSpec state, decision log entry, and rollback guidance.

Review fixes included

  • Keeps per-group aggregate diagnostics inside each group while preserving original source-array indexes for row correlation.
  • Calculates grouped grand totals from rows with valid group keys so group subtotals and grand totals reconcile.
  • Uses the grouped-row grand aggregate as the sole outer row-level value diagnostic source.
  • Rejects e/E decimal notation instead of parsing it through Number(...).toFixed(...), preserving the no-floating-point decimal parsing contract.

Validation

  • corepack pnpm --filter @asym/pdf-template-schema test -- calculations.spec.ts - red first for the two review issues, then passed after the fix.
  • corepack pnpm --filter @asym/pdf-template-schema test - passed, 66 tests.
  • corepack pnpm --filter @asym/pdf-template-schema typecheck - passed.
  • corepack pnpm --filter @asym/pdf-template-schema build - passed.
  • corepack pnpm test - passed, 16 Turbo tasks.
  • corepack pnpm lint - passed with one existing warning in untouched apps/web/src/app/editor/editor-overrides.css.
  • corepack pnpm --filter /pdf-renderer test - passed, 83 tests.
  • corepack pnpm --filter /pdf-renderer typecheck - passed.
  • corepack pnpm --filter /pdf-renderer build - passed.
  • corepack pnpm --filter /pdf-editor test - passed, 38 tests after building legacy workspace package dist entries required by the test harness.
  • corepack pnpm --filter /pdf-editor typecheck - passed.
  • corepack pnpm --filter /pdf-editor build - passed.
  • corepack pnpm --filter -email/editor test - passed, 460 passed and 1 skipped, with existing Vite/React warnings.
  • corepack pnpm asym:editor-export-smoke - passed.
  • corepack pnpm dlx -ai/openspec validate build-pdf-document-builder - passed.
  • corepack pnpm dlx -ai/openspec validate --all - passed.
  • git diff --check - passed.

Earlier Phase 22 validation also passed renderer/editor package tests, typechecks, builds, @react-email/editor tests, export smoke, OpenSpec validation, and full repo checks before this clean PR branch was created.

Known gaps

  • Renderer visible total rows and summary blocks are intentionally deferred to Phase 23.
  • Calculations are pure helpers only; templates still do not evaluate arbitrary JavaScript.

Rollback

Revert the commits in this PR to remove the calculation source and tests, restore the schema export/maturity metadata, and undo the Phase 22 documentation/OpenSpec updates. Then rerun schema, renderer, editor, lint, and OpenSpec validation.

OpenSpec

Phase 22: Build Calculation Engine for Totals, Subtotals, and Grouping. Next phase entry point: Phase 23 exposes summary blocks and total rows.

Greptile Summary

This PR adds the Phase 22 deterministic calculation engine to @asym/pdf-template-schema: six exported helpers (calculateNumericAggregate, calculateTableTotals, calculateGroupedTableTotals, calculateInvoiceTotals, calculateFinancialTotals, calculateTaxDeductibleAmount) built on BigInt minor-unit arithmetic with no new dependencies. Documentation, OpenSpec, and maturity metadata are updated throughout.

  • calculations.ts (~1 119 lines): pure BigInt decimal engine with half_away_from_zero rounding, exponential-notation rejection, global source-index preservation in grouped diagnostics, and a grand-total scoped to rows with valid group keys.
  • Tests: 10 new spec cases covering aggregates, invoice quantity×rate multiplication, grouped subtotals with error rows, precision edge cases, and tax-deductible floor — all documentation-first and passing.

Confidence Score: 4/5

Safe to merge after fixing the net.count defect in calculateFinancialTotals; all other calculation paths are correct.

In calculateFinancialTotals, net.count is set to rowsResult.rows.length (all source rows) rather than incomeCount + expenseCount (rows that actually contributed). The API contract documents count as the contributing count, and income.count/expense.count both honour it correctly. Any row with a missing category, unknown category, or invalid amount is skipped but still inflates net.count, so callers using that field for auditing or display would see wrong data from the moment the first invalid row appears in production financial data.

packages/pdf-template-schema/src/calculations.ts — specifically the net count argument in calculateFinancialTotals

Important Files Changed

Filename Overview
packages/pdf-template-schema/src/calculations.ts New 1119-line calculation engine using BigInt arithmetic; one defect: net.count uses total source rows instead of contributing income+expense row count, violating the API contract for CalculationDecimalValue.count
packages/pdf-template-schema/test/calculations.spec.ts Comprehensive test suite covering aggregates, invoice totals, grouped subtotals, financial totals, precision, and exponential notation rejection; does not assert net.count with invalid/uncategorized rows so the count defect is not caught
packages/pdf-template-schema/src/index.ts Exports all six calculation helpers and their types; maturity tag updated to phase-22-calculation-engine
packages/pdf-template-schema/test/public-entry.spec.ts Smoke-tests the public entry point; adds calculateNumericAggregate to the exports-defined check and updates the maturity boundary assertion

Reviews (2): Last reviewed commit: "fix(schema): reconcile grouped calculati..." | Re-trigger Greptile

Greptile also left 1 inline comment on this PR.

Comment thread packages/pdf-template-schema/src/calculations.ts
@II-ricky-bobby-II
II-ricky-bobby-II merged commit 5ce8e68 into canary May 15, 2026
7 checks passed
@II-ricky-bobby-II
II-ricky-bobby-II deleted the codex/phase-22-calculation-engine-clean branch May 15, 2026 06:52
Comment on lines +554 to +558
net: createDecimalValue(
income - expense,
precision.scale,
rowsResult.rows.length,
),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 net.count reports total source rows, not contributing rows

The API contract for CalculationDecimalValue documents count as the number of contributing rows. income.count = incomeCount and expense.count = expenseCount follow this contract, but net.count is set to rowsResult.rows.length, which includes every row regardless of whether it contributed — rows with missing categories, unknown categories, or invalid/missing amount fields are all counted despite being skipped in the loop. A caller auditing the net total with net.count would see an inflated figure whenever any row is invalid or uncategorized.

Suggested change
net: createDecimalValue(
income - expense,
precision.scale,
rowsResult.rows.length,
),
net: createDecimalValue(
income - expense,
precision.scale,
incomeCount + expenseCount,
),

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.

2 participants