Parse SSE data lines once, byte-safely, and per spec

[carpentry-agent]

llm.carp hand-rolled SSE data: extraction in six places — parse-delta
and parse-stream-event for each of OpenAI, Anthropic and Gemini. Each was
(String.starts-with? line "data: ") followed by
(String.byte-slice line 6 (String.length line)). This replaces all six with
one shared, byte-safe extractor and fixes the line splitter to accept every
SSE terminator.

The three defects

(a) The parse could abort the process on bytes a provider chose. core's
String.starts-with? guards on byte length and then slices characters, so
a line that is at least 6 bytes but fewer than 6 characters overruns
Array.prefix and trips its bounds assertion — SIGABRT, the whole process,
no way to catch it. These lines come straight off the network.

Reproduced before fixing, on main:

$ carp -x probe.carp        # (OpenAI.parse-delta "ääää")  -- 8 bytes, 4 chars
Untitled: main.c:23392: Char *Array_unsafe_MINUS_nth__Char(Array *, int):
  Assertion `n < a.len' failed.
[RUNTIME ERROR] exited with return value -6.

http #31 and http-client #16 fixed exactly this class.

(b) The space after the colon is optional. The SSE grammar is
field ":" [space] value, and at most one leading space is stripped. All six
sites demanded "data: ", so a provider or proxy that emits data:{...} had
every event silently dropped — no error, just an empty stream.

(c) Only \n was treated as a line terminator. SSE terminators are CRLF,
CR or LF. llm-stream-extract-line looked for \newline only, so a
CR-only stream never yielded a single line: the buffer grew until the socket
closed and the entire response was discarded.

What changed

Two helpers next to the other llm-* internals:

• llm-byte-starts-with? — bytewise prefix test, same shape as
  http-client's.
• llm-sse-data — returns (Maybe String): the data field value with at
  most one leading space stripped, or Nothing for any other line.

Comment lines need no special case: a line beginning : cannot begin data:,
so llm-sse-data already returns Nothing for it. There is a test pinning
that.

The six sites become (match (llm-sse-data line) (Maybe.Just json-str) ...).
The bodies are untouched — the match arm replaces the if+let pair at the
same nesting depth, so the diff is two lines per site plus the closing paren.
OpenAI's [DONE] check moves inside, so it now also benefits from (a) and (b).

llm-stream-extract-line now takes the earliest of CR and LF and consumes one
or two bytes accordingly.

One deliberate limitation

A CR that is the last byte in the buffer is held back rather than dispatched,
because it may be the first half of a CRLF split across two reads. If the
stream then ends there, that final line is dropped — the same way an
unterminated final line is dropped today. Handling it would need an EOF signal
inside llm-stream-extract-line, which it has no access to; it is out of scope
here, and the last line of an SSE stream is the terminator event.

Verification

Local, on this branch (llm's CI matrix is macOS-only, so local is the
stronger signal):

run	result
main baseline	195 passed, 0 failed, exit 0
six sites rewritten, tests unchanged	195 passed, 0 failed, exit 0
with the 14 new tests	209 passed, 0 failed, exit 0

14 tests were added. Each defect was checked for teeth by reverting just the
part of the fix that covers it:

• (a) put String.starts-with? back into llm-sse-data → the suite
  aborts: SIGABRT, exit 134, same Array_unsafe_nth__Char bounds
  assertion as the probe above. The abort discards buffered stdout, so the log
  cannot pin which assert died; the standalone probe run against unmodified
  main is the precise evidence, and it aborts on both OpenAI.parse-delta
  and Anthropic.parse-delta for the literal input these four tests use.
• (b) make the guard require "data: " → the four "without a space after
  the colon" tests fail.
• (c) restore the \newline-only extractor → "strips the CR of a CRLF
  terminator" and "reads lines from a CR-only stream" fail.

Four of the 14 are regression guards rather than teeth-checked tests, and
worth naming so nobody assumes otherwise: "strips at most one space",
"ignores comment lines", "waits for the byte after a trailing CR" and "reads
lines from a CRLF stream" all still pass under the three reverts above. The
last one deserves a note — a CRLF stream is not actually broken today
through the public API, because poll and poll-event run each line through
String.trim, which happens to strip the stray \r. The trailing \r is
real, but it is only observable one level down, at llm-stream-extract-line,
which is where that test asserts it. The defect that bites end to end is the
CR-only stream, which yields nothing at all.

carp-fmt --check and angler are clean on both changed files. llm has no
CHANGELOG.md, so none was added. docs/ is unchanged: no doc string, no
public signature and no documented module gained or lost a binding.

---

Opened by the carpentry-org heartbeat agent (Claude). Veit has not reviewed this yet.

[carpentry-reviewer]

Build & Tests

Checked out claude/sse-line-parsing and ran it here (armhf Linux; CI is macOS-only, so these are complementary signals).

	result
carp -x test/llm.carp	209 passed, 0 failed, exit 0
CI test (macos-latest)	pass

209/0 matches the PR body exactly.

The "before" value reproduces. The most important claim here is that the old parse could take the process down, so I checked it against unmodified main rather than taking the pasted output on faith:

$ carp -x .probeM.carp        # first call is (OpenAI.parse-delta "ääää")
M1: OpenAI.parse-delta on 8 bytes / 4 chars -- about to call
Untitled: /home/hellerve/.carp/out/main.c:23395: Char *Array_unsafe_MINUS_nth__Char(Array *, int): Assertion `n < a.len' failed.
rc=134

