relayburn-sdk: collapse parse_claude_session onto run_incremental (#346)#417
Conversation
Last remaining cleanup from #346: the non-incremental claude parser kept its own `ParseState` machinery in parallel with `run_incremental`, both reading the same JSONL shape but along separate codepaths. PR #371 deferred this because the two paths disagreed on trailing in-progress turns (incremental skips them; non-incremental emits them). This commit adds an `emit_in_progress` flag to `run_incremental`: - false (incremental callers): keep the existing "back up end_offset to the earliest in-progress message and skip those turns" behavior, so the next resume call re-reads them. - true (full-parse caller): use `cursor_offset` for end_offset and emit every working record. This matches the prior `ParseState::finish` output, including assistants without a `stop_reason`. `parse_claude_session_with_counter` now builds a `ParseIncrementalOptions { start_offset: Some(0), .. }` and calls `run_incremental(.., true)`, then converts via a new `From<ParseIncrementalResult> for ParseResult`. The ParseState struct, its impl, the `record_root` / `collect_explicit_claude_relationships` non-incremental helpers, and `derive_file_session_id` are gone — net -374 LoC in claude.rs. The full workspace test suite (claude reader + 600+ other tests) passes unchanged, validating that the single-shot output matches what `ParseState` produced.
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThis PR consolidates duplicate parsing logic by refactoring ChangesParse Session Unification
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Devin Review found 1 potential issue.
⚠️ 1 issue in files not directly in the diff
⚠️ Single-shot parse silently drops final line if file lacks trailing newline (crates/relayburn-sdk/src/reader/claude.rs:2171-2173)
The old ParseState codepath used BufReader::read_line, which returns the final line at EOF even without a trailing \n. The new delegation to run_incremental uses read_until(b'\n') with an explicit guard at crates/relayburn-sdk/src/reader/claude.rs:2171 that breaks when line_buf.last() != Some(&b'\n'). For the incremental path (emit_in_progress = false) this is correct — the next call re-reads from cursor_offset. But for the full single-shot parse (emit_in_progress = true), there is no subsequent call, so a syntactically complete JSON line that happens to lack a trailing newline (e.g. file truncated mid-write, or writer hadn't flushed the \n yet) is permanently and silently lost. The CHANGELOG states "Behavior is unchanged" but this is a semantic difference from the prior implementation.
View 3 additional findings in Devin Review.
Devin Review flagged that switching `parse_claude_session_with_counter` to delegate to `run_incremental` regressed one edge case: the prior `ParseState` path used `BufReader::read_line`, which surfaces the final line at EOF even without a trailing `\n`. `run_incremental`'s `read_until(b'\n')` loop guards against partial trailing lines because the incremental caller resumes at `cursor_offset` next tick — but a single-shot full parse has no next tick, so a syntactically complete final JSON line lacking `\n` (mid-write truncation, unflushed writer) was silently dropped. Fix: gate the partial-line `break` on `!emit_in_progress`. When the single-shot caller asks us to emit in-progress records, treat a non-newline-terminated tail as a complete line and bump `cursor_offset` past its body so the per-record offset filters still admit it. Regression test `full_parse_emits_final_line_without_trailing_newline` writes simple-turn.jsonl with the trailing `\n` stripped and asserts the turn still lands.
Summary
Last remaining cleanup from #346. PR #371 already landed items 2–6 plus
the codex/opencode half of item 1, but explicitly deferred the claude
side because the two parsers disagreed on trailing in-progress turns:
ParseState(non-incremental) emits assistants without astop_reason.run_incrementalskips them and backsend_offsetup so the nextresume call re-reads them.
This PR collapses the two by threading an
emit_in_progress: boolflagthrough
run_incremental:false→ existing incremental semantics (skip + back upend_offset).true→ usecursor_offsetforend_offsetand emit every workingrecord, matching the prior
ParseState::finishcontract.parse_claude_session_with_counternow builds aParseIncrementalOptions { start_offset: Some(0), .. }, callsrun_incremental(.., true), and converts via a newFrom<ParseIncrementalResult> for ParseResult. The 333-lineParseStatestruct + impl, the non-incrementalrecord_rootandcollect_explicit_claude_relationshipshelpers, andderive_file_session_idare deleted. Net -374 LoC inreader/claude.rs.Test plan
cargo build -p relayburn-sdkcleancargo test --workspace(all 781 tests pass, including the 53claude reader tests covering full-parse output, incremental
output, and the resume cursor)
cargo fmt --checkoncrates/relayburn-sdk/src/reader/claude.rsclean
Closes #346.
https://claude.ai/code/session_01FvHFirtBvP9m4jWjVcB6v7
Generated by Claude Code