Skip to content

read 'twenty second' as 22nd, not twenty seconds - #1261

Merged
spencermountain merged 1 commit into
spencermountain:devfrom
b84330808:fix-tens-second-ordinal
Nov 3, 2026
Merged

spencermountain merged 1 commit into
spencermountain:devfrom
b84330808:fix-tens-second-ordinal

Conversation

@b84330808

Copy link
Copy Markdown
Contributor

'second' after any cardinal became a unit:

nlp('he finished forty-second').numbers().get()  // [40]
// with compromise-dates, 'may twenty-second' is may 20th
const doc = nlp('22')
doc.numbers().toText().toOrdinal()               // 'twenty second'
nlp(doc.text()).numbers().get()                  // [20]

After a tens word a duration would be 'seconds', and 'a twenty second delay' already reads as 22 through the _ #NN rule, so the left-right model now tags it as an ordinal. 'twenty seconds', 'one second' and '30 second' stay units. Tests added to units.test.js and to the dates plugin's sentences test; they fail on dev and pass with the change. The full suite and the plugin suites pass.

I used Claude Code to write this change.

'second' after any cardinal became a unit, so 'may twenty-second' was
read as may 20th, and toOrdinal()'s own 'twenty second' parsed back as
20. A duration after a tens word would be 'seconds' (or a 'twenty
second delay', which is already an ordinal), so tag it as an ordinal.

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

Copy link
Copy Markdown
Owner

yikes! thank you.

@spencermountain
spencermountain merged commit c205148 into spencermountain:dev Nov 3, 2026
1 check passed
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