Skip to content

feat: Add Template Drafter for org.accordproject.money@1.0.0.PreciseAmount - #173

Open
ujjwalv01 wants to merge 2 commits into
accordproject:mainfrom
ujjwalv01:feat/add-preciseamount-drafter
Open

feat: Add Template Drafter for org.accordproject.money@1.0.0.PreciseAmount#173
ujjwalv01 wants to merge 2 commits into
accordproject:mainfrom
ujjwalv01:feat/add-preciseamount-drafter

Conversation

@ujjwalv01

Copy link
Copy Markdown

Closes #172

Description

This PR introduces a dedicated template drafter for the new org.accordproject.money@1.0.0.PreciseAmount type, allowing templates to render arbitrary-precision monetary amounts as human-readable text.

The PreciseAmount model separates the value into an exact BigInteger string (unscaledValue) and a decimal scale (carried on unit) to avoid IEEE-754 double-precision drift. This drafter correctly reconstructs the decimal string representation directly from the unscaledValue without ever converting to a JavaScript Number or Float during the default rendering phase, ensuring that precision is fully maintained at any magnitude.

Key Implementation Details

1. Arbitrary-Precision String Reconstruction
A helper function reconstructDecimalString was introduced. It uses pure string manipulation to insert the decimal point based on the unit.scale, safely handling leading zeroes and negative values without precision loss.

function reconstructDecimalString(value: string, scale: number): string {
    if (scale === 0) return value;
    
    let isNegative = false;
    let absValue = value;
    if (absValue.startsWith('-')) {
        isNegative = true;
        absValue = absValue.substring(1);
    }
    
    if (absValue.length <= scale) {
        absValue = absValue.padStart(scale + 1, '0');
    }
    
    const insertPos = absValue.length - scale;
    const integerPart = absValue.substring(0, insertPos);
    const decimalPart = absValue.substring(insertPos);
    
    let result = `${integerPart}.${decimalPart}`;
    return isNegative ? '-' + result : result;
}

2. Formatting Support
The drafter supports two modes:

  • Default Rendering: Outputs reconstructDecimalString(...) + ' ' + unit.code (e.g. 1234.00 USD).
  • Format-String Rendering: For custom formats like {{payment as "K 0,0.00 CCC"}}, the drafter re-uses the existing draftDoubleFormat utility to replace K (currency symbol) and CCC (currency code) tokens, ensuring backward compatibility with existing monetary formatting conventions.

3. Drafter Registration
The new drafter is registered in src/drafting/index.ts:

case 'org.accordproject.money@1.0.0.PreciseAmount':
    return preciseAmountDrafter(data, format);

Testing

  • Unit Tests: Added a comprehensive test suite in test/PreciseAmountDrafter.test.ts covering:
    • Zero, positive, and negative unscaledValues
    • Amounts requiring zero-padding (where length of value <= scale)
    • scale = 0 (no decimal point)
    • Custom format strings
  • End-to-End Fixtures: Added a new template fixture in test/templates/good/preciseamount/ containing a model.cto, data.json, and template.md which successfully passed snapshot tests via TemplateMarkInterpreter.

Files Changed

  • src/drafting/PreciseAmount/index.ts (new) — drafter implementation
  • src/drafting/index.ts (modified) — drafter registration
  • test/PreciseAmountDrafter.test.ts (new) — unit tests
  • test/templates/good/preciseamount/ (new) — E2E fixture (model.cto, data.json, template.md)

@ujjwalv01
ujjwalv01 requested a review from a team August 5, 2026 16:08
@ujjwalv01
ujjwalv01 force-pushed the feat/add-preciseamount-drafter branch from 33b3e42 to 32a755a Compare August 5, 2026 16:11
@devanshi00

Copy link
Copy Markdown
Contributor

Hey @ujjwalv01 , thanks for this pr. I added a few comments.
We will still need a review from @mttrbrts before merging.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds drafting support for the new org.accordproject.money@1.0.0.PreciseAmount type so TemplateMark variables render as human-readable monetary strings instead of raw serialized objects.

Changes:

  • Introduces a PreciseAmount drafter with default rendering and optional format-string rendering.
  • Registers the new drafter in the drafting registry (getDrafter).
  • Adds unit tests and a new TemplateMark snapshot entry for preciseamount.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
src/drafting/PreciseAmount/index.ts New PreciseAmount drafting implementation (default + format-string modes).
src/drafting/index.ts Registers the PreciseAmount drafter in getDrafter().
test/PreciseAmountDrafter.test.ts Adds unit tests for PreciseAmount drafting.
test/snapshots/TemplateMarkInterpreter.test.ts.snap Adds snapshot output for a preciseamount template rendering case.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread test/PreciseAmountDrafter.test.ts
Comment thread test/__snapshots__/TemplateMarkInterpreter.test.ts.snap
Comment thread src/drafting/PreciseAmount/index.ts
@ujjwalv01
ujjwalv01 force-pushed the feat/add-preciseamount-drafter branch from fed727e to 6c8e134 Compare August 5, 2026 18:27
Signed-off-by: Ujjwal Verma <ujjwalverma010305@gmail.com>
@ujjwalv01
ujjwalv01 force-pushed the feat/add-preciseamount-drafter branch from 6c8e134 to 03eb473 Compare August 5, 2026 18:33
Signed-off-by: Ujjwal Verma <ujjwalverma010305@gmail.com>
@ujjwalv01

