Skip to content

[Fix] forge yaml_utils self-parser embedded newline escaping#192

Merged
BlueEventHorizon merged 2 commits into
developfrom
feature/work
Jul 9, 2026
Merged

[Fix] forge yaml_utils self-parser embedded newline escaping#192
BlueEventHorizon merged 2 commits into
developfrom
feature/work

Conversation

@BlueEventHorizon

Copy link
Copy Markdown
Owner

概要

背景

Issue #189: /forge:review の評価結果統合時、finding の body に複数行 Markdown 箇条書きが含まれると、本来 5 件のはずの finding が 19 件に断片化される不具合が報告された。

原因は reader ではなく writer 側にあった。yaml_scalar() がダブルクォート文字列内の実改行をエスケープしていなかったため、複数行テキストを書き出すと「クォートされた値が複数物理行に割れる」という、自家製パーサーの前提(1 フィールド = 1 物理行)を破る不正な YAML が生成されていた。読み込み側の行ベース parser はこの前提に従うため、埋め込まれた - priority: P2 のような行を新しいフィールド開始と誤認していた。

Issue 本文が提案していた「PyYAML 導入」は採用していない。本リポジトリの plugins/ 配下スクリプトは外部依存ゼロ・標準ライブラリのみで動作する方針(配布物として python3 script.py を直接実行するため、インストールステップを前提にできない)であり、今回は自家製パーサー側の root cause 修正で解決した。

やったこと

  • yaml_utils.pyyaml_scalar(): 文字列に \n / \r が含まれる場合 \\n / \\r にエスケープし、常に単一物理行に収める
  • yaml_utils.py_unescape_double_quoted(): 読み込み時に \\n / \\r を実際の改行文字へ復元する
  • 回帰テスト追加(複数行ラウンドトリップ、item 断片化しないことの確認、ユーザーデータ中のリテラル \n と実改行エスケープの混同がないことの確認)

やらないこと(このプルリクエストのスコープ外とすること)

  • PyYAML 等の外部依存導入(別途の方針判断が必要)
  • 既存の破損済み plan.yaml / findings_state.yaml のマイグレーション(.claude/.temp/ は完了セッションで消える ephemeral ディレクトリのため対象外)

レビュー観点

  • yaml_scalar() のエスケープ順序(バックスラッシュ → 改行)が、ユーザーデータ中のリテラル \n(バックスラッシュ+n の2文字)と実改行のエスケープを混同しないことをテストで確認済み
  • 全 1377 テスト green(既存 1373 + 新規 4)

レビューレベル

  • Lv0: まったく見ないでAcceptする
  • Lv1: ぱっとみて違和感がないかチェックしてAcceptする
  • Lv2: 仕様レベルまで理解して、仕様通りに動くかある程度検証してAcceptする
  • Lv3: 実際に環境で動作確認したうえでAcceptする

備考

Issue #189 にも同内容の技術的検討をコメント済み。

BlueEventHorizon and others added 2 commits July 9, 2026 21:16
yaml_scalar() quoted multi-line values without escaping embedded \n/\r,
so the writer emitted raw newlines inside a "quoted" scalar spanning
several physical lines. The line-based self-parser then misread each
embedded line (e.g. markdown bullets like "- priority: P2") as a new
field, fragmenting a single item into many. Escape \n/\r to \\n/\\r on
write and restore them on read, keeping every field on one physical
line as the self-parser assumes. Fixes #189.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Collapsing "\r\n" to "\\n" before the independent \n/\r replacements
discarded the \r, so CRLF values silently became LF on read. Escape
\n and \r independently instead so CRLF, lone CR, and LF all restore
exactly. Addresses PR #192 review feedback.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@BlueEventHorizon BlueEventHorizon self-assigned this Jul 9, 2026
@BlueEventHorizon
BlueEventHorizon marked this pull request as ready for review July 9, 2026 12:40
@BlueEventHorizon
BlueEventHorizon merged commit 1b99eb6 into develop Jul 9, 2026
1 check passed
@BlueEventHorizon
BlueEventHorizon deleted the feature/work branch July 9, 2026 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant