diff --git a/src/itn/en/cardinal.rs b/src/itn/en/cardinal.rs index 7047a87..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,12 @@ 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()) { + // 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()) + { return words .iter() .map(|w| single_digit_char(w).unwrap()) @@ -219,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(); @@ -240,7 +258,12 @@ 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()) { + // 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()) + { return words .iter() .map(|w| single_digit_char(w).unwrap()) @@ -341,15 +364,30 @@ fn grammatical_words_to_number(words: &[&str]) -> Option { let mut result: i128 = 0; let mut current: i128 = 0; let mut found_number = false; + 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 +402,7 @@ fn grammatical_words_to_number(words: &[&str]) -> Option { result += current; current = 0; found_number = true; + last = Last::Scale; } } else { return None; @@ -502,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)); @@ -540,12 +578,43 @@ 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/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 084a0bc..b69a4d5 100644 --- a/tests/en_tests.rs +++ b/tests/en_tests.rs @@ -583,21 +583,25 @@ 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 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"), @@ -923,9 +927,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..fb0df6a 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"); } // ════════════════════════════════════════════════════════════════════════ @@ -1185,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]