fix: preserve markdown formatting when responses split across Discord messages - #12
fix: preserve markdown formatting when responses split across Discord messages#12wolframs wants to merge 3 commits into
Conversation
… messages Discord parses each message independently, so when a long LLM response is divided across multiple messages, a markdown construct straddling the boundary breaks — an open code fence corrupts every block after it (the matching closer becomes an opener, cascading through the rest of the response). Add a pure markdown balancer (src/utils/discord-markdown.ts) that closes any construct open at a chunk boundary and reopens it in the next message, with carry state threaded across separate sends. Covers code fences, inline code, and emphasis spans (*, **, ***, _, __, ~~, ||). - connector.sendSegmentChunks resolves mentions/emojis then markdown-splits once (post-resolution), returning per-message bridge metadata so resolution can never trigger an unrecorded second split - sendSegments threads markdownCarry across the native + inline tool paths; only exclusive constructs (fence/inline) cross a send boundary - synthetic close/reopen strings recorded on MessageContext and stripped during context reconstruction, so the model sees its original unbroken markdown - persist messageContexts in the generated-image response path Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@antra-tess Greptile Review has started over twice for this review already, no idea what's going on here, just thought I'd let you know. |
Greptile SummaryThis PR adds a pure, dependency-free markdown balancer (
Confidence Score: 4/5The core markdown-balancing logic is correct and well-tested; carry threading is properly wired across all send paths in both the native and inline tool execution branches. The src/utils/discord-markdown.ts — the guard-loop escape path where a chunk may exceed maxLength; src/context/stages/injections.ts — the broad fallback in
|
| Filename | Overview |
|---|---|
| src/utils/discord-markdown.ts | New 640-line parser + splitter: parses fences, inline code, and emphasis into constructs, then splits text so open constructs are bridged at chunk boundaries. Logic is thorough; guard-loop escape path and non-exclusive inherited carry are minor edge cases that do not cause bugs in the current calling convention. |
| src/discord/connector.ts | Adds sendSegmentChunks (resolve → split → send per-chunk with individual retries) and extracts sendChunk helper; splitMessage now delegates to splitPreservingMarkdown. Retry separation correctly prevents re-sending earlier chunks. |
| src/agent/loop.ts | markdownCarry threaded through native pre-tool flush, streaming pre-tool flush, inline pre-tool send, and finalizeInlineExecution; all call sites correctly update carry after each send. Also fixes a pre-existing dropped messageContexts in the image response path. |
| src/context/stages/injections.ts | Adds stripBridgeOpen/stripBridgeClose to remove synthetic delimiters during context reconstruction; fallback in stripBridgeOpen matches by fence marker type to handle Discord normalization of info-string content. |
| src/activation/types.ts | Adds optional bridgeOpen/bridgeClose fields to MessageContext with clear documentation. |
| src/utils/discord-markdown.test.ts | New 1200-line test suite covering fences, inline code, emphasis, nesting, startCarry continuation, and edge cases; includes reconstruct invariant checker. |
| src/discord/connector.send-segments.test.ts | Tests for sendSegmentChunks: single-message, inherited carry, split-after-resolution, reply targeting, and per-chunk retry without re-sending earlier chunks. |
| src/context/builder.test.ts | Three new tests verify stripBridgeOpen/Close in injectActivationCompletions: exact match, emoji normalization, and mention normalization in info strings. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant AL as AgentLoop
participant SS as sendSegments
participant SC as sendSegmentChunks
participant SPM as splitPreservingMarkdown
participant DC as Discord API
Note over AL: markdownCarry = []
AL->>SS: "segments, markdownCarry=[]"
SS->>SC: "segment.visible, carry=[]"
SC->>SC: resolveMentions + resolveEmojis
SC->>SPM: "resolvedContent, 1800, startCarry=[]"
SPM-->>SC: "chunks[{text,bridgeClose},...], endCarry=[fence]"
SC->>DC: send chunk[0] (reply)
DC-->>SC: "id=msg-1"
SC->>DC: send chunk[1] (with bridgeOpen)
DC-->>SC: "id=msg-2"
SC-->>SS: "[{id,bridgeClose},{id,bridgeOpen}], endCarry=[fence]"
SS->>SS: "carry = exclusiveOnly(endCarry) = [fence]"
SS-->>AL: "endCarry=[fence], messageContexts"
Note over AL: markdownCarry=[fence]
AL->>SS: "next segments, markdownCarry=[fence]"
SS->>SC: "segment.visible, carry=[fence]"
SC->>SPM: "resolvedContent, 1800, startCarry=[fence]"
SPM-->>SC: "chunks (bridgeOpen prepended), endCarry=[]"
SC->>DC: send chunk (reopened fence)
DC-->>SC: "id=msg-3"
SC-->>SS: "[{id,bridgeOpen}], endCarry=[]"
SS-->>AL: "endCarry=[]"
Note over AL,DC: Context reconstruction strips bridgeOpen/bridgeClose
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant AL as AgentLoop
participant SS as sendSegments
participant SC as sendSegmentChunks
participant SPM as splitPreservingMarkdown
participant DC as Discord API
Note over AL: markdownCarry = []
AL->>SS: "segments, markdownCarry=[]"
SS->>SC: "segment.visible, carry=[]"
SC->>SC: resolveMentions + resolveEmojis
SC->>SPM: "resolvedContent, 1800, startCarry=[]"
SPM-->>SC: "chunks[{text,bridgeClose},...], endCarry=[fence]"
SC->>DC: send chunk[0] (reply)
DC-->>SC: "id=msg-1"
SC->>DC: send chunk[1] (with bridgeOpen)
DC-->>SC: "id=msg-2"
SC-->>SS: "[{id,bridgeClose},{id,bridgeOpen}], endCarry=[fence]"
SS->>SS: "carry = exclusiveOnly(endCarry) = [fence]"
SS-->>AL: "endCarry=[fence], messageContexts"
Note over AL: markdownCarry=[fence]
AL->>SS: "next segments, markdownCarry=[fence]"
SS->>SC: "segment.visible, carry=[fence]"
SC->>SPM: "resolvedContent, 1800, startCarry=[fence]"
SPM-->>SC: "chunks (bridgeOpen prepended), endCarry=[]"
SC->>DC: send chunk (reopened fence)
DC-->>SC: "id=msg-3"
SC-->>SS: "[{id,bridgeOpen}], endCarry=[]"
SS-->>AL: "endCarry=[]"
Note over AL,DC: Context reconstruction strips bridgeOpen/bridgeClose
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
src/context/stages/injections.ts:21-31
The fallback in `stripBridgeOpen` matches any fence-opening first line whose marker type (` ``` ` vs `~~~`) is the same as the one recorded in `bridgeOpen`, regardless of the info string. This is intentional for handling mention/emoji normalisation in the info string, but it also silently succeeds when the fetched message text happens to start with a *different* fence (e.g. `bridgeOpen` was `` ```python
`` but Discord returned `` ```java
... ``). In practice the bot always prepends `bridgeOpen` verbatim before sending, so the fetched text should start with a normalised form of it; still, adding a log at the fallback site makes unexpected mismatch observable without changing any behaviour.
```suggestion
const marker = /^(`{3,}|~{3,}).*\n$/.exec(bridgeOpen)?.[1]
if (!marker) return text
const lineEnd = text.indexOf('\n')
if (lineEnd < 0) return text
const firstLine = text.slice(0, lineEnd)
const actualMarker = /^(`{3,}|~{3,})/.exec(firstLine)?.[1]
if (actualMarker === marker) {
// Fallback: info-string was normalised by Discord (mention/emoji rewrite).
// We matched by marker type only; log so unexpected mismatches are visible.
if (!text.startsWith(bridgeOpen)) {
logger.debug({ bridgeOpen, firstLine }, 'stripBridgeOpen: info-string normalised, stripping by marker type')
}
return text.slice(lineEnd + 1)
}
return text
```
### Issue 2 of 3
src/utils/discord-markdown.ts:569-611
**Guard loop cap of 64 may still emit an over-length chunk**
The `guard` loop reduces `end` by one character per iteration until either `reopenLen + bodyLen + closeLen <= maxLength` or `cut <= chunkStart + 1`. On the escape path (`cut <= chunkStart + 1`) the chunk is pushed even if it exceeds `maxLength` — the comment calls this "the pathological case where the closers alone exceed maxLength". Callers rely on chunks being ≤ `maxLength` for Discord's 2000-char hard limit. If `startCarry` contains a fence with a very long info string (the opener is echoed verbatim as `bridgeOpen`), the reopener alone could push the first chunk over the limit. Worth a `logger.warn` when the escape fires so it surfaces in production logs.
### Issue 3 of 3
src/discord/connector.ts:1133-1141
`sendSegmentChunks` correctly separates resolution/split (outer retry) from individual sends (inner retries), preventing re-sends of earlier chunks. However, the inner `retryDiscord` calls hold a reference to `channel` captured from the outer retry block. If the channel reference goes stale between the two stages (rare but possible under prolonged rate-limiting), all inner retries fail against the same stale object. Fetching the channel lazily inside `sendChunk` would give each retry a fresh reference, at the cost of an extra round-trip per chunk.
```suggestion
const sent: SentMessageChunk[] = []
for (let i = 0; i < chunks.length; i++) {
const piece = chunks[i]!
// `channel` is captured from the outer retry block. On a long rate-limit
// delay the reference could go stale; re-fetching inside the retry would
// give a fresh reference but adds a round-trip per chunk.
const id = await retryDiscord(
() => this.sendChunk(channel, piece.text, i === 0 ? replyToMessageId : undefined),
this.options.maxBackoffMs,
)
sent.push({ id, bridgeOpen: piece.bridgeOpen, bridgeClose: piece.bridgeClose })
}
```
Reviews (1): Last reviewed commit: "fix: preserve markdown formatting when r..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| while (chunkStart < text.length) { | ||
| const reopenLen = reopenMarks(inherited).length | ||
|
|
||
| // Pick a cut whose real size (reopener + body + the closers for constructs | ||
| // open AT THE CUT) fits maxLength. Shrink from the largest feasible end, | ||
| // re-deriving the cut and its close-stack each pass so closers added by a | ||
| // snapped cut are counted. `cut <= chunkStart + 1` escapes the pathological | ||
| // case where the closers alone exceed maxLength. | ||
| let end = Math.min(text.length, chunkStart + Math.max(1, maxLength - reopenLen)) | ||
| let cut = -1 | ||
| let closeStack: OpenMark[] = [] | ||
| for (let guard = 0; guard < 64; guard++) { | ||
| if (end >= text.length) { | ||
| cut = text.length | ||
| // Final chunk: close only exclusive constructs (emphasis stays literal). | ||
| closeStack = exclusiveOnly(stackAt(text.length)) | ||
| } else { | ||
| // Prefer a newline within (chunkStart, end]; keep it at this chunk's end | ||
| // so reconstruction is exact. Otherwise hard-split without bisecting a | ||
| // delimiter run. | ||
| const body = text.slice(chunkStart, end) | ||
| const lastNl = body.lastIndexOf('\n') | ||
| if (lastNl >= 0) { | ||
| cut = chunkStart + lastNl + 1 | ||
| } else { | ||
| cut = avoidDelimiterBisect(end, bridgeable, chunkStart) | ||
| if (cut <= chunkStart) cut = end | ||
| } | ||
| closeStack = stackAt(cut) | ||
| } | ||
|
|
||
| const bodyLen = cut - chunkStart | ||
| const closeLen = closeMarksForBody(text.slice(chunkStart, cut), closeStack).length | ||
| if (reopenLen + bodyLen + closeLen <= maxLength || cut <= chunkStart + 1) break | ||
| end = cut - 1 | ||
| if (end <= chunkStart) end = chunkStart + 1 | ||
| } | ||
|
|
||
| if (cut < 0) cut = chunkStart + 1 | ||
| const isLast = cut >= text.length | ||
| pushChunk(cut, closeStack) | ||
| if (isLast) break | ||
| } |
There was a problem hiding this comment.
The guard loop reduces end by one character per iteration until either reopenLen + bodyLen + closeLen <= maxLength or cut <= chunkStart + 1. On the escape path (cut <= chunkStart + 1) the chunk is pushed even if it exceeds maxLength — the comment calls this "the pathological case where the closers alone exceed maxLength". Callers rely on chunks being ≤ maxLength for Discord's 2000-char hard limit. If startCarry contains a fence with a very long info string (the opener is echoed verbatim as bridgeOpen), the reopener alone could push the first chunk over the limit. Worth a logger.warn when the escape fires so it surfaces in production logs.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/utils/discord-markdown.ts
Line: 569-611
Comment:
**Guard loop cap of 64 may still emit an over-length chunk**
The `guard` loop reduces `end` by one character per iteration until either `reopenLen + bodyLen + closeLen <= maxLength` or `cut <= chunkStart + 1`. On the escape path (`cut <= chunkStart + 1`) the chunk is pushed even if it exceeds `maxLength` — the comment calls this "the pathological case where the closers alone exceed maxLength". Callers rely on chunks being ≤ `maxLength` for Discord's 2000-char hard limit. If `startCarry` contains a fence with a very long info string (the opener is echoed verbatim as `bridgeOpen`), the reopener alone could push the first chunk over the limit. Worth a `logger.warn` when the escape fires so it surfaces in production logs.
How can I resolve this? If you propose a fix, please make it concise.| const sent: SentMessageChunk[] = [] | ||
| for (let i = 0; i < chunks.length; i++) { | ||
| const piece = chunks[i]! | ||
| const id = await retryDiscord( | ||
| () => this.sendChunk(channel, piece.text, i === 0 ? replyToMessageId : undefined), | ||
| this.options.maxBackoffMs, | ||
| ) | ||
| sent.push({ id, bridgeOpen: piece.bridgeOpen, bridgeClose: piece.bridgeClose }) | ||
| } |
There was a problem hiding this comment.
sendSegmentChunks correctly separates resolution/split (outer retry) from individual sends (inner retries), preventing re-sends of earlier chunks. However, the inner retryDiscord calls hold a reference to channel captured from the outer retry block. If the channel reference goes stale between the two stages (rare but possible under prolonged rate-limiting), all inner retries fail against the same stale object. Fetching the channel lazily inside sendChunk would give each retry a fresh reference, at the cost of an extra round-trip per chunk.
| const sent: SentMessageChunk[] = [] | |
| for (let i = 0; i < chunks.length; i++) { | |
| const piece = chunks[i]! | |
| const id = await retryDiscord( | |
| () => this.sendChunk(channel, piece.text, i === 0 ? replyToMessageId : undefined), | |
| this.options.maxBackoffMs, | |
| ) | |
| sent.push({ id, bridgeOpen: piece.bridgeOpen, bridgeClose: piece.bridgeClose }) | |
| } | |
| const sent: SentMessageChunk[] = [] | |
| for (let i = 0; i < chunks.length; i++) { | |
| const piece = chunks[i]! | |
| // `channel` is captured from the outer retry block. On a long rate-limit | |
| // delay the reference could go stale; re-fetching inside the retry would | |
| // give a fresh reference but adds a round-trip per chunk. | |
| const id = await retryDiscord( | |
| () => this.sendChunk(channel, piece.text, i === 0 ? replyToMessageId : undefined), | |
| this.options.maxBackoffMs, | |
| ) | |
| sent.push({ id, bridgeOpen: piece.bridgeOpen, bridgeClose: piece.bridgeClose }) | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/discord/connector.ts
Line: 1133-1141
Comment:
`sendSegmentChunks` correctly separates resolution/split (outer retry) from individual sends (inner retries), preventing re-sends of earlier chunks. However, the inner `retryDiscord` calls hold a reference to `channel` captured from the outer retry block. If the channel reference goes stale between the two stages (rare but possible under prolonged rate-limiting), all inner retries fail against the same stale object. Fetching the channel lazily inside `sendChunk` would give each retry a fresh reference, at the cost of an extra round-trip per chunk.
```suggestion
const sent: SentMessageChunk[] = []
for (let i = 0; i < chunks.length; i++) {
const piece = chunks[i]!
// `channel` is captured from the outer retry block. On a long rate-limit
// delay the reference could go stale; re-fetching inside the retry would
// give a fresh reference but adds a round-trip per chunk.
const id = await retryDiscord(
() => this.sendChunk(channel, piece.text, i === 0 ? replyToMessageId : undefined),
this.options.maxBackoffMs,
)
sent.push({ id, bridgeOpen: piece.bridgeOpen, bridgeClose: piece.bridgeClose })
}
```
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
The markdown splitter targets 1800 chars but has a pathological escape (closers alone exceeding the budget) that can emit an over-length chunk. Surface it at the send boundary instead of letting Discord 400 silently. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thanks for the thorough pass, @greptile-apps — the sequence diagram is a nice touch. Went through all three: Issue 2 (guard-loop escape may emit an over-length chunk) — addressed in 99143fa, though at the send boundary rather than inside the splitter. Issue 1 (stripBridgeOpen fallback log) — leaving as-is. The fallback only matches by fence marker ( Issue 3 (stale Appreciate the review! |
|
All three responses are solid — thanks for the detailed follow-through. The Nothing blocking here. Looks good to merge. Tip: You can customize Greptile's behavior for this repo with |
✅ Acceptance-tested (2026-07-04)Verified live on a Discord server (GPT-5.5 bot, 1. Code fence across the boundary — a ~2500-char fenced block split into two messages; both chunks render as code (fence closed at the end of chunk 1, reopened at the start of chunk 2), no content lost:
2. Inline span across the boundary, hostile case — a single ~2500-char bold paragraph with no line breaks anywhere, so the splitter had no safe break point and had to cut mid-span — in fact mid-word (
The oversize-chunk warning ( 🤖 Generated with Claude Code |
…splitter
Property-based fuzzing of splitPreservingMarkdown surfaced two defect classes.
Defect 1 (invariant A — chunk.text.length <= maxLength): a fence whose opening
line carries a long info string was reopened by replaying that entire verbatim
line, so a chunk that reopened it could exceed maxLength (fuzz saw 1018 > 300).
Fences now reopen as the marker run plus at most the first whitespace-delimited
word of the info string (fenceReopener); in the splitter that word is further
capped so the reopener always leaves room for the fence's own closer and one
body char, making invariant A a hard guarantee for maxLength >= 60. The
reopener is a synthetic bridge (stripped on reconstruction), so losslessness is
unaffected.
Defect 2 (seam run-merge): the cut avoided bisecting a delimiter run but not a
cut where an inserted bridge string abutted an identical delimiter char in the
adjacent body, fusing runs and changing parsing (e.g. reopener "`" + body "`x"
-> "``x"; body "...a~~" + closer "~~" -> "~~~~"). The cut selection now (a)
does not prefer a newline that lands inside a non-fence construct (which would
fragment a small span and force a fusing bridge), and (b) pulls a cut left off
the offending delimiter until neither the reopener fuses with the next body's
first char nor the closer fuses with this body's last char, for the run-merging
chars ` ~ * _ | (fences are newline-anchored and never fuse). On the final
chunk, an unclosed inline-code span whose original content ends in backticks is
left open rather than emitting a closer that would fuse — Discord already
renders it literally and endCarry still reports it for continuation.
Fuzz (50k cases, maxLength in {60..2000}): zero A/B/D and zero seam-adjacency
failures. Losslessness and no-crash preserved. The scanMarkdown "C" oracle
still flags pre-existing, unrelated line-position backtick reparse (3+ backtick
inline code becoming a fence at a chunk's line start / mixed ```-vs-~~~ nesting
in isolation); these dropped ~55% vs baseline and never lose text.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
🔬 Property-based fuzz hardening (2026-07-04, follow-up commit
|
1 participant
Problem
Discord parses each message independently. When the bot's response is long enough to be divided across multiple messages, a markdown construct that straddles the boundary breaks. For code fences this cascades: an unclosed
```bashat the end of one message leaves the next message's content rendering as code, and the matching closing fence then becomes an opener — inverting every block for the rest of the response.This bites at two layers, both previously markdown-unaware:
Fix
A pure, dependency-free balancer (
src/utils/discord-markdown.ts) that closes any construct open at a chunk boundary and reopens it in the next message, threading carry state across separate sends. Covers code fences (```/~~~), inline code, and emphasis (*,**,***,_,__,~~,||).connector.sendSegmentChunksresolves mentions/emojis, then markdown-splits once, returning per-message bridge metadata — so resolution can never push a chunk over the limit and trigger an unrecorded second split.sendSegmentsthreadsmarkdownCarryacross the native + inline tool paths. Only exclusive constructs (fence/inline) cross a send boundary; dangling emphasis is left literal (re-opening a span the model may never close would render worse).MessageContext(bridgeOpen/bridgeClose) and stripped during context reconstruction, so the model reads back its original unbroken markdown rather than the bridging delimiters.messageContextsin the generated-image response path (was dropping prefix/suffix + bridge metadata there).Tests
src/utils/discord-markdown.test.ts— exhaustive unit suite (fences of varying run lengths, info-string rules, inline code, nested emphasis, flanking/escaping, startCarry/endCarry, budget/boundary edges, cross-send round-trip). Plus connector-level tests for resolve-then-split (incl. emoji expansion past the limit) and context-reconstruction stripping. Full suite green.Notes for reviewers
The emphasis parser is pragmatic, not full CommonMark — well-formed/nested cases are handled; pathological interleaving (
**a*b**) degrades to literal (no worse than today). Fences and inline code are the rock-solid path.🤖 Generated with Claude Code