Skip to content

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

Merged
mttrbrts merged 2 commits into
accordproject:mainfrom
ujjwalv01:feat/add-preciseamount-drafter
Aug 6, 2026
Merged

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

Conversation

@ujjwalv01

Copy link
Copy Markdown
Contributor

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 2 times, most recently from 6c8e134 to 03eb473 Compare August 5, 2026 18:33
@ujjwalv01

Copy link
Copy Markdown
Contributor 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

@github-actions github-actions Bot added the maintainer-engaged A maintainer has commented or reviewed this item label Aug 6, 2026
@ujjwalv01
ujjwalv01 force-pushed the feat/add-preciseamount-drafter branch from 8219298 to bb58a13 Compare August 6, 2026 08:20
Signed-off-by: Ujjwal Verma <ujjwalverma010305@gmail.com>
@ujjwalv01
ujjwalv01 force-pushed the feat/add-preciseamount-drafter branch from bb58a13 to 26364d1 Compare August 6, 2026 08:22
@ujjwalv01

Copy link
Copy Markdown
Contributor Author

Hi @mttrbrts, thanks for the thorough review! I've addressed all four points in the latest push:

1. Version Dispatch
Added a drafterKey() helper using ModelUtil.isPrimitiveType, getNamespace, getShortName, and parseNamespace — exactly as you suggested. The switch now keys on org.accordproject.money@1.PreciseAmount (major-only), so patch and minor releases are handled automatically. I also wrapped it in a try-catch to gracefully handle user-defined types without versioned namespaces (which would otherwise throw on getNamespace). The existing MonetaryAmount and Duration/Period cases were updated consistently.

2. Format Path Precision
Removed the Number() conversion entirely from preciseAmountFormatDrafter. The formatting is now done directly on the reconstructed decimal string using pure string manipulation — splitting at the decimal point, applying digit grouping, and padding/truncating the decimal part without ever touching a JavaScript float. This preserves full precision even for 18-decimal token balances.

3. Code Duplication
Moved codeSymbol and the token replacement logic into a new shared file src/drafting/MonetaryAmount/formatTokens.ts. Both MonetaryAmount and PreciseAmount drafters now import replaceCurrencyTokens from this single source of truth.

4. Linting
Ran eslint --fix : trailing whitespace and extra blank lines cleaned up. License headers verified.

All 8 unit tests and all 38 E2E interpreter tests pass. Let me know if there's anything else to adjust!

…nd ETH precision

Add unit tests covering:
- JPY (scale 0), KWD (scale 3), USDC (scale 6) as non-standard scale currencies
- BTC (scale 8) including supply-cap satoshi value
- ETH (scale 18): 1 ETH, 0.1 ETH, 1 wei, 10 ETH, 100 ETH+1 wei
- ETH precision preservation for values that lose digits as JS floats
- Negative ETH values with fractional precision
- ETH formatted output with grouping separator and unknown symbol fallback

Signed-off-by: mttrbrts <code@rbrts.uk>

@mttrbrts mttrbrts left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the quick turnaround on the feedback. This is looking much stronger now.

I added some more test cases for some of the more extreme examples too.

@mttrbrts
mttrbrts enabled auto-merge (squash) August 6, 2026 08:52
@mttrbrts
mttrbrts merged commit a8d5555 into accordproject:main Aug 6, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintainer-engaged A maintainer has commented or reviewed this item

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