Repository navigation
PRD-9707 Keep merged Markdown byte-faithful and check the output (#7060) - #40
Merged
Merged
Conversation
* PRD-9707 Keep merged Markdown byte-faithful and check the output The block splitter no longer trims. Each block keeps its exact bytes, first-line indentation, trailing spaces and interior line endings included, along with the separator bytes in front of it, and the document keeps the bytes after its last block. It splits on LF and CRLF and still ignores blank lines inside a fence. Block identity reads CRLF as LF, so a difference only in line endings is never an edit and any other byte difference is. The merge writes each emitted block behind a separator: separators, leading bytes and trailing bytes take theirs' version when theirs changed it from base, otherwise ours'. So a kept block keeps base's separator unless a side changed it, and a side's modified or inserted block brings that side's. A block that was first in its source but lands later gets the document's usual blank line. Line endings follow D37. When theirs uses one convention throughout and it differs from base's, the whole output takes it. The spec states only that outside-side rule. When theirs did not convert, the same rule is applied to ours, which is its symmetric reading. Otherwise each block and separator keeps the bytes of the side it came from. Every merged or conflicts result now passes checkMergeOutput before reconcile returns it. Untouched base blocks must appear verbatim and in base order, every block a side changed must appear with that side's bytes (theirs' side of a recorded conflict excepted), each emitted block must trace to exactly one source occurrence, and each emitted block must parse to the same root construct as in a source document. The core parse is synchronous, so the construct check runs inline. Identical blocks both sides added in an overlapping region count as one occurrence unless the engine skipped insert dedup, which is what turns away the insert-read-as-edit duplication behind D43. A failure returns a new refusal reason, check-failed, with the failed check and block index. Callers map it like too-large, and both refusal sites log the failed check. Existing test expectations changed: - reconciliation.test.ts, "an extra blank line on the disk side does not manufacture a conflict": newContent is now "A\n\n\n\nC\n" instead of "A\n\nC\n", since theirs' two blank lines are kept. The title gained "and keeps its bytes". * Skip the construct check on unparseable or oversized documents The construct check used the strict MDX parse, so a document holding something like <Foo>broken</Bar> in a block nobody touched could no longer be merged. When base, ours or theirs fails to parse, the construct check is now skipped and the byte checks alone decide. When every source parses but the merged output does not, the check fails with a new typed failure, parse, instead of a construct mismatch at block 0. The mocked-throw test is replaced by real inputs: an unparseable untouched block merges through reconcile, and a hand-built output that fails to parse is refused as parse. The construct check is also bounded. When the largest of base, ours, theirs and the output is over MAX_CONSTRUCT_CHECK_BYTES (256 KiB), it is skipped with one debug log, since the synchronous parses cost about 0.9 s at 265 KB on the watcher and agent-write paths. Tests pin both sides of the bound. The skill reference for conflict resolution and the MCP reference now say that refused-too-large is also returned, rarely, when the merged output failed the fidelity check, and that the server log names the failed check. No wire reason is added, since the next change retires this refusal path. Check failures are reported through one helper, reportReconcileCheckFailed, which logs the failure detail and increments a new reconcileCheckFailed counter that /api/metrics/reconciliation returns. The watcher arm, the agent-write guard and the branch-switch restore all call it, so the branch-switch path now records the detail too. checkMergeOutput now requires conflicts and dedupSkipped, block identity uses core's spellLineEndings, and a new test pins the line-ending tie-break: when both sides converted a mixed file to different conventions, theirs' convention wins. Swapping the two branches in mergeOutputLineEnding turns that test red. GitOrigin-RevId: 8f2f9b1b873ec97a185421e551dced1baac95525
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.
Copybara-translated 1 Inkeep OSS change. Rebase-merge this PR so the prepared commit lands directly on public main.
Linear: this mirror replays a change that already merged upstream, so it must not drive ticket status.
skip PRD-9707