relayburn-sdk: reader cleanup safe subset (#346)

[willwashburn]

Summary

Three safe reader-module cleanups flagged in #346:

• Collapse three near-identical string_field / first_nonempty_string / first_string_field helpers in reader/claude.rs into one parameterized helper string_field(obj, keys, require_nonempty). All 39 callsites updated.
• Simplify the two-arm verb() match in reader/classifier.rs by hoisting the prefix/normalized logic above the body.
• Extract the duplicated fallback block in reader/git.rs:resolve_uncached into a small closure.

No behavior change; this is dedup. The riskier subset of #346 (parser duplication, dead let _ audit in codex.rs) is deferred.

3 files changed, +62/-81 (net -19 LOC).

Test plan

• cargo build --workspace --all-targets clean
• cargo test --workspace passes (729/729)
• cargo clippy --workspace --all-targets no new warnings
• Reader fixtures parse byte-identically (covered by the existing parse-fixture suite)

🤖 Generated with Claude Code

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ec52c5a4-f4d2-44ff-bbc5-14c6f76dce21

📥 Commits

Reviewing files that changed from the base of the PR and between f4fd3da and 0792e6e.

📒 Files selected for processing (1)

• crates/relayburn-sdk/src/reader/claude.rs

---

📝 Walkthrough

Walkthrough

Refactors three independent reader modules: simplifies bash classifier verb normalization, consolidates alias-aware multi-key field access and updates many Claude parser sites with tests, and centralizes git resolver fallback behavior.

Changes

Bash Classifier Verb Logic

Layer / File(s)	Summary
Verb Computation crates/relayburn-sdk/src/reader/classifier.rs	parse_bash_command_inner's verb(binary, subcommand) refactored to compute normalized via a single match on subcommand; BashParse fields (binary, subcommand, normalized) remain functionally equivalent.

Claude JSONL Field Parsing

Layer / File(s)	Summary
Constants / Helper crates/relayburn-sdk/src/reader/claude.rs (2879-2899)	Introduce SESSION_ID_KEYS, TIMESTAMP_KEYS, SOURCE_VERSION_KEYS and a multi-key string_field(..., require_nonempty) helper that replaces prior single-key/first-* helpers.
Parser Integration (sync) crates/relayburn-sdk/src/reader/claude.rs (345-347, 392-394, 437-438, 590-607, 1151-1153, 1272-1284, 1348-1362, 1459-1516, 1803-1878, 1851-1876, 1971-1976)	Assistant, user, and system ingestion paths, record building, tool-result parsing, and relationship/evidence construction now use alias-aware string_field calls (with require_nonempty where applicable) for sessionId/session_id, timestamp/ts, uuid, tool_use_id, source_version, and provenance fields.
Parser Integration (incremental) crates/relayburn-sdk/src/reader/claude.rs (2552-2651, 2879-2899)	Incremental ingestion/compact-boundary handling updated to use the alias-aware string_field for session/timestamp keys and related metadata.
Tests / Fixtures crates/relayburn-sdk/src/reader/claude.rs tests (3135-3269)	Adds in-memory JSONL fixture and tests asserting that alias-spelled sessionId/timestamp inputs produce synchronized outputs between parse_claude_session and parse_claude_session_incremental across turns, content, events, relationships, tool_result_events, and evidence.

Git Resolver Fallback Handling

Layer / File(s)	Summary
Error Path Consolidation crates/relayburn-sdk/src/reader/git.rs (71-79)	resolve_uncached(cwd) introduces a fallback closure returning { project: cwd, project_key: None } and reuses it for both missing .git discovery and failed .git/config reads to centralize duplicate fallback behavior.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related issues

• Rust refactor: reader code cleanup (parser dup, helper consolidation, dead let_ blocks) #346: Implements the reader-module refactors (classifier verb, Claude string_field consolidation, git resolver fallback) referenced by the issue.

🐰 I hopped through keys both old and new,
I stitched their names so parsing grew,
Verbs match tidy, git falls back,
Claude learns many keys on track,
Small rabbit cheers the parsing crew.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main change: reader module cleanups, which matches the three refactoring tasks detailed in the changeset.
Description check	✅ Passed	The description is directly related to the changeset, detailing the three specific cleanups across reader/claude.rs, reader/classifier.rs, and reader/git.rs with clear context about behavior preservation and test validation.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch rust-reader-cleanup-safe

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/relayburn-sdk/src/reader/claude.rs`:
- Around line 345-346: The parser still only looks up "sessionId" and
"timestamp" keys, so alias-only records using "session_id" or "ts" are dropped;
update all mirror entry points (the user/system/incremental parsing branches
that currently assign session_id and timestamp via string_field) to accept both
aliases by changing the lookup arrays to include both variants (e.g.,
string_field(obj, &["sessionId", "session_id"], false) and string_field(obj,
&["timestamp", "ts"], false)); propagate this same change through the code paths
that lead into record_root, the explicit relationship collection, compaction
emission, and the tool-result event emission so alias-only records reach the
downstream handlers.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 12ee7a08-b27c-4beb-8d88-5481e3d621de

📥 Commits

Reviewing files that changed from the base of the PR and between e5e5b99 and ca60a8c.

📒 Files selected for processing (3)

• crates/relayburn-sdk/src/reader/classifier.rs
• crates/relayburn-sdk/src/reader/claude.rs
• crates/relayburn-sdk/src/reader/git.rs

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/relayburn-sdk/src/reader/claude.rs`:
- Around line 2886-2894: The string_field function currently returns the first
string value even if it's empty when require_nonempty is false, which allows a
blank primary key to mask a populated alias; change the logic in string_field to
prefer non-empty values across the provided keys: iterate keys, if you find a
non-empty string return it immediately; if you encounter an empty string
remember it (e.g., first_empty) but keep searching; after the loop return the
remembered empty string only if no non-empty was found and require_nonempty is
false; keep require_nonempty=true behavior to only accept non-empty strings. Use
the existing symbols string_field, obj.get(...).and_then(Value::as_str), and
require_nonempty to locate and implement this change.
- Around line 1971-1977: In record_evidence_from_line, propagate any
sourceVersion aliases into evidence.source_version instead of reading only the
literal "version" key: call string_field with the unified alias set (e.g.,
SOURCE_VERSION_KEYS or a new SOURCE_VERSION_KEYS constant) and assign its result
to evidence.source_version; locate the existing code that seeds
evidence.source_version from the literal "version" and replace it so it uses
string_field(lo, SOURCE_VERSION_KEYS, true) (keeping the same nil-check
semantics) so files emitting "version"/"sourceVersion"/"source_version" all
populate evidence.source_version consistently.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 2f1a7db8-2b9b-4a0b-be45-eb7e70dfa70f

📥 Commits

Reviewing files that changed from the base of the PR and between ca60a8c and f4fd3da.

📒 Files selected for processing (1)

• crates/relayburn-sdk/src/reader/claude.rs