Reviewing an Agent's SSE PR: The Split UTF-8 Byte, the Replayed Event, and What to Revert

The pull request arrived at 01:40 with 412 changed lines. Continuous integration was green. The streamed answer still read caf done instead of café done.

The agent had written a tidy streaming client. One decoding shortcut deleted a character. A second shortcut replayed an entire prompt after a dropped socket.

This review covers that diff. It splits the work into three buckets: what to trust, what to revert, what to test. The artifact below is a fake SSE server, standard library only, that reproduces both failures in about a second.

What the agent actually shipped

The PR added token streaming to a chat endpoint. Three files changed: an HTTP client, an event parser, and a reconnect helper. The commit message promised full test coverage.

The parser decoded every network chunk on its own. The reconnect helper resent the whole conversation on failure. Both choices look reasonable in a diff and both are wrong at runtime.

A reviewer cannot prove that by reading. The reviewer needs a server that sends bytes on command. That is the whole trick.

Step 1: Freeze the failure with a local SSE server

Real model endpoints are slow, rate-limited, and non-deterministic. A local server removes all three variables. It emits fixed bytes on a fixed schedule.

# fake_sse.py  -- python fake_sse.py, stdlib only
import http.server, socketserver, time

RAW = b'id: 1\ndata: {"delta": "caf\xc3\xa9 "}\n\n'
CUT = RAW.index(b"\xc3") + 1          # land inside the two-byte character

EVENTS = [
    RAW,                                # split across two TCP writes
    RAW,                                # same event id, replayed on purpose
    b'id: 2\ndata: {"delta": "done"}\n\n',
    b'data: [DONE]\n\n',
]

class Handler(http.server.BaseHTTPRequestHandler):
    def do_POST(self):
        self.send_response(200)
        self.send_header("Content-Type", "text/event-stream")
        self.end_headers()
        for i, ev in enumerate(EVENTS):
            if i == 0:
                self.wfile.write(ev[:CUT]); self.wfile.flush()
                time.sleep(0.05)
                self.wfile.write(ev[CUT:])
            else:
                self.wfile.write(ev)
            self.wfile.flush()
            time.sleep(0.02)
    def log_message(self, *args):
        pass

if __name__ == "__main__":
    with socketserver.TCPServer(("127.0.0.1", 8765), Handler) as srv:
        srv.serve_forever()

Run the server, then point the PR branch at http://127.0.0.1:8765. Two defects surface on the first response. One character disappears. One delta appears twice.

The server never touches a model provider. It costs nothing to run and it fails identically every time.

Step 2: Classify every hunk before editing anything

A reviewer needs a sorting rule, not a mood. The three-column table below covers most agent-generated network code.

Hunk in the diff	Verdict	Why
Byte buffer that splits on \n\n	Trust	Framing is correct; boundaries are respected
Header setup and Content-Type check	Trust	Standard, no hidden behaviour
chunk.decode("utf-8", errors="ignore")	Revert	ignore silently drops split characters
Blind retry that resends the prompt	Revert	Duplicates output and doubles token spend
Blanket 5s socket timeout	Test	Long generations will be cut mid-event
Missing [DONE] handling	Test	Loop exits only on socket close today

Write the table into the review comment. It converts a subjective "looks fine" into six checkable claims.

The errors="ignore" line is the interesting one. It never throws, never logs, and never shows up in a stack trace. It turns a hard failure into quiet data loss.

Step 3: Fix the decoder, keep the framing

The framing was already right. The bug lived one layer above it, at decode time.

# test_stream.py -- python -m unittest test_stream
def read_events(resp, size=7):          # odd size forces split reads
    buf = b""
    while True:
        chunk = resp.read(size)
        if not chunk:
            break
        buf += chunk
        while b"\n\n" in buf:
            block, buf = buf.split(b"\n\n", 1)
            yield block.decode("utf-8")   # decode at event boundary

The rule is one sentence long. Buffer bytes, split events, decode whole blocks. Never decode a chunk you did not frame first.

The reverted line becomes unnecessary. No error handler hides a problem, because the problem no longer exists at that layer.

Step 4: Stop the replay, or make it idempotent

The reconnect helper resent the full prompt after any socket error. On a flaky link, that produced two answers merged into one transcript.

Two acceptable repairs exist. Send Last-Event-ID and let the stream resume, or keep the replay and deduplicate on event ids before appending.

seen = set()
for block in read_events(resp):
    if block.strip() == "data: [DONE]":
        break
    eid = re.search(r"^id: (\d+)$", block, re.M)
    if eid and eid.group(1) in seen:
        continue
    if eid:
        seen.add(eid.group(1))
    deltas.append(json.loads(data.group(1))["delta"])

The PR got neither repair. It got a retry loop with no memory of what it already printed.

Step 5: The test matrix that would have caught it

Four cases cover the failure surface. Each one runs against the fake server in under two seconds.

1. Split multi-byte character — expect café done, not caf done.
2. Duplicate event id — expect one occurrence in the joined output.
3. Missing [DONE] — server closes early; client must exit, not hang.
4. Mid-event disconnect — no partial JSON reaches the consumer.

Write assertions on the joined string, not on call counts. Joined output catches both the decode bug and the replay bug with one line.

Where a free model and a free server fit the loop

Locking a bug down with a local server is the cheap half. The other half is rerunning the review with a second pair of eyes on the same diff.

The operator lists free model access and a free server option for the MonkeyCode project, including a free token allowance (ten million tokens as stated at the time of writing). Terms can change, so read the current page before planning around them. Disclosure: This article was prepared as part of MonkeyCode's product outreach.

In practice, the free server hosts fake_sse.py, and a second model reviews only the parser file. Narrow scope keeps the second opinion useful. Asking one model to audit the whole 412-line diff mostly produces restated intent.

If the harness looks useful, start with the fake server before pointing anything at a live endpoint. The local repro is the part that keeps working after the model changes.

Limitations and who should skip this

This workflow fits diffs that touch network framing, retries, or decoding. It does not help with business-logic PRs, schema migrations, or UI work.

It also assumes the reviewer can run code locally. Teams without a disposable environment should not adopt the fake-server step first; the table in Step 2 still applies.

Finally, agent PRs change faster than review habits. A diff that passes this matrix can still be wrong about intent. Read the commit message last, and read the byte handling first.

What to take away

Trust framing, revert silent decoders and blind replays, test at event boundaries. That ordering finds the bug before production traffic does.

The 412-line PR needed six table rows and four assertions. The rest of the diff was fine, and saying so is part of the review.