Skip to content

relayburn-cli: burn state subcommands (#248 D4)#313

Merged
willwashburn merged 6 commits intomainfrom
rust-port/cli-state
May 6, 2026
Merged

relayburn-cli: burn state subcommands (#248 D4)#313
willwashburn merged 6 commits intomainfrom
rust-port/cli-state

Conversation

@willwashburn
Copy link
Copy Markdown
Member

Summary

  • Wire burn state as a typed clap subcommand over relayburn-sdk with status (default), rebuild {index,content,archive,all}, prune, and reset verbs. --json flips every read path onto a structured payload.
  • Add state_status to the SDK: free fn + LedgerHandle::state_status() method returning per-table row counts, archive_state metadata, and resolved retention config. Three unit tests cover the fresh / populated / free-fn round-trip paths.
  • Un-ignore the two state-status* golden invocations and reshape the snapshots to the 2.0 SQLite layout.

2.0 vs 1.x — golden snapshot divergence

The TS state-status output references the 1.x JSONL ledger artifacts (ledger.idx, ledger.content.idx, archive.sqlite, content/ sidecars). The 2.0 Rust ledger has two SQLite databases (burn.sqlite + content.sqlite) with no separate index files and the archive_state row collapsed inside burn.sqlite. The Rust state status therefore reports the 2.0-shape view: per-table row counts in the events DB, a row count in the content DB, and the archive_state schema/last-built/last-rebuild fields. File sizes are omitted from both human and JSON output because WAL checkpointing makes them non-deterministic on a logically-empty ledger. This is a deliberate divergence from "byte-equivalent with TS" tracked under #240; sibling D-PRs (D1/D2/D3) will hit the same architectural drift on their read-path snapshots.

Stubs + follow-ups

  • state reset (full wipe + optional --reingest) returns exit 1 with a typed message — the 1.x implementation walked $RELAYBURN_HOME and unlinked individual JSONL files, none of which exist in 2.0. The 2.0 equivalent (drop + recreate the two SQLite DBs, then re-ingest) is filed for follow-up under Epic: rewrite burn in Rust (v2 — supersedes #222) #240. Workaround: burn state rebuild all && burn ingest.
  • state rebuild classify (standalone reclassify pass) returns exit 1 with a typed message — the 2.0 ingest pipeline classifies at append time per [Rust port] relayburn-analyze: hotspots aggregator #274. Filed for follow-up.
  • The four 1.x rebuild subtargets (index / content / archive / all) collapse onto Ledger::rebuild_derivable in 2.0 (single SQL transaction over the derivable tables); they're kept as separate clap subcommands so existing automation doesn't break.

Files touched

  • crates/relayburn-cli/src/cli.rsCommand::State(StateArgs) with nested StateSubcommand enum and per-verb args structs (StateStatusArgs, StateRebuildArgs, StateRebuildTarget, StatePruneArgs, StateResetArgs).
  • crates/relayburn-cli/src/commands/state.rs — real presenter dispatching on the subcommand; human + JSON renderers; format_int matching TS Intl.NumberFormat('en-US'); typed-error mapping that pulls LedgerError out of anyhow::Error so EXIT_LEDGER_ERROR (3) still surfaces.
  • crates/relayburn-cli/src/main.rs — dispatch arm.
  • crates/relayburn-cli/tests/smoke.rsREAL_COMMANDS allowlist for the each_stub_exits_one test.
  • crates/relayburn-sdk/src/query_verbs.rsStateStatus + supporting types, LedgerHandle::state_status method, state_status(opts) free fn, three unit tests.
  • crates/relayburn-sdk/src/ledger.rsRawLedger::read_archive_state_json() helper.
  • tests/fixtures/cli-golden/invocations.json — flip state-status / state-status-json to enabled: true.
  • tests/fixtures/cli-golden/snapshots/state-status*.stdout.txt — regenerated against the 2.0 layout.
  • tests/fixtures/cli-golden/ledger/.gitignore — extended to cover content.sqlite* and -journal so a stray run doesn't dirty the tree.
  • CHANGELOG.md[Unreleased] bullet.

Test plan

  • cargo test --workspace (608+ pass)
  • BURN_GOLDEN=1 cargo test --test golden -p relayburn-clistate-status + state-status-json pass
  • burn state --help / burn state status --help / burn state rebuild --help / burn state prune --help / burn state reset --help mirror the TS flag set
  • burn state status on a fresh $RELAYBURN_HOME produces stable output across consecutive runs (verified — file sizes omitted)

🤖 Generated with Claude Code

Wire `burn state` as a typed clap subcommand over `relayburn-sdk`:
- `state status` (default) reports per-table row counts in `burn.sqlite`,
  the row count in `content.sqlite`, the `archive_state` schema /
  last-built / last-rebuild fields, and the resolved retention config.
  `--json` emits a structured `StateStatus` payload.
- `state rebuild {index,content,archive,all}` drives
  `Ledger::rebuild_derivable`. The four 1.x subtargets collapse onto the
  same SQL transaction in 2.0; standalone `rebuild classify` is stubbed
  with a follow-up note (the Rust ingest pipeline classifies at append
  time per #274).
- `state prune` drives `Ledger::prune_content_older_than`, sourcing the
  retention window from `--days` or the resolved config.
- `state reset` is stubbed (drops + re-ingest is filed for follow-up
  under #240); it returns exit 1 with a typed message pointing at
  `state rebuild all` + `burn ingest` as a workaround.

The TS sibling reports against the 1.x JSONL ledger layout
(ledger.idx, archive.sqlite, content/ sidecars). The 2.0 Rust port has
two SQLite databases with no separate index/archive files, so the
`state-status` golden snapshots are reshaped to match the 2.0 layout —
deliberate divergence tracked under #240. File sizes are omitted from
both human and JSON output because WAL checkpointing makes them
non-deterministic on a logically-empty ledger.

SDK additions:
- `state_status(ledger_home: Option<PathBuf>) -> Result<StateStatus>`
  free fn + `LedgerHandle::state_status()` method, with row-count,
  archive_state, and config blocks. Three new unit tests cover the
  fresh-ledger zero-row path, the appended-turn count path, and the
  free-fn `ledger_home` round-trip.
- `RawLedger::read_archive_state_json()` to surface the single-row
  `archive_state` snapshot without forcing callers to bind to rusqlite.

CLI tests:
- Two `state-status*` golden invocations flipped to `enabled: true`;
  snapshots regenerated.
- Smoke test's `each_stub_exits_one` filters out the now-real `state`
  command via a `REAL_COMMANDS` allowlist.
- Fixture-dir `.gitignore` extended to cover `content.sqlite*` /
  `*-journal` so a stray run doesn't dirty the working tree.

Follow-ups filed for: standalone `state rebuild classify` reclassify
pass, `state reset` (drop + re-ingest), and a dedicated 2.0-format
fixture corpus for the read-path golden snapshots.
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2922e0fb03

ℹ️ 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".

Comment on lines +336 to +337
let cutoff_ms = now_ms.saturating_sub(cutoff_ms);
let cutoff = format!("ts:{:013}.000", cutoff_ms);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Format prune cutoff using writer timestamp shape

run_prune computes cutoff as ts:{:013}.000 from epoch milliseconds, but content.created_at values are written by writer::now_iso as ts:{:020}.{:09} and compared lexically in prune_older_than. With the current format, normal rows like ts:000... are lexicographically less than cutoffs like ts:174..., so burn state prune can delete effectively all content rows regardless of the intended retention window.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 99ecb8f. run_prune now formats the cutoff via a new format_cutoff_ts helper that mirrors writer::now_iso's ts:{:020}.{:09} shape exactly. Added three regression tests under commands::state::tests: cutoff_matches_writer_format_byte_for_byte, cutoff_is_lex_comparable_against_writer_rows, cutoff_padding_widths_are_stable — the last one pins the 33-char total width so a future format tweak can't silently flip the lex ordering again.

Comment on lines +212 to +216
#[arg(long)]
pub full: bool,
/// Reclaim unused SQLite pages after the apply.
#[arg(long)]
pub vacuum: bool,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve archive vacuum positional compatibility

The typed clap shape for state rebuild archive only accepts --vacuum/--full flags, so burn state rebuild archive vacuum is now rejected as an extra argument. The TypeScript command still accepts archive vacuum as a positional action (runArchiveTarget), so this breaks existing scripts/automation that use that invocation style.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 99ecb8f. Added a typed ArchiveAction ValueEnum and an Option<ArchiveAction> positional on StateRebuildArchiveArgs, so burn state rebuild archive vacuum now parses cleanly. Verified burn state rebuild archive bogus still rejects with [possible values: vacuum]. The dispatch is unchanged: both the positional and --vacuum flag route through rebuild_derivable because 2.0 collapses archive.sqlite into burn.sqlite — kept the surface so 1.x automation doesn't break, with a doc comment explaining the no-op equivalence.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 6, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Implements a typed burn state CLI subcommand (status, rebuild, prune, reset) with JSON/text output; wires CLI dispatcher to accept StateArgs; adds SDK state-status APIs and ledger archive-state JSON reader; adds config helpers, updates tests/golden snapshots and .gitignore, and records an Unreleased changelog entry.

Changes

Burn State CLI + SDK

Layer / File(s) Summary
CLI Type Definitions
crates/relayburn-cli/src/cli.rs
Adds StateArgs, StateSubcommand (Status/Rebuild/Prune/Reset), per-subcommand argument structs, and rebuild targets with flags.
CLI Dispatcher
crates/relayburn-cli/src/main.rs
Command::State changed from unit-like to State(StateArgs); dispatcher forwards args to state::run.
Command Handlers / UX
crates/relayburn-cli/src/commands/state.rs
Adds run(globals, args) and handlers: run_status (JSON/text output, path relabeling, pretty formatters), run_rebuild/run_rebuild_derivable, run_prune (retention parsing, prune cutoff, reporting), and stubbed run_reset.
Error Rendering
crates/relayburn-cli/src/render/error.rs
Adds report_advisory(message, globals) to emit non-fatal warnings to stderr.
SDK Query APIs
crates/relayburn-sdk/src/query_verbs.rs, crates/relayburn-sdk/src/ledger.rs
Adds state-status structs (BurnDbRowCounts, BurnDbStatus, ContentDbStatus, ArchiveStateStatus, StateConfigSummary, StateStatus, StateStatusOptions), LedgerHandle::state_status(), free-function state_status(opts), and Ledger::read_archive_state_json().
Config helpers
crates/relayburn-sdk/src/ledger/config.rs, crates/relayburn-sdk/src/lib.rs
Adds config_path_at_home(home: &Path) -> PathBuf and load_config_with_home(home: Option<&Path>) -> Result<BurnConfig> and re-exports them from the SDK crate root.
Tests & Fixtures
crates/relayburn-cli/tests/smoke.rs, tests/fixtures/cli-golden/*
Enables state-status and state-status-json golden invocations, updates text/JSON snapshots, updates smoke test guards for implemented commands, and adds SQLite artifacts to ledger .gitignore.
Changelog
CHANGELOG.md
Adds Unreleased entry describing wiring of burn state subcommands and JSON/text behavior.

Sequence Diagram

sequenceDiagram
    participant User
    participant CLI as relayburn-cli
    participant SDK as relayburn-sdk
    participant DB as LedgerDB

    User->>CLI: burn state status
    CLI->>SDK: state_status(opts)
    SDK->>DB: query burn.sqlite (row counts, turns, etc.)
    DB-->>SDK: burn results
    SDK->>DB: query content.sqlite (row count)
    DB-->>SDK: content results
    SDK->>DB: query archive_state row
    DB-->>SDK: archive_state JSON
    SDK->>SDK: assemble StateStatus (burn, content, archive, config)
    SDK-->>CLI: StateStatus
    CLI->>CLI: format_status() (JSON or human)
    CLI-->>User: formatted output
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related issues

Possibly related PRs

Poem

🐰
I hopped into the code at dawn's first light,
Wired state to whisper what ledgers write.
JSON and text, prune crumbs and rebuild logs,
A stub here and there, yet progress for the frogs.
Hop on, little CLI—let state show its sight.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR implementation does not address the objectives from linked issue #39 (Plan-based monthly quota tracking). The changeset implements state subcommands but contains no plan-budget tracking, monthly reset cycles, or plan configuration features. Either remove issue #39 from linked issues, file it as separate work, or implement plan-based monthly quota tracking features (plans list/add/remove, monthly budget projections, custom reset cycles) alongside this PR.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'relayburn-cli: burn state subcommands (#248 D4)' clearly summarizes the main change: adding burn state subcommands to the CLI, which is the primary focus of the changeset.
Description check ✅ Passed The description comprehensively documents the PR changes including subcommand wiring, SDK state_status implementation, golden snapshot updates, and follow-up stubs, all directly related to the changeset.
Out of Scope Changes check ✅ Passed All code changes directly support the PR objective: state subcommand wiring, SDK query_verbs for state_status, golden test updates, and config-home plumbing. No unrelated work is introduced.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch rust-port/cli-state

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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (2)
crates/relayburn-cli/src/commands/state.rs (1)

389-391: 💤 Low value

Negative --days silently converts to "forever" — consider returning an error instead.

A user passing --days -5 likely made an input mistake, but this code silently treats it as "forever" rather than surfacing an error. This could mask user errors and lead to unexpected data retention behavior.

🔧 Suggested fix: return None to trigger the existing error path
     if !n.is_finite() {
         return None;
     }
     if n < 0.0 {
-        return Some(relayburn_sdk::Retention::Forever);
+        return None; // Negative values are invalid; let caller report the error
     }
     Some(relayburn_sdk::Retention::Days(n))
🤖 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/state.rs` around lines 389 - 391, The CLI
currently treats a negative --days value as "forever" instead of surfacing an
error; update the code that parses the --days flag (the branch that converts
negative days into the "forever" sentinel) to return None (or Err) when days < 0
so the existing error path is triggered and an error is returned to the user;
specifically modify the parsing logic that handles the --days flag/variable to
validate days >= 0 and bail out on negative input rather than silently mapping
it to forever.
crates/relayburn-cli/src/cli.rs (1)

226-231: ⚡ Quick win

Parse --days into a typed value instead of String.

Option<String> accepts arbitrary tokens even though this flag only allows a day count or forever. Moving that validation into clap would keep the new state surface fully typed and fail fast on bad input.

Possible direction
+pub enum StatePruneDays {
+    Forever,
+    Days(u32),
+}
+
 #[derive(Debug, Clone, ClapArgs, Default)]
 pub struct StatePruneArgs {
     /// Override the configured retention window. Accepts a number
     /// (days) or the literal `forever`.
-    #[arg(long)]
-    pub days: Option<String>,
+    #[arg(long, value_parser = parse_state_prune_days)]
+    pub days: Option<StatePruneDays>,
     /// Delete sidecars even when the source session file still exists.
     #[arg(long)]
     pub force: bool,
 }
fn parse_state_prune_days(raw: &str) -> Result<StatePruneDays, String> {
    if raw.eq_ignore_ascii_case("forever") {
        return Ok(StatePruneDays::Forever);
    }

    raw.parse::<u32>()
        .map(StatePruneDays::Days)
        .map_err(|_| "expected a non-negative integer day count or `forever`".to_string())
}
🤖 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/cli.rs` around lines 226 - 231, The --days flag is
currently parsed into Option<String>, allowing invalid tokens; add a typed
parser and wire it into clap so the flag yields Option<StatePruneDays> instead.
Implement a parser function (e.g. parse_state_prune_days) that maps "forever"
(case-insensitive) to StatePruneDays::Forever and parses a non-negative integer
into StatePruneDays::Days(u32) with a clear error message, then use clap's
.value_parser / .value_parser_fn or .value_hint to attach that parser to the
--days argument and change the CLI field type from Option<String> to
Option<StatePruneDays> so validation is done by clap and the rest of the code
receives a strongly typed value.
🤖 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-cli/src/cli.rs`:
- Around line 242-245: The --reingest flag is currently allowed without --force;
update the clap arg definition for the "reingest" option (the argument that
defines "--reingest" in crates/relayburn-cli/src/cli.rs) to require the "force"
argument by adding the clap requires dependency (e.g., using .requires("force")
on the builder or requires = "force" in the derive attribute) so the parser
enforces that --reingest can only be used together with --force.

---

Nitpick comments:
In `@crates/relayburn-cli/src/cli.rs`:
- Around line 226-231: The --days flag is currently parsed into Option<String>,
allowing invalid tokens; add a typed parser and wire it into clap so the flag
yields Option<StatePruneDays> instead. Implement a parser function (e.g.
parse_state_prune_days) that maps "forever" (case-insensitive) to
StatePruneDays::Forever and parses a non-negative integer into
StatePruneDays::Days(u32) with a clear error message, then use clap's
.value_parser / .value_parser_fn or .value_hint to attach that parser to the
--days argument and change the CLI field type from Option<String> to
Option<StatePruneDays> so validation is done by clap and the rest of the code
receives a strongly typed value.

In `@crates/relayburn-cli/src/commands/state.rs`:
- Around line 389-391: The CLI currently treats a negative --days value as
"forever" instead of surfacing an error; update the code that parses the --days
flag (the branch that converts negative days into the "forever" sentinel) to
return None (or Err) when days < 0 so the existing error path is triggered and
an error is returned to the user; specifically modify the parsing logic that
handles the --days flag/variable to validate days >= 0 and bail out on negative
input rather than silently mapping it to forever.
🪄 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: 11812568-fa57-4cd1-8f6c-2e0ec3fedb0e

📥 Commits

Reviewing files that changed from the base of the PR and between 54f0682 and 2922e0f.

📒 Files selected for processing (11)
  • CHANGELOG.md
  • crates/relayburn-cli/src/cli.rs
  • crates/relayburn-cli/src/commands/state.rs
  • crates/relayburn-cli/src/main.rs
  • crates/relayburn-cli/tests/smoke.rs
  • crates/relayburn-sdk/src/ledger.rs
  • crates/relayburn-sdk/src/query_verbs.rs
  • tests/fixtures/cli-golden/invocations.json
  • tests/fixtures/cli-golden/ledger/.gitignore
  • tests/fixtures/cli-golden/snapshots/state-status-json.stdout.txt
  • tests/fixtures/cli-golden/snapshots/state-status.stdout.txt

Comment thread crates/relayburn-cli/src/cli.rs
- Codex P1 (state.rs): prune cutoff was formatted as `ts:{:013}.000` from
  epoch ms but content rows are stamped by `writer::now_iso` as
  `ts:{:020}.{:09}` and compared lexically — the width mismatch made
  every stamped row sort lex-greater than the cutoff, so prune was
  effectively a no-op (or, after a rollover, a full wipe). Move the
  formatting into a `format_cutoff_ts` helper that mirrors the writer's
  shape exactly, with three regression tests covering byte-equivalence,
  lex ordering against writer-stamped rows, and constant-width invariant.

- Codex P2 (cli.rs): accept `burn state rebuild archive vacuum` (legacy
  TS positional shape) by adding a typed `ArchiveAction` ValueEnum.
  Today both the positional and `--vacuum` flag route through the same
  `rebuild_derivable` path in 2.0 (no separate archive.sqlite to vacuum),
  but existing scripts that target the 1.x surface no longer error out
  on the positional.

- CodeRabbit minor (cli.rs): `--reingest` now `requires = "force"` so
  the parser rejects bare `--reingest` instead of silently no-opping.
Folds D1+D2+D3 (PRs #315/#312/#314) into the D4 state branch.

Resolutions:
- CHANGELOG.md: union all four bullets, alphabetized.
- crates/relayburn-cli/src/cli.rs: keep all five typed Command variants
  (Summary/Hotspots/Overhead/Compare/State); merge ValueEnum import; keep
  D4's state-related types (StateArgs et al.) plus D2/D3's OverheadArgs /
  CompareArgs / OverheadKind.
- crates/relayburn-cli/src/main.rs: auto-merged — five typed dispatch arms.
- crates/relayburn-cli/tests/smoke.rs: drop REAL_COMMANDS allow-list and
  early-continue; remove "state" from UNIMPLEMENTED_SUBCOMMANDS, leaving
  ["run", "ingest", "mcp-server"].
- tests/fixtures/cli-golden/ledger/.gitignore: superset of patterns.
- Cargo.lock: regenerated from origin/main.
- tests/fixtures/cli-golden/snapshots/state-status*.stdout.txt: keep D4's
  2.0 SQLite layout (deliberate divergence) but refresh row counts to
  reflect the bootstrapped fixture (golden.rs on main now replays
  ledger.jsonl into burn.sqlite before running, so the fixture has 18
  rows instead of 0).
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 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-cli/src/cli.rs`:
- Around line 337-349: The handler in crates/relayburn-cli/src/commands/state.rs
currently ignores the flags in StateRebuildArchiveArgs (action, full, vacuum)
and StateRebuildAllArgs (force) and always calls run_rebuild_derivable(globals);
update the handler to inspect these fields and either (A) pass them through to
the underlying rebuild routine (extend run_rebuild_derivable or its caller to
accept an options struct with action/full/vacuum/force and apply the requested
behavior), or (B) return a clear, typed "not supported" error when any of those
flags are set so callers don't get a silent no-op; reference
StateRebuildArchiveArgs, StateRebuildAllArgs and run_rebuild_derivable to locate
where to validate and propagate or reject the flags.

In `@crates/relayburn-sdk/src/query_verbs.rs`:
- Around line 1356-1362: The computed total_rows (variable total_rows) only sums
tracked event fields on the rows struct and omits the separate archive_state row
fetched from burn.sqlite, so update the calculation to include the archive_state
count (e.g., add rows.archive_state or the archive_state value fetched later) or
change the variable/label (total_rows -> tracked_rows or similar) and adjust any
human/JSON output code that prints total_rows to avoid claiming a full DB total;
ensure you update both the numeric sum and the displayed key names for
consistency.
🪄 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: 79b89a0c-6861-4533-a3ea-873423e585bf

📥 Commits

Reviewing files that changed from the base of the PR and between 99ecb8f and b0ce9b5.

📒 Files selected for processing (9)
  • CHANGELOG.md
  • crates/relayburn-cli/src/cli.rs
  • crates/relayburn-cli/src/main.rs
  • crates/relayburn-cli/tests/smoke.rs
  • crates/relayburn-sdk/src/query_verbs.rs
  • tests/fixtures/cli-golden/invocations.json
  • tests/fixtures/cli-golden/ledger/.gitignore
  • tests/fixtures/cli-golden/snapshots/state-status-json.stdout.txt
  • tests/fixtures/cli-golden/snapshots/state-status.stdout.txt
🚧 Files skipped from review as they are similar to previous changes (1)
  • tests/fixtures/cli-golden/invocations.json

Comment thread crates/relayburn-cli/src/cli.rs
Comment thread crates/relayburn-sdk/src/query_verbs.rs Outdated
Comment thread crates/relayburn-sdk/src/query_verbs.rs Outdated
- state rebuild archive --full / --vacuum (and the legacy `vacuum`
  positional) and state rebuild all --force now print stderr
  breadcrumbs explaining the no-op in the 2.0 layout, instead of
  silently discarding the flag. Adds a small `report_advisory` helper
  in render/error.rs; stderr in both human and --json modes so JSON
  stdout stays single-shape.

- StateStatus.burn renames `total_rows` (`totalRows` JSON) to
  `tracked_rows` (`trackedRows`) — the field sums per-table event
  rows and excludes the `archive_state` metadata row, so the name
  now reflects the scope. Human format updates from "rows: N total"
  to "tracked rows: N". Snapshots refreshed.

- state_status now plumbs the active ledger home into config
  loading via a new `load_config_with_home` helper in
  ledger/config.rs. `--ledger-path foo state status` previously
  reported the env-default home's retention/store settings; now it
  reads `foo/config.json`. resolve_config_summary returns Result so
  IO errors propagate through the typed reporter instead of being
  unwrapped to defaults.
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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-cli/src/commands/state.rs`:
- Around line 307-331: The config is being loaded with load_config() which uses
the env home and mismatches the ledger path passed via globals.ledger_path;
update run_prune to load the config using the explicit ledger/home from
GlobalArgs instead of load_config() so the same path is used for retention logic
and database opening (mirror the fix applied in state_status). Locate
load_config call in run_prune and replace it with the equivalent config-loading
function that accepts the home/ledger path (e.g. a load_config_with_home or
load_config_from_path variant) or otherwise pass globals.ledger_path into
load_config so retention_days comes from the same ledger path used later. Ensure
any error handling and report_ledger_error usage remains unchanged.
🪄 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: bbedfb70-265f-4330-a79f-b94319e621f6

📥 Commits

Reviewing files that changed from the base of the PR and between b0ce9b5 and 344e899.

📒 Files selected for processing (7)
  • crates/relayburn-cli/src/commands/state.rs
  • crates/relayburn-cli/src/render/error.rs
  • crates/relayburn-sdk/src/ledger.rs
  • crates/relayburn-sdk/src/ledger/config.rs
  • crates/relayburn-sdk/src/query_verbs.rs
  • tests/fixtures/cli-golden/snapshots/state-status-json.stdout.txt
  • tests/fixtures/cli-golden/snapshots/state-status.stdout.txt
🚧 Files skipped from review as they are similar to previous changes (1)
  • crates/relayburn-sdk/src/ledger.rs

Comment thread crates/relayburn-cli/src/commands/state.rs
Follow-up to 344e899. The prior fixup wired `state_status` to load
config from the ledger's active home, but `run_prune` was missed —
it was still calling bare `load_config()`, so `burn --ledger-path
/foo state prune` would read retention_days from `$RELAYBURN_HOME`'s
config.json against `/foo`'s content DB.

Mirror the state_status fix: thread `globals.ledger_path` into
`load_config_with_home`. Re-export `load_config_with_home` and
`config_path_at_home` from `relayburn-sdk::lib` so external callers
(CLI today; future embedders) have access via the canonical surface
rather than reaching into the `ledger` module.
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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-cli/src/commands/state.rs`:
- Around line 145-150: The rel_to_home function currently rewrites any
byte-prefix match of home (path.starts_with(home)), which can mislabel paths
like "/x/home2/..." when home="/x/home"; fix by normalizing/trimming trailing
slashes from home and only treating path as inside home when either path == home
or path starts with home + "/" (i.e., check path == home ||
path.starts_with(&(home.to_owned() + "/"))), then compute the rest as before;
reference: rel_to_home, path, home.
🪄 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: 70f27b7d-f372-4290-8bf7-dbf2fd1af8e0

📥 Commits

Reviewing files that changed from the base of the PR and between 344e899 and 85f314a.

📒 Files selected for processing (2)
  • crates/relayburn-cli/src/commands/state.rs
  • crates/relayburn-sdk/src/lib.rs

Comment thread crates/relayburn-cli/src/commands/state.rs Outdated
`rel_to_home` operates on `&str`, so the prior `path.starts_with(home)`
was a byte-prefix match — `/x/home2/foo` against `home="/x/home"` would
get rewritten to `${RELAYBURN_HOME}/2/foo`. Switch to a separator-
bounded check via `strip_prefix` + `'/'` peek; normalize a trailing
slash on `home` so `home="/x/home"` and `home="/x/home/"` behave the
same; bail to a passthrough for degenerate inputs (`home=""`,
`home="/"`).

Five new unit tests cover the fix path, the byte-prefix collision the
guard prevents, the trailing-slash normalization, the degenerate-home
inputs, and the `path == home` boundary case (preserves the prior
trailing-slash output shape so downstream callers don't see a
behavioral shift).
@willwashburn willwashburn merged commit 067c096 into main May 6, 2026
8 checks passed
@willwashburn willwashburn deleted the rust-port/cli-state branch May 6, 2026 23:29
willwashburn added a commit that referenced this pull request May 7, 2026
…et hook scope)

- Merge origin/main (D6 codex landed); CHANGELOG additive, registry/mod
  auto-merged with codex slot from main.
- Codex P2: route burn ingest one-shot summary to stdout (matches TS
  runIngestOnce at packages/cli/src/commands/ingest.ts:121-126);
  --watch banner + --hook breadcrumbs stay on stderr.
- CodeRabbit Minor: scope --quiet to --hook via clap requires = "hook"
  (mirrors the --reingest requires = "force" pattern from #313).
  Standalone --quiet and --watch --quiet are now rejected at parse time.
willwashburn added a commit that referenced this pull request May 7, 2026
…319)

* relayburn-cli: burn ingest + burn mcp-server (#248 D8, closes #210)

Wire `burn ingest` as a thin presenter over `relayburn_sdk::ingest_all`
plus the SDK's `start_watch_loop` controller. Three modes mirror the TS
sibling at `packages/cli/src/commands/ingest.ts`:

- No flags: one full sweep, then exit. Logs
  `[burn] ingest: ingested N session(s) (+M turn(s))` on stderr.
- `--watch [--interval MS]`: foreground poll loop. Skips empty-tick
  log lines (TS shape); SIGINT / SIGTERM stop drains in-flight ticks
  before returning.
- `--hook claude [--quiet]`: stdin-driven Claude hook entrypoint.
  Validates the `session_id` / `transcript_path` payload shape, then
  drives a full sweep (cursor short-circuit means cost is bounded by
  new turns, not the hook payload). Failure paths log + exit 0 to
  avoid blocking the parent Claude tool call.

Wire `burn mcp-server` as a hand-rolled JSON-RPC 2.0 stdio server,
mirroring `packages/mcp/src/server.ts` rather than depending on
`rmcp`. The on-wire surface (`initialize`, `ping`, `tools/list`,
`tools/call`) is small enough that a tight in-tree implementation
beats freezing a heavy SDK version. Single tool ships in this PR:
`burn__sessionCost`, a 1:1 port of `packages/mcp/src/tools/session-cost.ts`
delegating to `LedgerHandle::session_cost`. Future tools (summary,
hotspots, …) land separately. Closes #210.

Smoke test drops `ingest` and `mcp-server` from
`UNIMPLEMENTED_SUBCOMMANDS`; `--help` for both subcommands still passes
the existing help-shape assertion. Manual stdio handshake verified:
`initialize` echoes the client's `protocolVersion`, `tools/list`
returns `burn__sessionCost`, and `tools/call burn__sessionCost`
returns the SDK payload as both stringified `text` content and a
`structuredContent` mirror.

* relayburn-cli: address PR #319 review (rebase + stdout report + --quiet hook scope)

- Merge origin/main (D6 codex landed); CHANGELOG additive, registry/mod
  auto-merged with codex slot from main.
- Codex P2: route burn ingest one-shot summary to stdout (matches TS
  runIngestOnce at packages/cli/src/commands/ingest.ts:121-126);
  --watch banner + --hook breadcrumbs stay on stderr.
- CodeRabbit Minor: scope --quiet to --hook via clap requires = "hook"
  (mirrors the --reingest requires = "force" pattern from #313).
  Standalone --quiet and --watch --quiet are now rejected at parse time.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant