Skip to content

fix(parser): preserve blank lines inside multi-line YAML block scalars (#387)#388

Merged
spinje merged 2 commits into
mainfrom
fix/multiline-yaml-block-scalar-blank-lines
May 12, 2026
Merged

fix(parser): preserve blank lines inside multi-line YAML block scalars (#387)#388
spinje merged 2 commits into
mainfrom
fix/multiline-yaml-block-scalar-blank-lines

Conversation

@spinje
Copy link
Copy Markdown
Owner

@spinje spinje commented May 11, 2026

Summary

Multi-line YAML block scalars in .pflow.md files (- prompt: |, - command: |, etc.) silently truncated at the first blank line, with post-blank content silently absorbed into the entity's purpose. Workflows validated and executed but only a fraction of the intended value reached the LLM/shell/HTTP body. The official pflow guide prompt-caching example hit this exact pattern.

Fixes #387

Changes

  • src/pflow/core/markdown_parser.py: collapse the YAML-continuation rule from two conditions to one (blank OR indented ≥ bullet column); strip trailing blanks in _flush_yaml_item so single-line items stay on the _coerce_yaml_scalar fast path.
  • src/pflow/core/CLAUDE.md: document the continuation rule so future agents don't reintroduce the bug.
  • src/pflow/guide/features/prompt-caching.md: rewrite the two - prompt: | examples (score-each-chorus, process-items) to use the preferred ```prompt fenced-block syntax.
  • tests/test_core/test_markdown_parser.py: 9 new regression tests.

Explanation

The parser's YAML-continuation state machine never inspected the |/> sigil — block-scalar handling was an emergent side effect of which lines were collected before yaml.safe_load ran. A blank line unconditionally flushed the in-progress item, dropping every subsequent line from PyYAML's view. The dropped lines then matched the parser's "prose" path (no leading - , line.strip() != "") and were silently captured as the entity's description.

The fix replaces the wrong rule rather than adding a special case: under standard YAML semantics, a | block ends only when a line is less indented than the block's content — blank lines are preserved as empty literal lines. The new continuation logic implements exactly that within markdown's nested context. PyYAML then handles |, >, plain, octal, hex, dates, and every other block-scalar variant correctly because it now sees the full block.

Trailing-blank stripping at flush time keeps the _coerce_yaml_scalar fast path active for single-line items between blank-padded multi-line ones — important because _coerce_yaml_scalar intentionally diverges from PyYAML for edge cases (e.g., 010 stays 10, not octal 8), as documented in core/CLAUDE.md.

Coverage map (parser-fix scope is the first row; rows 2-3 already worked):

Multi-line content path Before After
- key: | / - key: > / - key:\n ... in any section, any node type Broken (silent truncation) Fixed
Fenced code-block params ( ```prompt, ```yaml batch, ```shell command, etc.) Worked Unchanged
File references (- prompt: ./foo.prompt.md) Worked Unchanged

File stats:

src/pflow/core/CLAUDE.md                   |   2 +
src/pflow/core/markdown_parser.py          |  29 ++--
src/pflow/guide/features/prompt-caching.md |  28 ++--
tests/test_core/test_markdown_parser.py    | 218 +++++++++++++++++++++++++++++
4 files changed, 256 insertions(+), 21 deletions(-)

Testing

  • test_yaml_block_scalar_literal_preserves_blank_lines — exact score-each-chorus guide pattern; asserts prompt body + clean purpose.
  • test_yaml_folded_scalar_preserves_blank_lines> folded scalar with paragraph break.
  • test_plain_multiline_scalar_preserves_blank_lines- prompt: indented plain scalar.
  • test_block_scalar_with_blank_lines_in_non_prompt_paramscommand: (shell), body: (http).
  • test_block_scalar_blank_lines_in_inputs_and_outputsdescription: in ## Inputs and ## Outputs.
  • test_block_scalar_followed_by_another_bullet — trailing-blank handling vs next bullet.
  • test_single_line_item_between_block_scalars_uses_fast_path — regression guard; asserts max-retries: 010 parses as decimal 10 (fast path), not octal 8 (PyYAML path).
  • Full suite: 6573 passed, 1 skipped. make check clean.

Manual verification (verification-specialist pass):

  • End-to-end shell run with command: | + internal blanks — resolved command in trace matches source byte-for-byte; stdout shows expected paragraphs.
  • Sub-workflow with multi-line block scalar in the child — end-to-end run with ${message} template substitution works.
  • pflow analyze-cache on a multi-line-prompt workflow — sees the full prompt (17 tokens, not the truncated ~5) and correctly fires cache.prompt-body-duplicates-cache for ${rubric} duplication.
  • pflow save → re-load → re-run: file preserved verbatim, saved workflow re-runs correctly.
  • Source line tracking through multi-line block scalars: a ${check.nonexistent_field} template error in ## Outputs after a multi-line block with 2 internal blank lines correctly reports file:21 (the - source: line).
  • EOF with trailing blank lines: stripped at flush; content preserved.
  • Bullets (- item) inside | block scalar content: preserved as content, not stolen as new YAML items.
  • examples/invalid/ (10 files): all still fail with their original errors — no false-positive regressions.
  • pflow --dry-run and pflow visualize on multi-line-prompt workflows: parse and render correctly.

Pre-existing issue flagged (out of scope)

Indented markdown-heading-looking content (# foo, ## bar, ### baz) inside a | block scalar is incorrectly detected as a heading because the heading check uses stripped.startswith(...) and runs before the YAML-continuation check. Verified same behavior on main (this PR did not introduce it). Worth a follow-up issue — change heading checks to line.startswith(...) (column-0 only, matching markdown spec).

…scalars

Multi-line YAML block scalars (`- prompt: |`, `- command: |`, etc.) silently
truncated at the first blank line; the post-blank content was absorbed as
entity prose. Workflows validated and executed but the LLM/shell never saw
most of the value. The official `pflow guide prompt-caching` example hit this.

Root cause: the YAML-continuation state machine in `markdown_parser.py` treated
any blank line as an item terminator. It never inspected the `|`/`>` sigil —
block-scalar handling was an emergent side effect of which lines were collected
before yaml.safe_load ran.

Fix: collapse the continuation rule from two conditions to one — a line
continues the current item if it is blank OR indented past the bullet. Strip
trailing blanks in `_flush_yaml_item` so single-line items stay on the
`_coerce_yaml_scalar` fast path (preserves intentional PyYAML divergences for
octal/hex/dates). This matches YAML's actual block-scalar termination
semantics and applies uniformly to every multi-line value in every section.

Also updates the `prompt-caching.md` guide examples to use the preferred
fenced-block syntax (```prompt ... ```) and adds a note to `core/CLAUDE.md`
documenting the continuation rule.

9 new tests cover: `|`/`>`/plain multi-line preservation, non-prompt params
(command/body/description), Inputs/Outputs sections, trailing-blank handling,
and a fast-path regression guard.
@claude
Copy link
Copy Markdown

claude Bot commented May 11, 2026

Code Review — PR #388

Reviewed as an opened event (full diff against main). Read: markdown_parser.py diff and surrounding state machine, _coerce_yaml_scalar, _parse_yaml_items, _parse_multiline_yaml_item, the 9 new tests, and the prompt-caching guide edits.

TL;DR

Well-scoped fix. Root cause correctly identified (the continuation rule had two conditions where YAML semantics demand one). The diff replaces the wrong rule rather than papering it over with a |/> special case, which is the right move — PyYAML now sees the full block and handles every block-scalar variant correctly. Tests are tight and the fast-path regression guard (max-retries: 010) is a particularly nice touch. Not blocking on anything; a couple of small accuracy/comment nits below.


Critical — must fix before merge

(none)

Warnings — should be addressed

1. Test comment is wrong about what _coerce_yaml_scalar returns
In test_single_line_item_between_block_scalars_uses_fast_path (tests/test_core/test_markdown_parser.py:957-960):

# `_coerce_yaml_scalar` keeps "010" as the literal string "010"; PyYAML's
# yaml.safe_load would parse it as int 8 (octal). The fast path must run.
assert node[\"params\"][\"max-retries\"] == 10

The assertion is correct, but the comment is wrong: _coerce_yaml_scalar(\"010\") returns decimal int 10, not the string \"010\". _NUMERIC_INT_RE matches it and _coerce_numeric calls int(\"010\")10. The divergence vs PyYAML is real (10 vs 8) — just describe it accurately so future readers don't grep for behavior that doesn't exist:

# `_coerce_yaml_scalar` coerces \"010\" to decimal int 10 (Python int()); PyYAML's
# yaml.safe_load would parse it as int 8 (YAML 1.1 octal). The fast path must run.

Suggestions — optional improvements

2. Mention the chomp-indicator caveat in _flush_yaml_item's docstring
The strip-trailing-blanks justification says:

# is lossless for multi-line items (yaml.safe_load chomps trailing
# newlines for `|` block scalars by default).

True for the default (CLIP) chomping, but for |+ / >+ (keep) the trailing blanks are part of the scalar's value, and the strip silently drops them. Almost certainly a non-issue in practice (no real-world .pflow.md uses |+), but if anyone ever does, the loss will be confusing. Either a one-line caveat (\"|+\\/>+ lose trailing blanks — accepted edge case") or a regression test guarding that it stays acceptable would help future readers.

3. Pre-existing-issue acknowledgement is good — consider opening the follow-up issue
The PR description correctly flags that indented # foo / ## bar inside a | block is still misdetected as a heading because the heading checks at markdown_parser.py:387,396,424 run before the continuation check and use stripped.startswith(...). Same pre-existing issue applies to code fences (_is_code_fence at line 343, also runs before continuation). Since you've already done the analysis, filing the follow-up now would make sure it doesn't get lost.

4. Guide-doc rewrite is helpful but slight scope creep
The prompt-caching.md edits replace inline - prompt: | with fenced ```` prompt ``` blocks. With this fix landed, both syntaxes work — the rewrite is now a style preference, not a workaround. Worth keeping (the fenced form is what pflow guide recommends elsewhere), but the PR title/body could note this is a docs-style improvement, not part of the fix.


Things I verified and liked

  • Right fix, not a special case. The new continuation rule (blank OR indented ≥ bullet column) matches YAML's actual block-scalar termination rule. No |/> sniffing — PyYAML does that.
  • Fast-path preservation is load-bearing. The trailing-blank strip in _flush_yaml_item keeps single-line items off yaml.safe_load, preserving the documented PyYAML divergences (octal 010 → 10, scientific notation, hex, dates) called out in core/CLAUDE.md. The regression guard test is exactly the kind of assertion that catches a future refactor that "simplifies" by always routing through PyYAML.
  • Source-line tracking parallel-list invariant (yaml_items / yaml_item_lines / yaml_item_keys) is preserved — flush only appends, doesn't reorder.
  • Coverage across sections. Tests hit ## Steps (llm, shell, http), ## Inputs, and ## Outputs — not just the bug's original surface in ## Steps.
  • End-of-file path is correct. Loop exits → outer _flush_yaml_item() at line 548 → trailing blanks stripped → safe to feed to yaml.safe_load.
  • Comments document WHY, not WHAT. Both the continuation-rule comment and the flush-strip comment explain rationale; future agents won't reintroduce the bug.

Test quality

9 new tests; none look redundant. The shapes are well-chosen:

  • Literal |, folded >, and plain (no sigil) multi-line — three distinct YAML modes.
  • Non-prompt params (command:, body:) — confirms the fix isn't prompt-coupled.
  • Inputs and outputs description: — confirms it isn't ## Steps-coupled.
  • Block scalar followed by another bullet — the trailing-blank flush behavior.
  • Single-line items between block scalars — the fast-path regression guard (best test in the file).

No tests to remove. The only minor gap is the |+ / >+ chomp variants (see suggestion #2) — low priority.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request enhances the Markdown parser to correctly handle and preserve blank lines within multi-line YAML items, such as block scalars. Key changes include updating the continuation logic to include blank lines and modifying _flush_yaml_item to strip trailing whitespace. The documentation and test suite have been updated to reflect these improvements. Review feedback suggests optimizing the continuation check by using the pre-calculated stripped variable for better consistency and performance.

yaml_current_item_lines.append(line)
continue
content_start = len(line) - len(line.lstrip())
if line.strip() == "" or content_start >= yaml_indent_level:
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The variable stripped is already calculated at line 384. Using it here instead of calling line.strip() again is more efficient and consistent with the rest of the loop.

Suggested change
if line.strip() == "" or content_start >= yaml_indent_level:
if not stripped or content_start >= yaml_indent_level:

@spinje spinje merged commit 087d487 into main May 12, 2026
9 checks passed
…ip review]

Addresses two review comments on #388:

- `_flush_yaml_item`: document that the trailing-blank strip is lossless only
  for default-chomping (`|`/`>`) block scalars; `|+`/`>+` keep-chomping would
  preserve all trailing blanks and this strip would drop them. Accepted edge
  case — no shipped `.pflow.md` uses `|+`/`>+`.

- `test_single_line_item_between_block_scalars_uses_fast_path`: fix incorrect
  comment claiming `_coerce_yaml_scalar("010")` returns the string `"010"`.
  It actually coerces to decimal int 10 (Python `int()`), and the divergence
  vs PyYAML is 10 vs 8 (YAML 1.1 octal). Assertion was correct, comment was
  misleading.

Follow-up issue for the pre-existing heading/code-fence-detection-inside-
block-scalar issue filed as #389.
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.

Parser silently truncates multi-line - key: | values at the first blank line

1 participant