fix(docs): markdown 3+ tables drift placeholder offset by one per table#613
Open
chrischall wants to merge 2 commits into
Open
fix(docs): markdown 3+ tables drift placeholder offset by one per table#613chrischall wants to merge 2 commits into
chrischall wants to merge 2 commits into
Conversation
Closes openclaw#607. The per-table offset accumulator in replaceDocsMarkdownRange and insertDocsMarkdownAt added `(tableEnd - tableIndex) - 1` after each table, on the theory that the placeholder "\n" we wrote into plainText was consumed by the InsertTable call. Empirically the placeholder remains in the doc, so each subsequent table inserted one character too early — invisible for table 2 (the placeholder it shifted onto became its own leading newline), but for table 3 the off-by-2 drift landed mid-paragraph, splitting the preceding paragraph's trailing punctuation into a standalone paragraph after the table. Extracted the formula into `nextTableInsertOffset(currentOffset, tableIndex, tableEnd)` which accumulates the full (tableEnd - tableIndex) and updated both call sites. The helper is directly unit- tested against the issue repro shape. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #607.
Root cause
The per-table offset accumulator in both
replaceDocsMarkdownRangeandinsertDocsMarkdownAt:assumed the placeholder
"\n"we wrote intoplainTextfor each table position was consumed by theInsertTablecall. Empirically that placeholder remains in the doc —InsertTableinserts the new table BEFORE the existing character at the requested index, so every subsequent placeholder shifts forward by the full(tableEnd - tableIndex), not(tableEnd - tableIndex) - 1.That
-1accumulated one missing character of drift per table:Why the issue only became visually obvious at the 3rd table: with 1 char of drift, table 2 lands on the blank line between paragraph and table, which is itself a newline — the doc still parses, you just lose the visual gap. With 2 chars of drift, table 3 lands inside the preceding paragraph's terminator/punctuation, splitting
three.\nso the period becomes a standalone paragraph after the table.Fix
Extracted the formula into a small helper:
and replaced both inline call sites with
tableOffset = nextTableInsertOffset(tableOffset, tableIndex, tableEnd). The helper lives next to the rest of the table-insertion machinery indocs_table_inserter.go.Tests
TestNextTableInsertOffset_AccumulatesFullTableSize— exercises a 3-table sequence with hand-computed expected offsets and explicitly calls out the previous buggy values in the failure messages.TestNextTableInsertOffset_ZeroSizedTableLeavesOffsetUnchanged— guards thetableEnd <= tableIndexshort-circuit (previously hidden inside anif).TestNextTableInsertOffset_MatchesIssueRepro— parameterised over any table size G, asserts N tables of size G accumulate to NG (previous formula gave N(G-1)).go test ./internal/cmd/ -count=1→ ok 23.8s.