From 0dbd182eb6f04922b48ed98cd4f94f61808df032 Mon Sep 17 00:00:00 2001 From: Rohit Chandani Date: Mon, 26 Oct 2026 11:56:37 -0700 Subject: [PATCH 1/3] fix(itn/en): don't add up number words that aren't one number grammatical_words_to_number added every ones or tens word to the running total whatever came before it, so "ten thirty" read as 40 and "twenty one forty two" as 2043. Since 9035284 the digit-by-digit reading also applied after "and" was stripped, so "five and six" read as 56. Track what the previous word filled (unit, tens or a scale word) and return None for a run that isn't one number; the sentence loop then falls back to shorter spans. Digit-by-digit reading now requires the run to contain no "and". --- src/itn/en/cardinal.rs | 72 ++++++++++++++++++++++++++++++++++++---- tests/en_tests.rs | 25 +++++++------- tests/extensive_tests.rs | 10 +++--- 3 files changed, 82 insertions(+), 25 deletions(-) diff --git a/src/itn/en/cardinal.rs b/src/itn/en/cardinal.rs index 7047a87..6501643 100644 --- a/src/itn/en/cardinal.rs +++ b/src/itn/en/cardinal.rs @@ -191,7 +191,11 @@ pub fn words_to_number(input: &str) -> Option { // forms are only digits in the context of a longer code (e.g. "oh oh // seven"). Single-token "zero" / "one" / ... fall through to grammatical // and resolve correctly there. - if words.len() >= 2 && words.iter().all(|w| single_digit_char(w).is_some()) { + // Digits joined by "and" are a list ("five and six"), not a code. + if words.len() >= 2 + && words.len() == unfiltered.len() + && words.iter().all(|w| single_digit_char(w).is_some()) + { return words .iter() .map(|w| single_digit_char(w).unwrap()) @@ -240,7 +244,11 @@ pub fn words_to_number_aviation(input: &str) -> Option { // Digit-by-digit reading wins when unambiguous (multi-token only — see // [`words_to_number`] for the rationale on rejecting bare "oh" / "o"). - if words.len() >= 2 && words.iter().all(|w| single_digit_char(w).is_some()) { + // Digits joined by "and" are a list ("five and six"), not a code. + if words.len() >= 2 + && words.len() == unfiltered.len() + && words.iter().all(|w| single_digit_char(w).is_some()) + { return words .iter() .map(|w| single_digit_char(w).unwrap()) @@ -341,15 +349,41 @@ fn grammatical_words_to_number(words: &[&str]) -> Option { let mut result: i128 = 0; let mut current: i128 = 0; let mut found_number = false; + // What the previous word filled, so a run that isn't one number ("five + // six", "ten thirty", "twenty thirty") is rejected instead of summed. + // A ones word may follow a tens word or a scale; a teen or tens word + // only a scale (or nothing). + #[derive(PartialEq)] + enum Last { + Start, + Unit, + Tens, + Scale, + } + let mut last = Last::Start; for &word in words { if let Some(&val) = ONES.get(word) { + let allowed = if val < 10 { + last != Last::Unit + } else { + last == Last::Start || last == Last::Scale + }; + if !allowed { + return None; + } current += val as i128; found_number = true; + last = Last::Unit; } else if let Some(&val) = TENS.get(word) { + if !(last == Last::Start || last == Last::Scale) { + return None; + } current += val as i128; found_number = true; + last = Last::Tens; } else if word == "hundred" { + last = Last::Scale; if current == 0 { current = 1; } @@ -364,6 +398,7 @@ fn grammatical_words_to_number(words: &[&str]) -> Option { result += current; current = 0; found_number = true; + last = Last::Scale; } } else { return None; @@ -540,12 +575,37 @@ mod tests { } /// Generic `words_to_number` (the dispatch path) must NOT do aviation - /// reading: "seven eighty eight" stays grammatical 95 there, so date/time - /// taggers see consistent values. + /// reading, and a run that isn't one number is not summed either: + /// "seven eighty eight" is neither 788 nor 95 there. #[test] fn test_words_to_number_no_aviation_reading() { - assert_eq!(words_to_number("seven eighty eight"), Some(95)); - assert_eq!(words_to_number("twenty one forty two"), Some(63)); + assert_eq!(words_to_number("seven eighty eight"), None); + assert_eq!(words_to_number("twenty one forty two"), None); assert_eq!(words_to_number("two thousand seventeen"), Some(2017)); } + + #[test] + fn test_runs_that_are_not_one_number() { + for s in [ + "five and six", + "one and two", + "ten thirty", + "ten thirty five", + "twenty thirty", + "twenty and thirty", + "fifteen five", + "three thousand nine hundred nine nine", + ] { + assert_eq!(words_to_number(s), None, "{s}"); + } + // All single digits, no "and": still the digit-by-digit reading. + assert_eq!(words_to_number("one two three"), Some(123)); + assert_eq!(words_to_number("two thousand and five"), Some(2005)); + assert_eq!(words_to_number("one hundred and five"), Some(105)); + assert_eq!(words_to_number("three thousand nine hundred nine"), Some(3909)); + assert_eq!(words_to_number("twenty one hundred"), Some(2100)); + assert_eq!(words_to_number("nineteen hundred eighty four"), Some(1984)); + assert_eq!(words_to_number("a hundred and twelve"), Some(112)); + assert_eq!(words_to_number("one million two hundred thousand"), Some(1_200_000)); + } } diff --git a/tests/en_tests.rs b/tests/en_tests.rs index 084a0bc..8e9a715 100644 --- a/tests/en_tests.rs +++ b/tests/en_tests.rs @@ -583,21 +583,20 @@ fn test_max_span_tokens() { #[test] fn test_sentence_adjacent_spans() { - // Two number spans right next to each other — note the sliding window tries - // longest span first, so "twenty one forty two" → 2043 as one cardinal. - // This is correct behavior: the algorithm prefers the longest match. - assert_eq!(normalize_sentence("twenty one forty two"), "2043"); - // "and" is a number conjunction in English ("one hundred and twenty"), - // so "twenty one and forty two" → 2043 as one cardinal - assert_eq!(normalize_sentence("twenty one and forty two"), "2043"); + // Two number spans right next to each other: "twenty one forty two" isn't + // one number, so it is read as two rather than summed to 2043. + assert_eq!(normalize_sentence("twenty one forty two"), "21 42"); + // "and" joins parts of one number ("one hundred and twenty"), not two + // complete numbers. + assert_eq!(normalize_sentence("twenty one and forty two"), "21 and 42"); // With a non-number word separator, they parse as two spans assert_eq!( normalize_sentence("twenty one versus forty two"), "21 versus 42" ); - // "twenty one five dollars" — money tagger matches the longest span including - // the number prefix, so this becomes "$26" (twenty-one + five = 26 dollars) - assert_eq!(normalize_sentence("twenty one five dollars"), "$26"); + // "twenty one five dollars" is not 26 dollars (the money tagger no longer + // sums the prefix; what wins instead is the date tagger's old-year read). + assert_ne!(normalize_sentence("twenty one five dollars"), "$26"); // With a separator, they split correctly assert_eq!( normalize_sentence("twenty one then five dollars"), @@ -923,9 +922,9 @@ fn test_issue_14_default_dispatch_unchanged() { assert_eq!(normalize("one hundred"), "100"); assert_eq!(normalize_sentence("two thousand seventeen"), "2017"); - // Sentence-mode default dispatch: "seven eighty eight" stays grammatical - // 95, NOT aviation 788. - assert_eq!(normalize_sentence("seven eighty eight"), "95"); + // Sentence-mode default dispatch: "seven eighty eight" is NOT aviation + // 788, and not the sum 95 either: it isn't one number. + assert_eq!(normalize_sentence("seven eighty eight"), "7 88"); // Single-input `normalize` keeps its existing telephone-tagger // behaviour for whole-input flight-number-style phrases. diff --git a/tests/extensive_tests.rs b/tests/extensive_tests.rs index 377d3bf..5de0e9c 100644 --- a/tests/extensive_tests.rs +++ b/tests/extensive_tests.rs @@ -970,13 +970,11 @@ fn test_sentence_itn_max_span_tokens() { #[test] fn test_sentence_itn_adjacent_numbers() { - // KNOWN_ISSUE: Adjacent number words get combined by the cardinal tagger. - // "twenty one forty two" is parsed as a single cardinal: 20+1+40+2 = 2043 - // The sliding window tries the longest span first (all 4 tokens), - // and cardinal happily parses them all as one number. - // This is inherent to the greedy longest-match algorithm. + // Adjacent numbers that aren't one number are not summed (was 2043): the + // cardinal reader rejects the 4-token span, so the window falls back to + // the two 2-token spans. let result = normalize_sentence("twenty one forty two"); - assert_eq!(result, "2043"); + assert_eq!(result, "21 42"); } // ════════════════════════════════════════════════════════════════════════ From f78d8de95326dd7ea84f9d18ea9590b99ad23972 Mon Sep 17 00:00:00 2001 From: Alex-Wengg Date: Tue, 27 Oct 2026 15:44:40 -0400 Subject: [PATCH 2/3] style(itn/en): cargo fmt the new cardinal tests --- src/itn/en/cardinal.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/itn/en/cardinal.rs b/src/itn/en/cardinal.rs index 6501643..ba06ebe 100644 --- a/src/itn/en/cardinal.rs +++ b/src/itn/en/cardinal.rs @@ -602,10 +602,16 @@ mod tests { assert_eq!(words_to_number("one two three"), Some(123)); assert_eq!(words_to_number("two thousand and five"), Some(2005)); assert_eq!(words_to_number("one hundred and five"), Some(105)); - assert_eq!(words_to_number("three thousand nine hundred nine"), Some(3909)); + assert_eq!( + words_to_number("three thousand nine hundred nine"), + Some(3909) + ); assert_eq!(words_to_number("twenty one hundred"), Some(2100)); assert_eq!(words_to_number("nineteen hundred eighty four"), Some(1984)); assert_eq!(words_to_number("a hundred and twelve"), Some(112)); - assert_eq!(words_to_number("one million two hundred thousand"), Some(1_200_000)); + assert_eq!( + words_to_number("one million two hundred thousand"), + Some(1_200_000) + ); } } From fdedaa182de607b57df383d540480df826099dad Mon Sep 17 00:00:00 2001 From: Alex-Wengg Date: Sun, 1 Nov 2026 15:37:59 -0500 Subject: [PATCH 3/3] fix(itn/en): address review on non-summing cardinals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - telephone: four to six spoken digits are not a phone number. With runs like "ten thirty" no longer summed, whole-input normalize() fell through to the telephone tagger and formatted them as "103-0" / "202-0"; they now stay as written, and "one two three four" / "nine four one zero five" read as 1234 / 94105 instead of "123-4" / "941-05". - cardinal: move the Last enum to module level, fix the doc comments that still said "twenty one forty two" reads as the year 2042, and note that the digit-by-digit check also excludes runs with "a". - tests: pin "twenty one five dollars" → "2015 dollars" as a KNOWN_ISSUE, and assert 100 × "twenty one" → 100 × "21" now that adjacent numbers aren't combined. --- src/itn/en/cardinal.rs | 49 +++++++++++++++++++++------------------- src/itn/en/telephone.rs | 25 ++++++++++++++++++-- tests/en_tests.rs | 11 ++++++--- tests/extensive_tests.rs | 10 +++----- 4 files changed, 60 insertions(+), 35 deletions(-) diff --git a/src/itn/en/cardinal.rs b/src/itn/en/cardinal.rs index ba06ebe..ec9505c 100644 --- a/src/itn/en/cardinal.rs +++ b/src/itn/en/cardinal.rs @@ -69,6 +69,18 @@ lazy_static! { }; } +/// What the previous word of a grammatical number filled, so a run that +/// isn't one number ("five six", "ten thirty", "twenty thirty") is rejected +/// instead of summed: a ones word may follow a tens word or a scale word, a +/// teen or tens word only a scale word (or nothing). +#[derive(PartialEq)] +enum Last { + Start, + Unit, + Tens, + Scale, +} + /// Parse spoken cardinal number to string representation. /// /// Returns None if the input cannot be parsed as a number. @@ -162,13 +174,14 @@ fn single_digit_char(word: &str) -> Option { /// four"` → `1234`. Uses a left-to-right accumulator with scale words /// multiplying the current group. /// -/// Filler words `"and"` and `"a"` are stripped. +/// Filler words `"and"` and `"a"` are stripped. A run of ones, teens and +/// tens words that isn't one number is `None`, not a sum: `"ten thirty"`, +/// `"five six"`, `"twenty one forty two"`. /// /// Note: aviation flight-number reading (`"seven eighty eight"` → `788`) is -/// **not** applied here because it conflicts with date and time taggers (e.g. -/// `"twenty one forty two"` must remain readable as old-year `2042` for -/// `date::parse_old_year`). Use [`words_to_number_aviation`] for opt-in -/// flight-number / call-sign contexts. +/// **not** applied here because it conflicts with the time tagger (e.g. +/// `"two thirty five"` is the time 02:35). Use [`words_to_number_aviation`] +/// for opt-in flight-number / call-sign contexts. pub fn words_to_number(input: &str) -> Option { let input = input.to_lowercase().replace('-', " "); let unfiltered: Vec<&str> = input.split_whitespace().collect(); @@ -191,7 +204,8 @@ pub fn words_to_number(input: &str) -> Option { // forms are only digits in the context of a longer code (e.g. "oh oh // seven"). Single-token "zero" / "one" / ... fall through to grammatical // and resolve correctly there. - // Digits joined by "and" are a list ("five and six"), not a code. + // A run with a filler word ("and", "a") isn't a code: "five and six" is + // a list. if words.len() >= 2 && words.len() == unfiltered.len() && words.iter().all(|w| single_digit_char(w).is_some()) @@ -223,8 +237,8 @@ pub fn words_to_number(input: &str) -> Option { /// /// This is **opt-in**: callers reach for it explicitly from flight-number / /// call-sign contexts. Generic ITN/TN dispatch keeps using [`words_to_number`] -/// to avoid clobbering date/time/measure semantics (e.g. `"twenty one forty -/// two"` as old-year `2042`). +/// to avoid clobbering date/time/measure semantics (e.g. `"two thirty five"` +/// as the time 02:35). pub fn words_to_number_aviation(input: &str) -> Option { let input = input.to_lowercase().replace('-', " "); let unfiltered: Vec<&str> = input.split_whitespace().collect(); @@ -244,7 +258,8 @@ pub fn words_to_number_aviation(input: &str) -> Option { // Digit-by-digit reading wins when unambiguous (multi-token only — see // [`words_to_number`] for the rationale on rejecting bare "oh" / "o"). - // Digits joined by "and" are a list ("five and six"), not a code. + // A run with a filler word ("and", "a") isn't a code: "five and six" is + // a list. if words.len() >= 2 && words.len() == unfiltered.len() && words.iter().all(|w| single_digit_char(w).is_some()) @@ -349,17 +364,6 @@ fn grammatical_words_to_number(words: &[&str]) -> Option { let mut result: i128 = 0; let mut current: i128 = 0; let mut found_number = false; - // What the previous word filled, so a run that isn't one number ("five - // six", "ten thirty", "twenty thirty") is rejected instead of summed. - // A ones word may follow a tens word or a scale; a teen or tens word - // only a scale (or nothing). - #[derive(PartialEq)] - enum Last { - Start, - Unit, - Tens, - Scale, - } let mut last = Last::Start; for &word in words { @@ -537,9 +541,8 @@ mod tests { /// Aviation flight-number style (issue #14): opt-in helper. A leading run /// of single-digit words gets concatenated with the trailing grammatical /// compound, e.g. "seven eighty eight" = "7" ‖ 88 = 788. Generic - /// `words_to_number` deliberately does *not* do this — it would break - /// `date::parse_old_year` ("twenty one forty two" → 2042) and overlap with - /// the time tagger ("two thirty five" → 02:35). + /// `words_to_number` deliberately does *not* do this — it would overlap + /// with the time tagger ("two thirty five" → 02:35). #[test] fn test_words_to_number_aviation_flight_number() { assert_eq!(words_to_number_aviation("seven eighty eight"), Some(788)); diff --git a/src/itn/en/telephone.rs b/src/itn/en/telephone.rs index 45ece4a..19ad518 100644 --- a/src/itn/en/telephone.rs +++ b/src/itn/en/telephone.rs @@ -387,8 +387,11 @@ fn parse_phone_number(input: &str) -> Option { let (prefix, rest) = extract_phone_prefix(input); let digits = parse_digit_sequence_with_double(rest)?; - // Must have at least 7 digits for phone number (or 3 for short codes) - if !has_plus && digits.len() < 3 { + // Must have at least 7 digits for phone number (or 3 for short codes). + // Four to six spoken digits ("ten thirty" → 1030, "twenty twenty", + // "nine four one zero five") are a number, a year or a code, not a phone + // number, so they are left to the other taggers instead of "103-0". + if !has_plus && (digits.len() < 3 || (4..7).contains(&digits.len())) { return None; } @@ -719,6 +722,24 @@ mod tests { assert_eq!(parse("seven nine nine"), Some("799".to_string())); } + #[test] + fn test_four_to_six_digits_are_not_a_phone_number() { + for s in [ + "ten thirty", + "twenty twenty", + "thirty five sixty two", + "one two three four", + "nine four one zero five", + "twenty thirty forty", + ] { + assert_eq!(parse(s), None, "{s}"); + } + assert_eq!( + parse("five five five one two one two"), + Some("555-1212".to_string()) + ); + } + #[test] fn test_ip_address() { assert_eq!( diff --git a/tests/en_tests.rs b/tests/en_tests.rs index 8e9a715..b69a4d5 100644 --- a/tests/en_tests.rs +++ b/tests/en_tests.rs @@ -594,9 +594,14 @@ fn test_sentence_adjacent_spans() { normalize_sentence("twenty one versus forty two"), "21 versus 42" ); - // "twenty one five dollars" is not 26 dollars (the money tagger no longer - // sums the prefix; what wins instead is the date tagger's old-year read). - assert_ne!(normalize_sentence("twenty one five dollars"), "$26"); + // "twenty one five dollars" is no longer 26 dollars: the money tagger + // doesn't sum the prefix any more. + // KNOWN_ISSUE: the date tagger's old-year reading wins instead, giving + // "2015 dollars"; pinned so the next change to this path shows up here. + assert_eq!( + normalize_sentence("twenty one five dollars"), + "2015 dollars" + ); // With a separator, they split correctly assert_eq!( normalize_sentence("twenty one then five dollars"), diff --git a/tests/extensive_tests.rs b/tests/extensive_tests.rs index 5de0e9c..fb0df6a 100644 --- a/tests/extensive_tests.rs +++ b/tests/extensive_tests.rs @@ -1183,15 +1183,11 @@ fn test_boundary_unicode() { #[test] fn test_boundary_very_long_input() { - // A very long sentence shouldn't crash. - // KNOWN_ISSUE: Similar to adjacent numbers, 100 repetitions of "twenty one" - // get combined by the cardinal tagger's greedy span matching (up to 4 tokens - // at a time in sentence mode). The result is a series of combined numbers - // rather than individual "21"s. The important thing is it doesn't crash. + // A very long sentence shouldn't crash, and adjacent numbers aren't + // combined: each "twenty one" stays its own 21. let long_input = "twenty one ".repeat(100); let result = normalize_sentence(long_input.trim()); - assert!(!result.is_empty(), "Should not return empty for long input"); - // Don't assert specific content — the greedy algorithm combines tokens + assert_eq!(result, vec!["21"; 100].join(" ")); } #[test]