Skip to content

fix(http): get-stream yields lines on newline, not buffer-fill (ILO-489)#779

Merged
danieljohnmorris merged 3 commits into
mainfrom
feature/getstream-prompt-yield
May 25, 2026
Merged

fix(http): get-stream yields lines on newline, not buffer-fill (ILO-489)#779
danieljohnmorris merged 3 commits into
mainfrom
feature/getstream-prompt-yield

Conversation

@danieljohnmorris
Copy link
Copy Markdown
Collaborator

Summary

get-stream / pst-stream (ILO-448) yield lines lazily, but the native backend wrapped minreq's ResponseLazy in BufReader::lines(), which blocks on a full ~8 KiB read-buffer fill before surfacing any line. So a slow SSE upstream that flushes one short event then idles had its lines batched until the buffer filled or the connection closed. Functionally correct (all lines arrive in order) but latency was buffer-bound, not event-bound.

Motivation: this directly unblocks ILO-482's end-to-end test, which proxies an upstream via get-stream to prove httpd lazy-body streaming and was flaky (~1 in 3) because get-stream sometimes hadn't yielded the first line when the test read. Per-newline yield makes that test deterministic.

Repro before/after

A server flushes first\n, idles 300ms, then sends second\n without closing.

  • Before: the client doesn't surface first until second (or close) arrives - line 1 lands ~200us after line 2 hits the wire (batched).
  • After: the client surfaces first the instant its \n arrives, well before line 2 is sent.

What changed (per commit)

  • fix(http): stream get-stream lines on newline, not buffer-fill - replace the BufReader::lines() wrapper with a LazyLineSplitter that consumes ResponseLazy's byte iterator incrementally, emitting each line the moment a \n is seen. Strips a trailing \r for chunked/CRLF, yields any partial line at EOF, maps minreq::Error::IoError back to the io::Error the existing ILO-R009 http-stream read error path expects. Covers all four builtins (get-stream, get-stream-h, pst-stream, pst-stream-h).
  • test(http): prove get-stream yields per-newline, not per-buffer - slow-drip server reports the instant line 2 hits the wire; the test asserts line 1 arrived before then with margin. Verified to fail on the old buffered code and pass on the fix. Plus a trailing newline-less EOF line test.
  • docs: note get-stream per-newline yield (ILO-489) - CHANGELOG Unreleased entry + skill doc latency note.

Test plan

  • cargo fmt
  • cargo clippy --release --all-targets --features cranelift -- -D warnings clean
  • cargo test --release --features cranelift green (406 lib + all integration; only the two tolerated baseline doctests in src/pkg.rs / src/verify.rs fail, as on main)
  • new prompt-yield test fails on old code, passes on fix
  • existing ILO-448 streaming tests still pass (no regression)
  • rust-review subagent: approve, no changes required

Follow-ups

None.

@danieljohnmorris danieljohnmorris added the mac Mac (Dan's laptop) agent is working on this label May 23, 2026
@danieljohnmorris danieljohnmorris force-pushed the feature/getstream-prompt-yield branch from ac2783c to fcb06ec Compare May 23, 2026 22:08
@codecov
Copy link
Copy Markdown

codecov Bot commented May 23, 2026

Codecov Report

❌ Patch coverage is 80.55556% with 7 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/interpreter/http_wasm.rs 80.55% 7 Missing ⚠️

📢 Thoughts on this report? Let us know!

replace the BufReader::lines() wrapper around minreq's ResponseLazy
with a LazyLineSplitter that consumes the byte iterator incrementally
and emits each line the instant its newline arrives. the BufReader path
blocked on a full read-buffer fill before surfacing any line, so a slow
SSE upstream had its events batched until the buffer filled or the
connection closed. trailing \r stripped for chunked/CRLF encoding, EOF
yields any partial line, mid-stream errors still surface as io errors.
add a slow-drip server that flushes one line, idles 300ms, then sends a
second, reporting the instant line 2 hits the wire. assert the client
surfaces line 1 before that instant - this fails on the old buffered
code (line 1 batched until line 2 arrives) and passes on the fix. also
cover a trailing newline-less line yielded at EOF.
@danieljohnmorris danieljohnmorris force-pushed the feature/getstream-prompt-yield branch from fcb06ec to ee36fad Compare May 24, 2026 23:48
@danieljohnmorris danieljohnmorris added mac-reviewing Currently being merge-prepped by mac-side agent and removed mac Mac (Dan's laptop) agent is working on this labels May 24, 2026
@danieljohnmorris danieljohnmorris merged commit 0b7dccb into main May 25, 2026
19 of 22 checks passed
@danieljohnmorris danieljohnmorris deleted the feature/getstream-prompt-yield branch May 25, 2026 00:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mac-reviewing Currently being merge-prepped by mac-side agent

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant