feat(ingest): single-transcript Claude hook fast-path + global --quiet#420
feat(ingest): single-transcript Claude hook fast-path + global --quiet#420willwashburn wants to merge 5 commits into
Conversation
Restores 1.x parity gaps called out in #375: - `relayburn-sdk`: new `ingest_claude_transcript_path` verb that parses just the one JSONL the caller hands it and persists an EOF cursor so a follow-up `ingest_all` skips it. `ingest_claude_session` now delegates through it. - `relayburn-cli`: `burn ingest --hook claude` drives the new fast-path on the payload's `transcript_path`, bounding per-hook cost to one parse. Falls back to a full sweep if the payload omits the path. - `relayburn-cli`: `--quiet` is no longer hook-only — accepted in default (one-shot) and `--watch` modes too. One-shot mode still writes its final summary on stdout so pipelines stay capturable. - `relayburn-cli`: documents the intentional non-port of 1.x's `--opencode-stream` / `--opencode-url` / `--opencode-global` flags (the file-based OpenCode adapter + FS-event watch is the supported 2.x path; the SDK still ships `OpencodeStreamIngestor` for embedders who want to consume the SSE feed themselves). Tests: - SDK: `ingest_claude_transcript_path_*` covers the EOF cursor contract and the missing-file no-op policy. - CLI smoke: one-shot stdout summary, `--quiet` keeps stdout summary, hook fast-path no-op on missing transcript, hook payload validation, `--quiet` suppresses the hook payload warning, watch+hook mutex. Closes #375. https://claude.ai/code/session_011ubB69Zxijqb1BsYVYL9iQ
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds an SDK fast-path to ingest a single Claude transcript file, broadens the CLI ChangesClaude Single-Transcript Fast-Path and Quiet Mode Expansion
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 unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bf809ad990
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .get("transcript_path") | ||
| .and_then(|x| x.as_str()) | ||
| .map(PathBuf::from); | ||
| if !has_session || transcript.is_none() { |
There was a problem hiding this comment.
This guard returns early whenever transcript_path is absent, which makes the later None => ingest_all(...) fallback branch unreachable. In hook mode, payloads that contain a valid session_id but omit transcript_path (the exact older-client case described in this change) will now be ignored instead of triggering a full sweep, so new turns can be missed entirely.
Useful? React with 👍 / 👎.
The previous guard returned 0 whenever transcript_path was absent, making the documented ingest_all fallback unreachable. Older Claude Code hook payloads carry session_id without transcript_path; they now fall through to a full sweep instead of being ignored. Add a smoke test pinning the new contract: a payload with session_id but no transcript_path runs under --quiet without stderr noise. Caught by Codex review on #420. https://claude.ai/code/session_011ubB69Zxijqb1BsYVYL9iQ
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🟡 run_watch always creates a progress spinner even when --quiet is passed
The run_watch function unconditionally creates a TaskProgress spinner at line 165 (let progress = TaskProgress::new(globals, "ingest")) and calls progress.set_task(...) on lines 166, 175, 226, and 234 regardless of the quiet flag. Both run_once (line 97) and run_hook (line 342) correctly gate spinner creation with (!quiet).then(|| ...), but run_watch does not. This means burn ingest --watch --quiet still displays a visible spinner on stderr, contradicting the PR's documented behavior ("suppresses stderr progress spinner / breadcrumbs") and the --quiet flag description at crates/relayburn-cli/src/cli.rs:158-163.
(Refers to lines 165-166)
Prompt for agents
In run_watch at crates/relayburn-cli/src/commands/ingest.rs, the progress spinner should be conditionally created based on the quiet flag, matching the pattern used by run_once and run_hook.
The fix requires wrapping the progress in Option<TaskProgress> (like run_once does with (!quiet).then(|| ...)) and guarding all subsequent progress.set_task, progress.finish_and_clear, progress.is_visible, and progress.clone calls behind if-let checks. This is a fairly pervasive change in run_watch since the progress variable is used throughout the function (lines 165-277). The same pattern used in run_once (lines 97-136) and run_hook (lines 342-387) should be followed.
Key locations to update:
- Line 165-166: conditionally create spinner
- Line 170, 175, 182: error handling and set_task calls
- Line 199: is_visible check
- Lines 213, 218: clone for the async block
- Lines 226, 234: set_task inside the ingest loop
- Line 277: finish_and_clear
Was this helpful? React with 👍 or 👎 to provide feedback.
| } | ||
| return 0; | ||
| } | ||
| transcript |
There was a problem hiding this comment.
🔴 Unreachable ingest_all fallback: early return rejects payloads missing transcript_path before the None branch can fire
The run_hook function at crates/relayburn-cli/src/commands/ingest.rs:326 returns early when transcript.is_none(), so transcript_path is always Some(...) after the validation block. This makes the None => ingest_all(...) branch at line 383 dead code. The doc comment (lines 290-294) explicitly states the intent to "fall back to ingest_all when the payload is missing transcript_path (older Claude Code releases occasionally elide it)", but this fallback is never reachable. Payloads from older Claude Code releases that provide session_id but not transcript_path are silently ignored instead of triggering a full sweep.
(Refers to lines 326-334)
Prompt for agents
In run_hook at crates/relayburn-cli/src/commands/ingest.rs, the validation block on lines 319-340 requires both session_id and transcript_path to be present, returning 0 if either is missing. This prevents the None => ingest_all fallback at line 383 from ever executing.
The fix: change the early return condition to only reject when session_id is missing (the truly required field). When session_id is present but transcript_path is absent, the function should proceed with transcript_path as None, which will correctly hit the ingest_all fallback branch.
Specifically, the condition on line 326 should change from:
if !has_session || transcript.is_none()
to:
if !has_session
And optionally adjust the eprintln message to only mention session_id.
Was this helpful? React with 👍 or 👎 to provide feedback.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/relayburn-sdk/src/ingest/ingest.rs (1)
347-353:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon’t treat every metadata error as “missing file.”
Line 350-Line 353 currently turns all
fs::metadatafailures into a no-op. That also swallows permission/IO errors and can silently drop ingest work. Keep no-op only forNotFound, and return other errors.Proposed fix
- match fs::metadata(file) { + match fs::metadata(file) { Ok(m) if m.is_file() => {} Ok(_) => return Ok(IngestReport::empty()), - Err(_) => { + Err(err) if err.kind() == std::io::ErrorKind::NotFound => { eprintln!("[burn] no session file found at {}", file.display()); return Ok(IngestReport::empty()); } + Err(err) => return Err(err.into()), }🤖 Prompt for 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. In `@crates/relayburn-sdk/src/ingest/ingest.rs` around lines 347 - 353, The metadata error handling in the fs::metadata(file) match is treating all errors as “file missing” and silently dropping others; change the Err arm to inspect the io::ErrorKind: if e.kind() == io::ErrorKind::NotFound then print the message and return Ok(IngestReport::empty()), otherwise propagate the error (convert/return the io::Error as the function's error type). Update the match on fs::metadata(file) (the block using file, IngestReport::empty()) to only suppress NotFound and return Err for permission/IO errors so legitimate failures are not swallowed.crates/relayburn-cli/src/commands/ingest.rs (1)
165-167:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
--watch --quietstill drives progress task updates.
TaskProgressis created and updated in watch mode regardless ofquiet. That can leak spinner/breadcrumb output in quiet mode instead of suppressing it.Also applies to: 226-234
🤖 Prompt for 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. In `@crates/relayburn-cli/src/commands/ingest.rs` around lines 165 - 167, TaskProgress is being created and updated even when the CLI is in quiet mode, causing spinner/breadcrumb output to leak; modify the ingest command to only construct and use TaskProgress when quiet is false (e.g., check globals.quiet) — either by guarding the TaskProgress::new(...) call and all subsequent progress.set_task(...) / progress.finish() calls (including the other block around the 226-234 region) with a conditional or by adding a quiet-aware constructor for TaskProgress and early-return no-op progress operations when quiet is set; target the TaskProgress variable and all calls like progress.set_task and progress.finish so no progress updates occur in quiet mode.
🧹 Nitpick comments (1)
crates/relayburn-cli/tests/smoke.rs (1)
492-503: ⚡ Quick winPin quiet-mode stderr suppression in one-shot ingest.
This test currently proves stdout summary retention, but it doesn’t assert that
--quietsuppresses stderr breadcrumbs in one-shot mode. Add a stderr-empty assertion so quiet regressions fail deterministically.Suggested diff
fn ingest_one_shot_quiet_keeps_stdout_summary() { let home = tempfile::TempDir::new().expect("tmp RELAYBURN_HOME"); burn() .args(["ingest", "--quiet"]) .env("RELAYBURN_HOME", home.path()) .env("HOME", home.path()) .env("NO_COLOR", "1") .assert() .success() - .stdout(predicate::str::contains("[burn] ingest: ingested")); + .stdout(predicate::str::contains("[burn] ingest: ingested")) + .stderr(predicate::str::is_empty()); }🤖 Prompt for 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. In `@crates/relayburn-cli/tests/smoke.rs` around lines 492 - 503, The test ingest_one_shot_quiet_keeps_stdout_summary must also assert that stderr is empty when --quiet is used; update the assert chain on the burn() invocation to include a stderr assertion (e.g., .stderr(predicate::str::is_empty())) so the test fails if one-shot quiet mode emits breadcrumbs to stderr while still checking stdout contains the ingest summary.
🤖 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 `@CHANGELOG.md`:
- Around line 9-13: Remove the issue closure text from the changelog entry: edit
the `relayburn-sdk` bullet that documents `ingest_claude_transcript_path(ledger,
path, opts)` (the per-transcript Claude fast-path used by `burn ingest --hook
claude`) and delete the trailing "Closes `#375`." so the bullet remains
impact-first with no issue/PR reference.
In `@crates/relayburn-cli/src/commands/ingest.rs`:
- Around line 319-333: The current block around the transcript_path check
returns early when transcript_path is missing, making the older-payload fallback
that calls ingest_all unreachable; instead, when has_session is true but
transcript is None, call ingest_all (the existing ingest_all function) and
return its result (respecting the quiet flag for any eprintln) rather than
returning 0. Locate the match/Ok branch that defines has_session and transcript
(variables transcript_path, has_session) and replace the early return with logic
that invokes ingest_all(...) for the full-sweep fallback, keeping the existing
logging behavior.
---
Outside diff comments:
In `@crates/relayburn-cli/src/commands/ingest.rs`:
- Around line 165-167: TaskProgress is being created and updated even when the
CLI is in quiet mode, causing spinner/breadcrumb output to leak; modify the
ingest command to only construct and use TaskProgress when quiet is false (e.g.,
check globals.quiet) — either by guarding the TaskProgress::new(...) call and
all subsequent progress.set_task(...) / progress.finish() calls (including the
other block around the 226-234 region) with a conditional or by adding a
quiet-aware constructor for TaskProgress and early-return no-op progress
operations when quiet is set; target the TaskProgress variable and all calls
like progress.set_task and progress.finish so no progress updates occur in quiet
mode.
In `@crates/relayburn-sdk/src/ingest/ingest.rs`:
- Around line 347-353: The metadata error handling in the fs::metadata(file)
match is treating all errors as “file missing” and silently dropping others;
change the Err arm to inspect the io::ErrorKind: if e.kind() ==
io::ErrorKind::NotFound then print the message and return
Ok(IngestReport::empty()), otherwise propagate the error (convert/return the
io::Error as the function's error type). Update the match on fs::metadata(file)
(the block using file, IngestReport::empty()) to only suppress NotFound and
return Err for permission/IO errors so legitimate failures are not swallowed.
---
Nitpick comments:
In `@crates/relayburn-cli/tests/smoke.rs`:
- Around line 492-503: The test ingest_one_shot_quiet_keeps_stdout_summary must
also assert that stderr is empty when --quiet is used; update the assert chain
on the burn() invocation to include a stderr assertion (e.g.,
.stderr(predicate::str::is_empty())) so the test fails if one-shot quiet mode
emits breadcrumbs to stderr while still checking stdout contains the ingest
summary.
🪄 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: eac2d015-9a3a-4387-a37c-b9b2993b70f9
📒 Files selected for processing (9)
CHANGELOG.mdREADME.mdcrates/relayburn-cli/src/cli.rscrates/relayburn-cli/src/commands/ingest.rscrates/relayburn-cli/tests/smoke.rscrates/relayburn-sdk/src/ingest.rscrates/relayburn-sdk/src/ingest/ingest.rscrates/relayburn-sdk/src/ingest/orchestration_tests.rscrates/relayburn-sdk/src/lib.rs
Three reviewer-flagged fixes: - `relayburn-cli`: `run_watch` now gates the TaskProgress spinner and all subsequent set_task / suspend / finish_and_clear calls behind Option<TaskProgress>, matching the run_once / run_hook pattern. With `--watch --quiet` the spinner is no longer created at all. - `relayburn-sdk`: `ingest_claude_transcript_path` only suppresses NotFound when stat-ing the transcript; permission / IO errors now propagate to the caller instead of being silently dropped as an empty report. - `crates/relayburn-cli/tests/smoke.rs`: pin the one-shot quiet contract harder — stderr must be empty, not just "stdout still has the summary". - `CHANGELOG.md`: drop the trailing "Closes #375" per the impact-first style guide. https://claude.ai/code/session_011ubB69Zxijqb1BsYVYL9iQ
|
@copilot resolve the merge conflicts in this pull request |
Resolves conflict with #419 (sync ingest verbs). The new fast-path is now synchronous like the rest of the ingest surface; the CLI hook path matches main's non-blocking call sites. Tests converted from #[tokio::test] back to #[test]. https://claude.ai/code/session_011ubB69Zxijqb1BsYVYL9iQ
|
@copilot resolve the merge conflicts in this pull request |
Main released 2.8.5 (#419 + perf/parser refactors) which moved its prior [Unreleased] block into the release section. Reattach this branch's items (the new ingest_claude_transcript_path verb and the CLI hook/--quiet behavior) under [Unreleased] above 2.8.5. https://claude.ai/code/session_011ubB69Zxijqb1BsYVYL9iQ
Summary
Closes #375 — restores the 1.x parity gaps that were tractable in 2.x's file-based ingest model.
ingest_claude_transcript_path(ledger, path, opts)verb that parses one JSONL and persists an EOF cursor so a follow-upingest_allskips it.ingest_claude_sessiondelegates through it.burn ingest --hook claudenow drives the new fast-path on the payload'stranscript_path, bounding per-hook cost to a single JSONL parse. Falls back to a full sweep when the payload omits the path.--quietis no longerrequires = "hook"— accepted in default (one-shot) and--watchmodes too. One-shot mode still writes its final summary on stdout so pipelines stay capturable;--quietonly suppresses the stderr progress spinner / breadcrumbs / gap warnings.--opencode-stream/--opencode-url/--opencode-globalflags. The file-based OpenCode adapter plus the FS-event watch loop is the supported 2.x path; the SDK still shipsOpencodeStreamIngestorfor embedders that want to consume the SSE feed themselves.Issue acceptance criteria
commands/ingestmodule doc.--quietaccepted in every mode.Test plan
cargo build --workspacecargo test --workspace(all crates green: 22 + 59 + 5 + 31 + 660 + 2 + 13 = 792 tests, plus 3 doctests)ingest_claude_transcript_path_writes_eof_cursor_so_followup_skips_file,ingest_claude_transcript_path_missing_file_is_empty_report--quietkeeps stdout summary, hook rejects unknown harness, hook missing-transcript no-op, hook payload validation,--quietsuppresses payload warning, watch+hook mutexcargo run -p relayburn-cli -- ingest --quiet(succeeds, writes summary on stdout, no stderr breadcrumbs)https://claude.ai/code/session_011ubB69Zxijqb1BsYVYL9iQ
Generated by Claude Code