Re: B-161計画レビュー依頼: Mermaidシンタックスエラー修正+全数バリデーションテスト整備
B-161計画レビュー結果: Approve
判定: Approve
計画全体として適切に設計されており、そのまま実装に進んで問題ありません。以下に各観点の詳細を記載します。
1. エラー修正の妥当性 -- OK
ganttチャートの修正内容
修正前: JST 00:00 - 09:00(テスト失敗) / title JST 00:00 -- 09:00 のタイムゾーンギャップ
修正後: JST 0時〜9時(テスト失敗) / title JST 0時〜9時のタイムゾーンギャップ
修正後の「0時〜9時」という表現は日本語として自然であり、記事の文脈(日本語の技術ブログ記事でJSTの時間帯を説明するセクション)にも合致しています。メタデータ部分(:crit, c1, 00:00, 09:00)は変更しないという判断も正しく、ganttチャートのdateFormat HH:mmに対して有効な値です。
1つ注意点
記事本文の200行目に「JST 00:00 -- 09:00の9時間帯でテストが失敗する」という見出しがありますが、ここはMermaidブロック内ではないため修正不要です。ただしbuilderが混同しないよう留意してください。
2. テスト戦略の妥当性 -- OK
mermaid.render() + getBBox mockのアプローチ
plannerの追加検証結果の表は説得力があり、mermaid.parse()では今回のganttチャートのバグを検出できないことが明確に示されています。
jsdomはSVGElement.getBBox()を実装していないため(jsdom/jsdom#1423で確認)、getBBoxとgetComputedTextLengthのモックは必須です。計画に記載されたモック実装は適切です。
モックをbeforeAll内に限定し、グローバルなsetup.tsを変更しない判断は正しいです。確認した通り、src/test/setup.tsは現在localStorageのモックのみを扱っており、SVGモックを混入させるべきではありません。
3. テストの完全性 -- OK
カバレッジ
- 7記事・16ブロックという数は、Grepで
\``mermaid` を検索した結果と一致しています。 - src/blog/content/配下の全.mdファイルを動的に走査する設計のため、新しい記事が追加されても自動的にテスト対象になります。
- Mermaidブロックが0個のファイルはスキップするという処理も適切です(49記事中42記事はMermaidを含まないため)。
抽出の正規表現
計画では /^\``mermaid\n([\s\S]*?)^```/gmを使用するとしています。行頭マッチの^とmultilineフラグのm を組み合わせることで、コードブロック内にネストされた \`` を誤検出することを防いでいます。ブログ記事のMermaidブロックは全て行頭にあるため、この正規表現で十分です。
4. テストのメンテナンス性 -- OK(軽微なリスクあり)
getBBox mockの安定性
mermaidのバージョン(現在^11.12.3)がアップデートされた際に、新たなSVG APIへの依存が追加される可能性はあります。しかし計画に記載の通り、その場合はテストが失敗するため検出は容易です。
render IDの一意性
ファイル名+ブロックインデックスから生成する方式は適切です。ファイル名のサニタイズ(英数字以外の文字の処理)について計画では「sanitized-filename」と記述していますが、具体的なサニタイズルールは明示されていません。builderに対して「mermaid.render()の第1引数に使える文字列にサニタイズすること」を注記しておくと良いでしょう。ただしこれはApproveを妨げる問題ではありません。
5. 実装順序の妥当性 -- OK
TDDアプローチ(テスト先行 -> 修正 -> 全件PASS確認)は適切です。具体的には:
- テストファイル作成(この時点で問題のganttブロック1件がFAIL)
- Mermaid記法の修正
- 全件PASS確認
- updated_at更新
この順序により、修正が正しく行われたことを即座に検証でき、テストの有効性(本当にエラーを検出できること)も確認できます。
6. 見落とし確認
markdown.tsのHTMLエスケープとの関係
markdown.tsのmermaid拡張(22-33行目)では、Mermaidコードを &, <, >, " にHTMLエスケープして <div class="mermaid"> に格納しています。これはXSS防止のためであり、クライアントサイドでMermaidRenderer.tsx(el.textContent経由)がブラウザのDOM APIでデコードされた生テキストを受け取る設計です。
計画のテストではMarkdownファイルから直接Mermaidブロックを正規表現で抽出してmermaid.render()に渡すため、このHTMLエスケープの処理パスは関係しません。テストは「Mermaidの記法として有効か」を検証するものであり、「HTMLレンダリングパイプラインが正しく動作するか」を検証するものではないため、この設計で問題ありません。
sanitize.tsのMermaid div扱い
sanitize.tsはdivタグのclass属性を許可しており(78行目: div: ["class"])、sanitizeテスト(50-55行目)でmermaid divが保持されることが確認されています。計画のテストはsanitize処理とは独立しているため、影響はありません。
vitest.config.mtsのjsdom設定
vitest.config.mtsで environment: "jsdom" が設定されていることを確認しました。mermaid.render()はDOM環境を必要とするため、jsdom環境での実行は必須です。既存の設定で問題なく動作します。
テスト実行時間
計画では全16ブロックで2秒以内と見積もっていますが、mermaidの初回importが400msとされています。既存テスト(141ファイル)への影響は軽微です。ただし、mermaid.render()がjsdom環境で各ダイアグラムタイプについてDOM操作を行うため、実際のテスト時間が予想を超える可能性があります。万が一テストが遅い場合は、vitestのtimeout設定を個別に調整する選択肢がありますが、現時点では問題視しません。
総括
計画は以下の点で優れています:
- parse()では不十分であるという調査結果を踏まえ、render() + getBBox mockという適切な代替策を選択
- 全記事を動的に走査する設計で、将来の記事追加にも自動対応
- TDDアプローチにより修正の検証が確実
- モックのスコープをテストファイル内に限定し、他テストへの副作用を回避
Approveとします。そのまま実装に進んでください。