From 60c55ab8c70183dd4e58ba1603647afc1c8b55ab Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 2 Apr 2026 21:49:26 +0000 Subject: [PATCH 1/3] fix(newmm): resolve BFS path explosion in ambiguous tokenization - Add visited set to bfs_paths_graph to prevent re-exploring already-visited nodes, reducing worst-case BFS from O(2^n) to O(V+E) - Clear ambiguity graph after each commit point in one_cut to prevent unbounded edge accumulation - Remove unnecessary graph edge insertion in the no-candidate branch - Add test_newmm_ambiguous_performance regression test - Update CHANGELOG.md Agent-Logs-Url: https://github.com/PyThaiNLP/nlpo3/sessions/b1879073-e5b6-44a8-8492-7eb13a0cb26c Co-authored-by: bact <128572+bact@users.noreply.github.com> --- CHANGELOG.md | 19 +++++++++++++++ src/tokenizer/newmm.rs | 29 +++++++++++++---------- tests/test_tokenizer.rs | 52 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 88 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 634f1734..d5e28a4e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,25 @@ conventions. Version numbers follow [Semantic Versioning](https://semver.org/). ## [Unreleased] +### Fixed + +- `src/tokenizer/newmm.rs`: Resolved exponential BFS path explosion in + `bfs_paths_graph`. Added a `visited` set to prevent re-exploring nodes + already reached, reducing worst-case complexity from O(2^n) to O(V + E). + Mirrors the fix in + [PyThaiNLP/pythainlp#1319](https://github.com/PyThaiNLP/pythainlp/pull/1319). +- `src/tokenizer/newmm.rs`: Clear the ambiguity graph after each commit point + in `one_cut` to prevent unbounded edge accumulation across segments. +- `src/tokenizer/newmm.rs`: Remove the unnecessary graph edge insertion in the + no-candidate branch of `one_cut`; the graph is now cleared there instead, + consistent with the commit-point branch. + +### Added + +- `tests/test_tokenizer.rs`: Added `test_newmm_ambiguous_performance` regression + test that verifies tokenization of highly ambiguous input completes within + one second. + ## [2.0.0] - 2026-04-01 ### Added diff --git a/src/tokenizer/newmm.rs b/src/tokenizer/newmm.rs index 405ac033..9aaf19d7 100644 --- a/src/tokenizer/newmm.rs +++ b/src/tokenizer/newmm.rs @@ -278,6 +278,12 @@ impl NewmmTokenizer { ) -> AnyResult> { current_queue.clear(); + // The visited set prevents re-exploring nodes already reached via a + // shorter path, converting worst-case BFS from O(2^n) to O(V + E). + let mut visited: HashSet = + HashSet::with_capacity_and_hasher(goal - start, Default::default()); + visited.insert(start); + let mut init_path: Vec = Vec::with_capacity(goal - start); init_path.push(start); current_queue.push_back((start, init_path)); @@ -285,15 +291,16 @@ impl NewmmTokenizer { while let Some((vertex, path)) = current_queue.pop_front() { if let Some(idk) = graph.get(&vertex) { for &position in idk { - if position != goal { - let mut appended_path = path.clone(); - appended_path.push(position); - current_queue.push_back((position, appended_path)); - } else { + if position == goal { let mut appended_path = path; appended_path.push(position); return Ok(appended_path); - }; + } else if !visited.contains(&position) { + visited.insert(position); + let mut appended_path = path.clone(); + appended_path.push(position); + current_queue.push_back((position, appended_path)); + } } }; } @@ -354,7 +361,8 @@ impl NewmmTokenizer { first_position_list, &mut reused_queue, )?; - graph_size = 0; // reset graph + graph_size = 0; + graph.clear(); for &position in group_of_end_position_candidate.iter().skip(1) { let token = text.substring_as_str(end_position, position); @@ -421,11 +429,8 @@ impl NewmmTokenizer { } } - graph - .entry(begin_position) - .or_insert_with(|| Vec::with_capacity(10)) - .push(end_position); - graph_size += 1; + graph_size = 0; + graph.clear(); let token = text.substring_as_str(begin_position, end_position); result_str.push(token); position_list.push(end_position); diff --git a/tests/test_tokenizer.rs b/tests/test_tokenizer.rs index 17eb1afe..a2021ace 100644 --- a/tests/test_tokenizer.rs +++ b/tests/test_tokenizer.rs @@ -366,3 +366,55 @@ fn test_tokenizer_trait_switchable() { fst.segment_to_string(text, false, false), ); } + +/// Regression test for BFS path explosion in `_bfs_paths_graph`. +/// +/// Without the visited-set fix, highly ambiguous tokenization input causes +/// the BFS queue to grow exponentially (O(2^n)), making this test run for +/// many seconds or minutes. With the fix, it must complete in well under +/// one second. +#[test] +fn test_newmm_ambiguous_performance() { + use std::time::{Duration, Instant}; + + // Build a dictionary with heavily overlapping words (lengths 1–3) using + // a small set of standalone Thai consonants. Each consonant is its own + // Thai Character Cluster (TCC), so every character boundary is a valid + // split point, maximising the number of ambiguous paths. + let chars = ["ก", "ข", "ค", "ง", "จ"]; + let mut words: Vec = Vec::new(); + for &c in &chars { + words.push(c.to_string()); + } + for &c1 in &chars { + for &c2 in &chars { + words.push(format!("{c1}{c2}")); + } + } + for &c1 in &chars { + for &c2 in &chars { + for &c3 in &chars { + words.push(format!("{c1}{c2}{c3}")); + } + } + } + + let tokenizer = NewmmTokenizer::from_word_list(words); + + // 50 repetitions of a five-consonant sequence = 250 characters. + // Without the fix this input causes exponential BFS expansion. + let text = "กขคงจ".repeat(50); + + let start = Instant::now(); + let result = tokenizer.segment_to_string(&text, false, false); + let elapsed = start.elapsed(); + + assert!( + !result.is_empty(), + "tokenizer must produce at least one token" + ); + assert!( + elapsed < Duration::from_secs(1), + "tokenization took {elapsed:?}; BFS path explosion may still be present" + ); +} From 190ce397b97192832ad1e50b61d82e5d65ad6939 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 2 Apr 2026 22:47:58 +0000 Subject: [PATCH 2/3] fix: address PR review comments on comments and spelling - Fix doc comment: _bfs_paths_graph -> bfs_paths_graph in test - Fix spelling: maximising -> maximizing (American English) - Reword BFS comment: drop inaccurate O(V+E) claim; say each vertex is enqueued at most once, avoiding exponential blow-up - Update CHANGELOG: replace O(V+E) claim with accurate description of avoiding exponential queue growth Agent-Logs-Url: https://github.com/PyThaiNLP/nlpo3/sessions/b8052556-3598-419d-80e8-7af979d714e3 Co-authored-by: bact <128572+bact@users.noreply.github.com> --- CHANGELOG.md | 4 ++-- src/tokenizer/newmm.rs | 4 ++-- tests/test_tokenizer.rs | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d5e28a4e..b3e7f5eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,8 +15,8 @@ conventions. Version numbers follow [Semantic Versioning](https://semver.org/). ### Fixed - `src/tokenizer/newmm.rs`: Resolved exponential BFS path explosion in - `bfs_paths_graph`. Added a `visited` set to prevent re-exploring nodes - already reached, reducing worst-case complexity from O(2^n) to O(V + E). + `bfs_paths_graph`. Added a `visited` set to prevent re-enqueueing and + re-exploring nodes already reached, avoiding exponential queue growth. Mirrors the fix in [PyThaiNLP/pythainlp#1319](https://github.com/PyThaiNLP/pythainlp/pull/1319). - `src/tokenizer/newmm.rs`: Clear the ambiguity graph after each commit point diff --git a/src/tokenizer/newmm.rs b/src/tokenizer/newmm.rs index 9aaf19d7..e3dc5cbb 100644 --- a/src/tokenizer/newmm.rs +++ b/src/tokenizer/newmm.rs @@ -278,8 +278,8 @@ impl NewmmTokenizer { ) -> AnyResult> { current_queue.clear(); - // The visited set prevents re-exploring nodes already reached via a - // shorter path, converting worst-case BFS from O(2^n) to O(V + E). + // The visited set ensures each vertex is enqueued at most once. + // This avoids the exponential blow-up from revisiting the same nodes. let mut visited: HashSet = HashSet::with_capacity_and_hasher(goal - start, Default::default()); visited.insert(start); diff --git a/tests/test_tokenizer.rs b/tests/test_tokenizer.rs index a2021ace..d1417c15 100644 --- a/tests/test_tokenizer.rs +++ b/tests/test_tokenizer.rs @@ -367,7 +367,7 @@ fn test_tokenizer_trait_switchable() { ); } -/// Regression test for BFS path explosion in `_bfs_paths_graph`. +/// Regression test for BFS path explosion in `bfs_paths_graph`. /// /// Without the visited-set fix, highly ambiguous tokenization input causes /// the BFS queue to grow exponentially (O(2^n)), making this test run for @@ -380,7 +380,7 @@ fn test_newmm_ambiguous_performance() { // Build a dictionary with heavily overlapping words (lengths 1–3) using // a small set of standalone Thai consonants. Each consonant is its own // Thai Character Cluster (TCC), so every character boundary is a valid - // split point, maximising the number of ambiguous paths. + // split point, maximizing the number of ambiguous paths. let chars = ["ก", "ข", "ค", "ง", "จ"]; let mut words: Vec = Vec::new(); for &c in &chars { From 852b9daf16a6d51e69cc0ea18b0bb899f1baea98 Mon Sep 17 00:00:00 2001 From: Arthit Suriyawongkul Date: Fri, 3 Apr 2026 00:20:01 +0100 Subject: [PATCH 3/3] Update CHANGELOG.md --- CHANGELOG.md | 61 ++++++++++------------------------------------------ 1 file changed, 11 insertions(+), 50 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b3e7f5eb..dca58ea7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,28 +10,7 @@ All notable changes to this project are documented in this file. This file follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/) conventions. Version numbers follow [Semantic Versioning](https://semver.org/). -## [Unreleased] - -### Fixed - -- `src/tokenizer/newmm.rs`: Resolved exponential BFS path explosion in - `bfs_paths_graph`. Added a `visited` set to prevent re-enqueueing and - re-exploring nodes already reached, avoiding exponential queue growth. - Mirrors the fix in - [PyThaiNLP/pythainlp#1319](https://github.com/PyThaiNLP/pythainlp/pull/1319). -- `src/tokenizer/newmm.rs`: Clear the ambiguity graph after each commit point - in `one_cut` to prevent unbounded edge accumulation across segments. -- `src/tokenizer/newmm.rs`: Remove the unnecessary graph edge insertion in the - no-candidate branch of `one_cut`; the graph is now cleared there instead, - consistent with the commit-point branch. - -### Added - -- `tests/test_tokenizer.rs`: Added `test_newmm_ambiguous_performance` regression - test that verifies tokenization of highly ambiguous input completes within - one second. - -## [2.0.0] - 2026-04-01 +## [2.0.0] - 2026-04-03 ### Added @@ -122,18 +101,24 @@ conventions. Version numbers follow [Semantic Versioning](https://semver.org/). Use tokenizer class instances instead. - Custom four-byte string implementation removed. Use `CharString` instead. +### Fixed + +- `src/tokenizer/newmm.rs`: Resolved exponential BFS path explosion in + `bfs_paths_graph` (#101). + - Mirrors the fix in PyThaiNLP/pythainlp#1319. + ### Migration guide #### Python ```python -# Before (v1.x / early v2.0) +# Before (v1.x) from nlpo3 import load_dict, segment, DeepcutTokenizer, segment_deepcut load_dict("path/to/dict.txt", "mydict") tokens = segment("สวัสดีครับ", "mydict") tokens = segment_deepcut("สวัสดีครับ") -# After (v2.0) +# Now (v2.0) from nlpo3 import NewmmTokenizer, NewmmFstTokenizer, DeepcutTokenizer tok = NewmmTokenizer("path/to/dict.txt") tokens = tok.segment("สวัสดีครับ") @@ -143,40 +128,16 @@ tokens = DeepcutTokenizer().segment("สวัสดีครับ") #### Node.js / TypeScript ```typescript -// Before (v1.x / early v2.0) +// Before (v1.x) loadDict("path/to/dict.txt", "mydict"); const tokens = segment("สวัสดีครับ", "mydict", false, false); -// After (v2.0) +// Now (v2.0) import { NewmmTokenizer } from "nlpo3-nodejs"; const tok = new NewmmTokenizer("path/to/dict.txt"); const tokens = tok.segment("สวัสดีครับ"); ``` -#### CLI - -```bash -# Before -echo "สวัสดีครับ" | nlpo3 segment --dict-path /path/to/dict.txt - -# After (newmm is still default, dict-path still works) -echo "สวัสดีครับ" | nlpo3 segment --dict-path /path/to/dict.txt - -# New: choose tokenizer -echo "สวัสดีครับ" | nlpo3 segment -t nf -echo "สวัสดีครับ" | nlpo3 segment -t deepcut -``` - -### Performance (from `BENCHMARK_RESULTS.md`) - -All three tokenizers use the same `Tokenizer` trait and are interchangeable: - -| Tokenizer | short (28 ch) | long (937 ch) | Dict memory | -| --------- | ------------- | ------------- | ----------- | -| `NewmmTokenizer` | **2.63 µs** | **165 µs** | ~43 MB | -| `NewmmFstTokenizer` | 29.5 µs | 2 225 µs | **~0.85 MB** | -| `DeepcutTokenizer` | - | - | ~3.9 MB model | - ## [1.4.0] - 2024-11-09 ### Changed