Copy link
Copy Markdown
Author

Thanks for the detailed Copilot review! Here's a summary of what was addressed:

Suggestion 1 : - Zero-padding test coverage
Added the missing test case for the absValue.length <= scale branch in reconstructDecimalString (e.g. unscaledValue: "1", scale: 2"0.01 USD").

Suggestion 2 - Missing fixture files
The fixture files (model.cto, data.json, template.md) under test/templates/good/preciseamount/ were not committed in the initial push because test/templates/ is listed in .gitignore. They have now been force-added (consistent with how all other existing fixtures in the repo are tracked) and are included in the latest commit.

Suggestion 3 - Number.isFinite guard for large amounts
Applied the suggested guard in preciseAmountFormatDrafter to prevent a crash when unscaledValue is outside the IEEE-754 Number range (e.g. very large ETH amounts with 18 decimals). In that case the drafter falls back to the plain "<value> <code>" string instead of throwing.

Additionally, two more test cases were proactively added:

  • Negative zero-padding: unscaledValue: "-1", scale: 2"-0.01 USD"
  • Number.isFinite fallback: a 400-digit value with a format string correctly returns a plain string instead of "Infinity USD"

All 8 unit tests pass.

mttrbrts commented Aug 5, 2026

Copy link
Copy Markdown
Member

Thanks for iterating on the Copilot feedback — the default-rendering path is solid, and the string-based reconstructDecimalString is the right way to preserve precision. A few points remain that I'd like to see addressed before merge, plus one architectural question.

1. Version dispatch — the drafter is pinned to an exact version (highest priority)

getDrafter matches on the fully-qualified type name including the exact namespace version:

case 'org.accordproject.money@1.0.0.PreciseAmount': return preciseAmountDrafter;

The moment the money model ships a 1.0.1, 1.1.0, or 2.0.0, elementType becomes org.accordproject.money@1.1.0.PreciseAmount, which no longer matches. getDrafter returns null and the interpreter falls back to JSON.stringify(variableValue) (TemplateMarkInterpreter.ts:478), so the variable renders as a raw JSON blob in the document rather than 1234.00 USD — silently, with no error. Since money@1.x is a new and actively-evolving line, this will bite quickly.

Rather than pinning to @1.0.0, I'd suggest normalizing the type name to namespace + major version before the switch, so patch/minor releases reuse the drafter and only a breaking major bump (where the shape could legitimately change) requires a new case. ModelUtil (already a dependency) has everything needed:

import { ModelUtil } from '@accordproject/concerto-core';

function drafterKey(fqn: string): string {
    if (ModelUtil.isPrimitiveType(fqn)) {
        return fqn; // Boolean, String, Integer, ...
    }
    const ns = ModelUtil.getNamespace(fqn);            // org.accordproject.money@1.1.0
    const name = ModelUtil.getShortName(fqn);          // PreciseAmount
    const { name: nsName, version } = ModelUtil.parseNamespace(ns);
    const major = version ? version.split('.')[0] : '';
    return `${nsName}@${major}.${name}`;               // org.accordproject.money@1.PreciseAmount
}

The cases then become version-line keys — org.accordproject.money@1.PreciseAmount, org.accordproject.money@0.MonetaryAmount, etc. This is arguably a pre-existing issue (MonetaryAmount@0.3.0 is pinned the same way), so it's reasonable to split it into a follow-up — but PreciseAmount is the type most exposed to it, so I'd rather not add another exact-version pin. Happy to take it either way as long as we've decided consciously.

2. Format path still round-trips through Number — defeats the type's premise

preciseAmountFormatDrafter does Number(strValue) and hands the double to draftDoubleFormat. This reintroduces exactly the IEEE-754 precision loss PreciseAmount exists to avoid. The Number.isFinite guard only catches full overflow to Infinity (~309+ digits); the far more common case — any value past ~15–17 significant digits (e.g. an 18-decimal token balance) — rounds silently and formats a wrong number that no guard catches. And when the guard does fire, the fallback returns the plain value + code string, dropping the requested format entirely. To honor the precision guarantee on the format path too, the grouping/decimal formatting should be applied to the reconstructed string (or via BigInt) rather than a double. If a full string formatter is out of scope for this PR, that's worth stating explicitly and filing as a follow-up, because as written formatted PreciseAmount is no more precise than legacy MonetaryAmount.

3. Duplication with MonetaryAmount

codeSymbol is copied verbatim, and the .replace(/K/gi, …).replace(/CCC/gi, …) token substitution is duplicated from monetaryAmountFormatDrafter. Both should move to a shared helper (e.g. alongside src/drafting/MonetaryAmount/currencycode.ts) and be imported by both drafters, so currency-symbol handling has a single source of truth.

4. Minor: trailing whitespace inside reconstructDecimalString and the trailing blank lines in the test file will likely trip eslint — a local npm run lint should catch them.


Generated by Claude Code

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.

feat: add drafter for org.accordproject.money@1.0.0.PreciseAmount

4 participants