Render a code patch whose opening fence ends a prose line - #6104
jurgenwerk wants to merge 1 commit into
Conversation
Preview deploymentsHost Test Results 1 files ± 0 1 suites ±0 2h 39m 45s ⏱️ + 33m 10s Results for commit ad87ffa. ± Comparison against earlier commit 8f50979. Realm Server Test Results 1 files ± 0 223 suites +5 1h 15m 15s ⏱️ - 6m 7s Results for commit ad87ffa. ± Comparison against earlier commit 8f50979. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f50979ce0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let glued = lines[i].match(FENCE_GLUED_TO_PROSE_PATTERN); | ||
| if ( | ||
| glued && | ||
| !CODE_FENCE_PATTERN.test(lines[i]) && | ||
| FILE_URL_LINE_PATTERN.test(lines[i + 1]) && | ||
| SEARCH_MARKER_PATTERN.test(lines[i + 2]) |
There was a problem hiding this comment.
Avoid rewriting glued fences inside existing code blocks
When a valid outer patch—such as a Markdown or documentation patch—contains a literal example with prose glued to a fence followed by a file URL and SEARCH marker, this whole-document scan also matches inside that existing fenced block. If the example is in SEARCH content, the inserted newline prevents the patch from matching the target; in REPLACE content or a new file, it silently changes the requested file contents. Track fenced-block context and only repair top-level prose.
Useful? React with 👍 / 👎.
A model that finishes a sentence and opens the patch on the same line — "Let's write the block!```json" — has written a patch the renderer reads as prose: a fence opens a code block only at the start of a line, so the url, the markers and the file content collapse into one paragraph. The host finds no code block and applies nothing, and the bot, which counts patches by their markers, waits for a result that never comes. Before rendering, a line that ends in a fence and is followed by a file url line and the SEARCH marker is split so the fence starts its own line. Prose that merely ends in backticks is left alone. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
8f50979 to
ad87ffa
Compare
habdelra
left a comment
There was a problem hiding this comment.
[Claude Code 🤖] Reviewed the new split pass against what runs either side of it — the HTML escaping before it, the fence widening after it, and the patch-extraction path in the host — and traced the regex against multi-backtick, CRLF, list-item and patch-inside-a-patch inputs.
No blocking issues. The pass sits in the right place in the chain and the ordering relative to fence widening is necessary, but it has one reachable path where it edits patch content instead of prose.
- The scan has no fenced-block context, so a patch that writes a document demonstrating the glued-fence anti-pattern gets the split applied to its own SEARCH/REPLACE content — silent wrong write, or a patch that no longer matches its target. Confirmed by running the case; see the comment on
splitCodePatchFencesGluedToProse. FENCE_GLUED_TO_PROSE_PATTERNmis-splits a four-backtick glued fence and drops CRLF; suggestion inline on that line.
The one red check, Software Factory Tests shard 1/3, is factory-test-realm failing on testRunCard.ok after a run of ECONNRESET / UND_ERR_SOCKET against the realm proxy — a Playwright e2e that never renders a Matrix message body, so nothing in this diff reaches it. Ignorable for this PR.
Generated by Claude Code
| for (let i = 0; i + 2 < lines.length; i++) { | ||
| let glued = lines[i].match(FENCE_GLUED_TO_PROSE_PATTERN); | ||
| if ( | ||
| glued && | ||
| !CODE_FENCE_PATTERN.test(lines[i]) && | ||
| FILE_URL_LINE_PATTERN.test(lines[i + 1]) && | ||
| SEARCH_MARKER_PATTERN.test(lines[i + 2]) | ||
| ) { | ||
| let [, prose, fence, cr] = glued; | ||
| lines.splice(i, 1, `${prose}${cr}`, `${fence}${cr}`); | ||
| i++; | ||
| } | ||
| } | ||
| return lines.join('\n'); |
There was a problem hiding this comment.
[Claude Code 🤖] This scan carries no fenced-block context, so it rewrites patch content too — the bot flagged it and it is reachable. A patch that writes a document about the patch format carries the anti-example inside its own halves:
```markdown
https://example.com/realm/skill.md
╔══ SEARCH ══╗
╠══╣
Wrong — the fence must start a line:
Here it is!```json
https://example.com/realm/a.json (new)
╔══ SEARCH ══╗
╠══╣
{}
╚══ REPLACE ══╝
```
╚══ REPLACE ══╝
```I ran that through splitCodePatchFencesGluedToProse: the inner Here it is!```json is split into two lines. In a REPLACE half that silently writes a file the model did not ask for; in a SEARCH half the extra newline makes the patch stop matching the target and the write fails.
normalizeDecorativeBullets in this file already carries the fence tracker this needs, for the same reason — fenced content has to survive verbatim. Tracking inFence here and splitting only at top level covers it; when a split does fire, the fence line it produces opens a block, so the state should be set from it.
Regression, introduced by this pass. Not a merge blocker on its own, but small enough that I'd fix it here rather than leave it as a follow-up.
Generated by Claude Code
| // line break before the fence is missing. Put it back when the two lines after | ||
| // the fence are a file url and the SEARCH marker, which is what makes this a | ||
| // patch rather than prose that happens to end in backticks. | ||
| const FENCE_GLUED_TO_PROSE_PATTERN = /^(.*\S)(`{3,}\w*)\s*(\r?)$/; |
There was a problem hiding this comment.
[Claude Code 🤖] Two edges in this pattern, both non-blocking:
A glued fence longer than three backticks splits in the wrong place. (.*\S) is greedy, so on Writing now!\```jsonthe prose group takes the first backtick: it yields proseWriting now!`and fence```json`. The prose line then ends in a stray backtick that opens inline code across the rest of the paragraph, and the opener is three backticks where the model wrote four.
(\r?) can never capture. The preceding \s* is greedy and consumes the \r first, so on CRLF input it is always '' and both produced lines lose their carriage return. Harmless to the render, but it reads as preserving CRLF and doesn't.
Both go away with a lazy prose group that can't end on a backtick, plus a horizontal-whitespace class so the \r survives to its group:
| const FENCE_GLUED_TO_PROSE_PATTERN = /^(.*\S)(`{3,}\w*)\s*(\r?)$/; | |
| const FENCE_GLUED_TO_PROSE_PATTERN = /^(.*?[^\s`])(`{3,}\w*)[ \t]*(\r?)$/; |
Checked against both inputs: Go!\``json\r→["Go!", "```json", "\r"], and the four-backtick line → ["Writing now!", "````json", ""]`.
Generated by Claude Code
Gemini 3.5 Flash writes its patches like this, in every run today:
A fence opens a code block only at the start of a line, so the renderer reads the whole patch as one paragraph of prose. The host finds no code block and applies nothing. The bot counts patches by their markers, sees one with no result, and waits forever. The user sees a wall of text and a room that never answers again. The block itself is correct; only the line break before the fence is missing.
The body pass in marked-sync.ts gets one more rule, run before the fence widening: a line that ends in a fence, followed by a file url line and the SEARCH marker, is split so the fence starts its own line. Prose that ends in backticks without a patch behind it is left alone, and a fence already on its own line is unchanged.
Checked against the two real messages from today: the room from the screenshot goes from zero code blocks to one with its url on the first line, and the smoke-run room recovers both of its patches.
Stacked on the failed-patch feedback PR, which holds the pass this extends. The matching skill sentence, that the fence starts on a line of its own, is a separate boxel-skills PR.
🤖 Generated with Claude Code