Skip to content

fix numbers with hundreds inside a thousand or million group - #1254

Merged
spencermountain merged 2 commits into
spencermountain:devfrom
b84330808:fix-number-hundreds-groups
Oct 29, 2026
Merged

spencermountain merged 2 commits into
spencermountain:devfrom
b84330808:fix-number-hundreds-groups

Conversation

@b84330808

Copy link
Copy Markdown
Contributor

Numbers with a hundreds digit inside a bigger group went wrong in both directions:

nlp('two million three hundred forty five thousand').numbers().get() // [2000345000]
nlp('123456').numbers().toText().text() // 'one hundred thousand twenty three thousand four hundred and fifty six'
  • parse: in the 'hundred .. thousand' branch the whole running sum was multiplied, millions included. Now only the part after the last bigger multiple is.
  • toText: 'hundred thousand' and 'thousand' were separate magnitudes. A 'hundred ' followed by its now writes just 'hundred', giving 'one hundred twenty three thousand …'.

They go together: toOrdinal() re-parses the text, so the toText change alone would turn 1994683 into 'one billion …'. Added cases to the number-parse and to-text tests; they fail on dev (11 of 85) and pass with the change. The full suite and the plugin suites pass.

I used Claude Code to write this change.

b84330808 and others added 2 commits October 28, 2026 22:20
'two million three hundred forty five thousand' parsed as 2000345000:
when a smaller group with hundreds ended in a bigger multiple
('three hundred forty five thousand'), the whole running sum was
multiplied, including the millions already counted. Only multiply
the part that came after the last bigger multiple.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
toText() turned 123456 into 'one hundred thousand twenty three
thousand ...', because 'hundred thousand' and 'thousand' were written
as separate magnitudes. When a 'hundred <unit>' is followed by its
<unit>, write 'hundred' and let the next group add the unit:
'one hundred twenty three thousand ...'.

This needs the parse fix before it, since toOrdinal() re-parses the
text.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@spencermountain

Copy link
Copy Markdown
Owner

wow! thank you Wei-Lun!!
This must have been going on for 8 or 10 years. I had no idea!
This is very helpful - thank you.

@spencermountain
spencermountain merged commit fb8d9ca into spencermountain:dev Oct 29, 2026
@spencermountain spencermountain mentioned this pull request Nov 8, 2026
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