fix(drafting): keep the group separator off the minus sign

[rajanpanth]

Problem

Formatting a negative number with a grouped format puts the group separator immediately after the minus sign:

getDrafter('Integer')(-123, '0,0')          // '-,123'      expected '-123'
getDrafter('Integer')(-999, '0,0')          // '-,999'      expected '-999'
getDrafter('Integer')(-123456, '0,0')       // '-,123,456'  expected '-123,456'
getDrafter('Double')(-123456.78, '0,0.00')  // '-,123,456.78'
getDrafter('Long')(-123456, '0,0')          // '-,123,456'

Cause

draftIntegerFormat and draftDoubleFormat group the integer part by repeatedly peeling off its last three characters:

let i = vs.substring(0, vs.length);
while (i.length > 3) {
    res = sep1 + i.substring(i.length - 3) + res;
    i = i.substring(0, i.length - 3);
}
return i + res;

vs comes from value.toFixed(...), so for a negative number it starts with -. The loop treats that sign as if it were a digit. Once the digits have been consumed, i is left holding just "-", and since res already begins with a separator the result is -,123.

This only shows up when the digit count is a multiple of three. With -1234 the sign plus four digits leaves a two character remainder, so -1,234 comes out correctly, which is why the common cases look fine.

Fix

Separate the sign before grouping and put it back afterwards:

+const sign = i.startsWith('-') ? '-' : '';
+i = i.substring(sign.length);
 while (i.length > 3) {
     res = sep1 + i.substring(i.length - 3) + res;
     i = i.substring(0, i.length - 3);
 }
-return i + res;
+return sign + i + res;

Applied to both Double/format.ts and Integer/format.ts. Long imports draftIntegerFormat, so it is covered by the same change.

After the fix:

getDrafter('Integer')(-123, '0,0')          // '-123'
getDrafter('Integer')(-123456, '0,0')       // '-123,456'
getDrafter('Double')(-123456.78, '0,0.00')  // '-123,456.78'
getDrafter('Double')(-1234.5, '0,0.00')     // '-1,234.50'

Testing

Added cases to test/DraftFormat.test.ts covering negative Integer, Long and Double values at the digit counts that trigger the bug, plus a check that positive grouping is unchanged.

Reverting the two source files while keeping the new tests fails 3 of them, so they cover the change rather than restating current behaviour.

On the full suite, test/JavaScriptEvaluator.test.ts, test/GenerateOptions.test.ts and test/TemplateMarkInterpreter.test.ts fail for me, but they fail identically on a clean checkout of main, so they look unrelated to this change. Excluding those, the suite goes from 92 to 96 passing with the added tests and no new failures.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[dselman]

Good work. Well spotted bug fix.

[rajanpanth]

CI triage on the red Unit Tests (22.x) leg: the failure is test/JavaScriptEvaluator.test.ts receiving {"code": 0, "elapsed": 133, "result": undefined}, a worker timing case, while all 113 tests including the DraftFormat suite this PR touches pass. This PR only changes Double/format.ts, Integer/format.ts and DraftFormat.test.ts, so it cannot reach the evaluator.

That test also has a timeout-tuning history (test: increase timeout, Apr 2025), which fits a flake. A re-run should clear it; I cannot trigger one from a fork.

[rajanpanth]

Ping on this one, since it has been approved and is only held up by CI.

The single red leg is Unit Tests (22.x, ubuntu-latest) failing in test/JavaScriptEvaluator.test.ts with {"code": 0, "elapsed": 133, "result": undefined}, a worker timing case. That file has a timeout-tuning history (test: increase timeout, April 2025), and this PR only touches Double/format.ts, Integer/format.ts and DraftFormat.test.ts, so it cannot reach the evaluator. The DraftFormat suite and the other 112 tests pass.

A re-run should clear it, but I cannot trigger one from a fork. Happy to push an empty commit if you would rather have it re-run that way.