Skip to content

fix(docs): markdown empty-header tables leak last row after the table#611

Open
chrischall wants to merge 2 commits into
openclaw:mainfrom
chrischall:fix/609-empty-header-table-leak
Open

fix(docs): markdown empty-header tables leak last row after the table#611
chrischall wants to merge 2 commits into
openclaw:mainfrom
chrischall:fix/609-empty-header-table-leak

Conversation

@chrischall
Copy link
Copy Markdown

Closes #609.

Root cause

isTableSeparator accepted rows of empty pipe cells like | | | as separators because the per-segment loop hit continue for empty segments and never tripped the "must have at least one dash" check at the end of the loop body. Concretely:

for _, seg := range segments {
    seg = strings.TrimSpace(seg)
    if seg == "" {
        continue          // ← skips the dash check
    }
    // … dash-only check + must-have-a-dash check …
}
return len(segments) > 1

With input

|     |     |       ← row 0 (empty header) — also matched isTableSeparator
|-----|-----|       ← row 1 (real separator)
| Label A | Value A |
| Label B | Value B |

parseMarkdownTable then skipped both row 0 (false separator) and row 1, returning only 2 data rows. The outer parser advanced i += len(tableCells) = 2, putting the next iteration on row 3, where | Label B | Value B | was re-parsed as a literal pipe paragraph.

Fix

Track sawDashSegment in isTableSeparator and require at least one non-empty, dash-containing segment. || is still rejected by the existing len(segments) > 1 guard, so this change is strictly additive.

Tests

  • TestIsTableSeparator_EmptyPipeRowRejected — table-driven cases covering empty-cell forms, normal separators, and alignment variants.
  • TestParseMarkdown_EmptyHeaderTableKeepsAllDataRows — full ParseMarkdown regression that exercises the exact repro from the issue and asserts there is exactly one MDTable element with all three rows intact.

go test ./internal/cmd/ -count=1 → ok 23.5s.

chrischall and others added 2 commits May 19, 2026 19:55
Closes openclaw#609.

`isTableSeparator` accepted rows of empty pipe cells like `|     |     |`
as separators because the per-segment loop hit `continue` for empty
segments and never tripped the "must have at least one dash" check. The
parser then consumed the empty header AND the actual `|---|---|`
separator, advanced the outer loop too far, and re-parsed the last data
row as a literal pipe paragraph after the table.

Track a `sawDashSegment` flag and require at least one non-empty segment
that actually contains dashes. `||` is still rejected by the existing
`len(segments) > 1` guard.

Tests:
- TestIsTableSeparator_EmptyPipeRowRejected — table-driven cases
  including the empty-cell forms from the issue
- TestParseMarkdown_EmptyHeaderTableKeepsAllDataRows — full
  ParseMarkdown regression covering the exact repro

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

docs write --append --markdown: empty-header tables leak last row as literal pipe text after the table

1 participant