docs(algorithm): update walk algorithm documentation for Viterbi by tianjianjiang · Pull Request #785 · openvanilla/McBopomofo

tianjianjiang · 2026-02-14T14:50:23Z

Summary

Replace the DAG shortest-path algorithm documentation in algorithm.md with the Viterbi implementation merged in Refactor DAG shortest path algorithm using Viterbi #777
Document the linear lattice forward pass with relaxation and backward path reconstruction (2 steps instead of 4)
Update code reference table, TOC link, and reading references
Bump document version to 1.3

#785 (independent, base: master)

Test plan

Verify all internal TOC links resolve correctly
Verify code snippets match master:Source/Engine/gramambular2/reading_grid.cpp
Verify no stale DAG/TopologicalSort/Relax references remain

Generated with Claude Code

claude

Critical Issue Found

The complexity analysis in line 275 is technically incorrect and could mislead readers about the nature of the performance improvement. See inline comment for details.

gemini-code-assist

Code Review

這份 PR 更新了 algorithm.md 文件，用 Viterbi 演算法的說明取代了舊的 DAG 最短路徑演算法文件。新的文件清晰地解釋了 Viterbi 演算法的兩個主要步驟：前向傳遞和回溯路徑，並提供了對應的程式碼片段和複雜度分析。我也確認了文件中的目錄連結、程式碼參考表格以及延伸閱讀部分都已正確更新。整體而言，這是一次很棒的文件更新，提高了演算法說明的準確性和清晰度。我沒有發現任何問題。

Copilot

Pull request overview

This PR updates the algorithm.md documentation to accurately reflect the Viterbi algorithm implementation that replaced the previous DAG shortest-path approach in PR #777. The documentation now describes a streamlined 2-step process (forward pass with relaxation + backward path reconstruction) instead of the previous 4-step DAG algorithm.

Changes:

Replaced DAG shortest-path algorithm documentation with Viterbi algorithm explanation
Updated TOC link from "最佳路徑演算法：DAG 最短路徑" to "最佳路徑演算法：Viterbi"
Removed code reference table entries for deprecated TopologicalSort() and Relax() functions
Updated reading references to cite Jurafsky & Martin's textbook and vene.ro lattice Viterbi resource
Bumped document version to 1.3 with updated timestamp

github-actions · 2026-02-14T18:36:24Z

Claude Code Review Failed

The automated Claude review encountered an error and could not complete. You can:

Check the workflow logs for details
Trigger a manual review by commenting @claude on this PR
The review will be retried automatically on the next push

This does not affect the PR approval process.

claude

Documentation review complete. Found 2 critical issues requiring attention.

claude

Critical Issues

Line reference error: The main walk() method reference points to line 216, but the actual implementation is at line 132. This must be corrected.

Code snippet accuracy: The documentation code snippets omit important implementation details (comments, assertions, metrics tracking) that exist in the actual code. While educational simplification is reasonable, key correctness guarantees like the totalReadingLen assertion should be included.

All other aspects (algorithm description, complexity analysis, reference updates) appear correct and consistent with the merged Viterbi implementation in #777.

claude

Reviewed documentation update for Viterbi algorithm. Found two critical issues requiring attention:

Missing line number reference in code reference table (inconsistent with other entries)
Unverified complexity and performance claims that need fact-checking against PR #777 or removal

The code snippets accurately match the implementation in reading_grid.cpp:132-198.

github-actions · 2026-02-14T22:34:19Z

Claude Code Review Failed

The automated Claude review encountered an error and could not complete. You can:

Check the workflow logs for details
Trigger a manual review by commenting @claude on this PR
The review will be retried automatically on the next push

This does not affect the PR approval process.

github-actions · 2026-02-15T08:44:06Z

Claude Code Review Failed

The automated Claude review encountered an error and could not complete. You can:

Check the workflow logs for details
Trigger a manual review by commenting @claude on this PR
The review will be retried automatically on the next push

This does not affect the PR approval process.

github-actions · 2026-02-26T17:23:15Z

Claude Code Review Failed

The automated Claude review encountered an error and could not complete. You can:

Check the workflow logs for details
Trigger a manual review by commenting @claude on this PR
The review will be retried automatically on the next push

This does not affect the PR approval process.

claude

Documentation-only PR. Two issues found:

Non-existent file references (critical): The new "演算法擴展" table references walk_strategy.h/cpp and contextual_user_model.h/cpp, which don't exist in master. These are forward references to files introduced by downstream PRs (#779–#781). Merging this PR as-is leaves broken references in the documentation.
Version number mismatch: PR description says "Bump to 1.3" but the diff sets **文件版本**：2.1.

The Viterbi algorithm description, pseudocode, complexity analysis, and example table are all technically accurate and match the actual implementation in reading_grid.cpp.

github-actions · 2026-02-26T18:07:14Z

Claude Code Review Failed

The automated Claude review encountered an error and could not complete. You can:

Check the workflow logs for details
Trigger a manual review by commenting @claude on this PR
The review will be retried automatically on the next push

This does not affect the PR approval process.

claude

This is a documentation-only PR. Three issues found:

1. "演算法擴展" table references unmerged PRs (critical for readers)
The table entries for walk_strategy.h/cpp (PR #779) and contextual_user_model.h/cpp (PR #780) point to files that don't exist in master yet. This breaks the PR's own test plan item: "Verify code snippets match master:Source/Engine/...". Suggest labeling these rows as pending or deferring them to a follow-up doc PR after #779/#780 land.

2. VITERBI-BACKWARD: no guard against nil fromNode
If walk() is ever called when no complete path exists through the lattice, the backward pass silently appends nil nodes. In normal operation insertReading prevents this via hasUnigrams, but the pseudocode should document the precondition. Inline suggestion added.

3. VITERBI-FORWARD: unreachable positions not skipped
Minor clarity/performance issue — iterating over positions with maxScore = -infinity is harmless but obscures the invariant. Inline suggestion added.

The algorithm descriptions, KN smoothing formulas, and complexity analysis all look technically correct.

The stack consists of 3 stacked PRs (#779, #780, #781) plus 4 independent PRs (#784, #785, #786, #787) targeting master directly, not "a stack of 6 PRs each building on the previous". Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Describe the forward-pass Viterbi DP walk (PR #777) with verified file/line references against current master, an O(|V| + |E|) complexity analysis matching the implementation comment, and measured stress-test numbers (vertices/edges from WalkResult). Replace the dropped WalkStrategy/fixedSpans walk-integration design with the actual candidate-override mechanism (overrideCandidate plus re-walk), and align the contextual user model section with the shipped ContextualUserModel design (PR #780): two-level interpolated Kneser-Ney with per-reading continuation normalization, wall-clock decay with a 5400-second half-life, LRU capacity bound, TSV persistence, and implicit base-LM fallback via empty suggestions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

claude

Documentation-only PR; no code correctness or security concerns. Three issues found:

Unreleased code referenced — The new 情境式使用者模型 section and 演算法擴展 table document ContextualUserModel from PRs #780/#781, which are not yet in master. This breaks the PR's own test-plan item ("Verify code snippets match master:…") and will mislead readers who follow the file paths.
Version number mismatch — PR description says "Bump to 1.3" but the document is set to 1.4.
Missing precondition on backward-pass pseudocode — The assert viterbi[curr].fromNode is not nil is correct but relies on an implicit invariant (every position is reachable) that is only guaranteed because insertReading() validates unigrams. Worth documenting inline so the invariant is explicit.

claude · 2026-06-10T19:44:24Z


 ---

+## 情境式使用者模型


Documentation accuracy: references unreleased code

The new 情境式使用者模型 section (and the 演算法擴展 table later) explicitly references Source/Engine/ContextualUserModel.h / .cpp from PR #780, and Source/KeyHandler.mm changes from PR #781 — neither of which is merged into master yet.

This means:

The test plan item "Verify code snippets match master:Source/Engine/gramambular2/reading_grid.cpp" cannot pass for this section (the referenced files don't exist on master).

Readers on master who follow the file paths will find nothing.

Suggestion: Either gate this section behind a clearly marked callout ("Planned in PR #780, not yet merged"), or extract it into a separate follow-up PR that is stacked on #780/#781.

claude · 2026-06-10T19:44:31Z

 - [X/Twitter 演算法說明串](https://x.com/McBopomofo/status/1559356063622631424)
- DAG 最短路徑演算法：Cormen et al., *Introduction to Algorithms*, 3rd Edition

 ---


Version number mismatch with PR description

The PR summary says "Bump document version to 1.3", but this line sets it to 1.4, skipping 1.3 entirely. Either the PR description or the version number is wrong.

Suggested change

---

**文件版本**：1.3

claude · 2026-06-10T19:44:41Z

+| 轉移機率 | $a_{ij}$（狀態間轉移） | 隱含 = 1（無 bigram 模型） |
+| 發射機率 | $b_j(o_t)$ | `node->score()`（unigram 對數機率） |
+| DP 遞迴式 | $v_t(j) = \max_i [v_{t-1}(i) \cdot a_{ij} \cdot b_j(o_t)]$ | `viterbi[i+L] = max(viterbi[i] + score)` |
+| DP 表格形狀 | N x T（狀態數 x 時間步） | 一維：n+1 個位置 |


Correctness: backward-pass assertion can fail on an empty or unreachable grid

The pseudocode asserts viterbi[curr].fromNode is not nil unconditionally before dereferencing it. If readingLen > 0 but position readingLen was never reached (e.g. there is a gap in the lattice — though the insertReading guard makes this unlikely in practice), the assert fires without any explanation.

Consider adding a precondition note so readers understand the invariant being relied upon:

Suggested change

| DP 表格形狀 | N x T（狀態數 x 時間步） | 一維：n+1 個位置 |

// Precondition: every position 1..readingLen must be reachable;

// insertReading() guarantees this by rejecting readings with no unigrams.

curr = readingLen

while curr > 0:

assert viterbi[curr].fromNode is not nil

append viterbi[curr].fromNode to nodes

curr = viterbi[curr].fromIndex

Copilot AI review requested due to automatic review settings February 14, 2026 14:50

Copilot started reviewing on behalf of tianjianjiang February 14, 2026 14:50 View session

claude Bot reviewed Feb 14, 2026

View reviewed changes

Comment thread algorithm.md Outdated

gemini-code-assist Bot reviewed Feb 14, 2026

View reviewed changes

Copilot AI reviewed Feb 14, 2026

View reviewed changes

Comment thread algorithm.md Outdated

tianjianjiang force-pushed the docs/algorithm_viterbi branch from fb72feb to 15820a9 Compare February 14, 2026 18:35

tianjianjiang changed the base branch from master to docs/user_model_requirements February 14, 2026 18:35

tianjianjiang force-pushed the docs/user_model_requirements branch from 6106a7a to e246baa Compare February 14, 2026 19:08

tianjianjiang force-pushed the docs/algorithm_viterbi branch from 15820a9 to 02b06d1 Compare February 14, 2026 19:08

claude Bot reviewed Feb 14, 2026

View reviewed changes

Comment thread algorithm.md Outdated

Comment thread algorithm.md

claude Bot reviewed Feb 14, 2026

View reviewed changes

Comment thread algorithm.md Outdated

Comment thread algorithm.md

Comment thread algorithm.md

Comment thread algorithm.md Outdated

tianjianjiang force-pushed the docs/user_model_requirements branch from e246baa to c9154c8 Compare February 14, 2026 21:02

tianjianjiang force-pushed the docs/algorithm_viterbi branch from 02b06d1 to 104108a Compare February 14, 2026 21:05

claude Bot reviewed Feb 14, 2026

View reviewed changes

Comment thread algorithm.md Outdated

Comment thread algorithm.md Outdated

tianjianjiang force-pushed the docs/user_model_requirements branch from c9154c8 to 8df0535 Compare February 14, 2026 21:58

claude Bot mentioned this pull request Feb 14, 2026

docs(requirements): add contextual user model requirements with scoring model rationale #786

Draft

8 tasks

tianjianjiang force-pushed the docs/algorithm_viterbi branch 2 times, most recently from 325ef66 to 4f90773 Compare February 14, 2026 22:33

tianjianjiang force-pushed the docs/algorithm_viterbi branch from 4f90773 to 8b213a9 Compare February 14, 2026 22:54

tianjianjiang mentioned this pull request Feb 15, 2026

docs(agents): add development guardrails, C++ language server docs, and Claude Code automation #787

Draft

3 tasks

tianjianjiang force-pushed the docs/user_model_requirements branch from 8df0535 to 0ab7122 Compare February 15, 2026 08:42

tianjianjiang force-pushed the docs/algorithm_viterbi branch from 8b213a9 to eedacac Compare February 15, 2026 08:42

tianjianjiang force-pushed the docs/user_model_requirements branch from 0ab7122 to edb593e Compare February 15, 2026 09:03

tianjianjiang force-pushed the docs/algorithm_viterbi branch from eedacac to 16b1f7d Compare February 15, 2026 09:03

tianjianjiang self-assigned this Feb 24, 2026

tianjianjiang marked this pull request as draft February 24, 2026 04:55

tianjianjiang force-pushed the docs/user_model_requirements branch from edb593e to d39c375 Compare February 26, 2026 17:18

tianjianjiang force-pushed the docs/algorithm_viterbi branch from 16b1f7d to bb597f0 Compare February 26, 2026 17:18

tianjianjiang force-pushed the docs/user_model_requirements branch from d39c375 to 05b59d5 Compare February 26, 2026 18:02

tianjianjiang force-pushed the docs/algorithm_viterbi branch from bb597f0 to 004f316 Compare February 26, 2026 18:02

tianjianjiang changed the base branch from docs/user_model_requirements to master February 26, 2026 18:02

claude Bot reviewed Feb 26, 2026

View reviewed changes

Comment thread algorithm.md Outdated

Comment thread algorithm.md

Comment thread algorithm.md

Comment thread algorithm.md Outdated

tianjianjiang force-pushed the docs/algorithm_viterbi branch from 004f316 to 87bac41 Compare February 26, 2026 18:48

claude Bot reviewed Feb 26, 2026

View reviewed changes

Comment thread algorithm.md

Comment thread algorithm.md

tianjianjiang force-pushed the docs/algorithm_viterbi branch from 87bac41 to 8175575 Compare June 10, 2026 19:40

claude Bot reviewed Jun 10, 2026

View reviewed changes

-| DP 表格形狀 | N x T（狀態數 x 時間步） | 一維：n+1 個位置 |
+    // Precondition: every position 1..readingLen must be reachable;
+    // insertReading() guarantees this by rejecting readings with no unigrams.
+    curr = readingLen
+    while curr > 0:
+        assert viterbi[curr].fromNode is not nil
+        append viterbi[curr].fromNode to nodes
+        curr = viterbi[curr].fromIndex

Uh oh!

Conversation

tianjianjiang commented Feb 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Critical Issue Found

Uh oh!

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

github-actions Bot commented Feb 14, 2026

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Critical Issues

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Feb 14, 2026

Uh oh!

github-actions Bot commented Feb 15, 2026

Uh oh!

github-actions Bot commented Feb 26, 2026

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Feb 26, 2026

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Uh oh!

claude Bot Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

claude Bot Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

claude Bot Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

tianjianjiang commented Feb 14, 2026 •

edited

Loading