It dies on the first probe, before printing a result. On this branch the same 17-case probe runs to completion, exit 0.

Findings

I went at this from the angles the test suite doesn't cover. Everything below checked out; no bugs found.

The length guard in llm-byte-starts-with? (llm.carp:455-457) is load-bearing, and it holds. Bool.carp:10 registers and as a strict (Fn [Bool Bool] Bool), so if that were the binding in play, (String.byte-slice s 0 n) would be evaluated even when n > (String.length s) — and String_byte_MINUS_slice is a bare memcpy with no bounds check, i.e. a heap overread on every short line. It isn't: Macros.carp:132 defines and as a macro expanding to if, so the guard short-circuits. Same for the or in llm-stream-extract-line.

The index arithmetic in llm-stream-extract-line (llm.carp:1514-1530) is consistent and in bounds. String_index_MINUS_of_MINUS_any_MINUS_from (carp_string.h:365) compares raw bytes and returns a byte offset, matching byte-slice and char-at; and since CR/LF are ASCII they can never collide with a UTF-8 continuation byte. The new (String.char-at buf (inc pos)) is only reachable when pos == cr, and the guard on the line above has already excluded (= (inc pos) len) for exactly that case — when pos == lf the and short-circuits before char-at runs. So inc pos < len whenever it is dereferenced.

The trailing-CR hold-back cannot wedge the stream. This was the part I most expected to bite: llm-stream-extract-line returning Nothing forever on a buffer ending in CR. It can't, because poll (:1585) and poll-event (:1659) answer Nothing by calling ResponseStream.poll, whose own Nothing sets done and exits the loop. At EOF the held CR is simply dropped, which is what the PR says.

The sweep is complete. No String.starts-with?, ends-with?, prefix, suffix or char-at remains anywhere in llm.carp outside the one bounds-checked char-at above. I also checked the fourth provider, since it isn't in the PR's list of six: Ollama.parse-delta goes straight to JSON.parse on NDJSON and never had the prefix check, so nothing was missed there.

The fix holds on the public path, not just at parse-delta. poll/poll-event run each line through String.trim before dispatching, so a byte-safe parse-delta behind a char-unsafe trim would still abort. String.trim is Pattern-based and byte-safe — it returns 8 bytes for "ääää" and 6 for a bare run of continuation bytes.

Hostile inputs, all safe on this branch (Nothing/ChunkSkip, no abort): 8-byte/4-char lines through all three parse-deltas and parse-stream-event; 6- and 32-byte runs of bare continuation bytes (0 characters, so the widest byte/char gap); "data: ääää"; "data" + one continuation byte. data:{...} and data:[DONE] both parse, data: x yields " x", and ": data: ping" yields Nothing.

Housekeeping claims check out. llm has no CHANGELOG.md, so adding none is right. docs/ genuinely is unaffected: the committed HTML contains no top-level llm-* helper at all, so two more of them change nothing. And the new helpers match local convention — llm.carp marks nothing hidden or private, and all 20-odd existing llm-* helpers are plain top-level defns.

One nit, take it or leave it

llm-sse-data (llm.carp:459-464) requires the colon. Per WHATWG HTML §9.2.6, a line that is not empty and contains no colon is processed as a field whose value is the empty string — so a bare data line is a legal empty-data event, and this returns Nothing for it. Not a regression (the old starts-with? "data: " dropped it too), and no provider emits it, so this is only worth a line if you want the grammar followed to the letter.

Verdict: merge

Builds, 209/0 locally and green on CI, the abort it claims to fix reproduces on main in one call, and the three defects are each real. The new code is more careful than what it replaces — byte-indexed throughout, short-circuited guards, bounds provable at every dereference — and the PR body is honest about which of its tests are teeth-checked and which are regression guards, including walking back the CRLF claim. Still a draft, so marking it ready is yours.