Skip to content

Rust refactor: reader code cleanup (parser dup, helper consolidation, dead let_ blocks) #346

@willwashburn

Description

@willwashburn

Context

A handful of reader-module cleanup items that don't fit elsewhere.

1. Two parsers for one job

crates/relayburn-sdk/src/reader/claude.rs:89-108 parse_claude_session_with_counter uses BufReader::lines() (allocates String per line); :2442+ run_incremental reads bytes manually. They overlap in scope but neither delegates.

Fix: make the non-incremental path call run_incremental(start_offset = 0). Drop the duplicate. Same opportunity in codex.rs:144-164 and opencode.rs:81-99parse_*_session is a manual repack of the _incremental result into a smaller struct. Either implement From<…Incremental> for …Result or drop the repack.

2. Three near-identical string-field helpers

crates/relayburn-sdk/src/reader/claude.rs:2896-2917 defines string_field, first_nonempty_string, first_string_field — three nearly-identical helpers.

Fix: collapse to one with keys: &[&str], require_nonempty: bool.

3. Dead let _ = (...) blocks in codex.rs

crates/relayburn-sdk/src/reader/codex.rs:1097let _ = (next_event_index, tool_result_counters);
:1189 — a larger let _ = (cumulative, session_id, …); block.

Comments justify these as "kept for parity with the TS state machine," but say nothing about whether the Rust code actually uses them. Audit each one — if a committed_* snapshot is set but never read post-loop, the path is broken or the snapshot is genuinely dead.

4. verb() two-arm match could collapse

crates/relayburn-sdk/src/reader/classifier.rs:434-447 — two match arms differing only in whether subcommand is Some.

Fix: let normalized = match sub { Some(s) => format!("{binary} {s}"), None => binary.to_string() }; then a single arm.

5. resolve_uncached duplicated fallback blocks

crates/relayburn-sdk/src/reader/git.rs:69-90 — two identical "fallback" blocks. Extract fn fallback(cwd: &str) -> ResolvedProject.

6. Misnamed function

crates/relayburn-sdk/src/reader/codex.rs:1231-1233 memchr_newline is named for memchr but uses iter().position(). Either rename or actually use the memchr crate (already a transitive dep). Covered by perf issue #(typed deser); flag here for visibility.

References

  • Reader review notes from the May 2026 Rust review.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions