relayburn-sdk: integration test + make workspace publish-ready (closes #246)#304
Merged
willwashburn merged 1 commit intomainfrom May 5, 2026
Merged
Conversation
Closes the last acceptance gates on #246: * Integration test (`crates/relayburn-sdk/tests/integration.rs`): opens a fixture ledger, seeds one turn / one content blob with a unique FTS token / one stamp, then exercises every one of the nine verbs in both forms (LedgerHandle method + free function). Asserts structural correctness (turn count, FTS hit, exported `kind=turn` / `kind=stamp` rows, ingest scan-zero report). * `cargo publish --workspace --dry-run --allow-dirty` now succeeds end to end. To get there: - Drop `publish = false` from relayburn-{reader,ledger,analyze,ingest}; the lower crates publish in lockstep with relayburn-sdk per the #240 architecture (the TS sibling does the same with the eight @relayburn npm packages). - Add `version = "0.0.0"` alongside every internal `path` dep so cargo accepts the manifests for upload (path-only deps fail "all dependencies must have a version requirement"). - Vendor `models.dev.json` into `crates/relayburn-analyze/data/` and point `include_str!` at the in-crate copy. Without this, the verify step of `cargo package` couldn't see the file (the TS-tree path is outside the crate's tarball). The TS copy stays in place; the `pricing:update` script now writes both copies in lockstep so they can't drift. Per-crate `cargo publish --dry-run -p relayburn-sdk` requires the lower crates to be on crates.io first (cargo resolves via the registry once the path is dropped from the packaged manifest); the workspace flag is the right gate for local verification and that's now green. Closes #246. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR prepares relayburn crates for publication by removing ChangesSDK Publishing & Integration Validation
Sequence DiagramsequenceDiagram
participant Test as Integration Test
participant SDK as SDK Handle
participant Ledger as Ledger
participant Analysis as Analysis Engine
participant Export as Export
Test->>Ledger: Populate fixture with Turn, Content, Stamp
Test->>SDK: Create handle with fixture home
Test->>SDK: summary()
SDK->>Analysis: Analyze ledger state
Analysis-->>SDK: Summary stats
SDK-->>Test: Results with counts
Test->>SDK: session_cost()
SDK->>Ledger: Query session data
Ledger-->>SDK: Session records
SDK-->>Test: Cost metrics
Test->>SDK: overhead/hotspots/search()
SDK->>Analysis: Process queries
Analysis-->>SDK: Analysis results
SDK-->>Test: Row data with kinds
Test->>SDK: export_ledger/export_stamps()
SDK->>Export: Serialize data
Export-->>SDK: Exported records
SDK-->>Test: Exported content
Test->>SDK: ingest()
SDK->>Ledger: Append new data
Ledger-->>SDK: Ingest report
SDK-->>Test: Report with scanned/appended counts
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 2a3e2f682b
ℹ️ 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
+15
to
+18
| relayburn-reader = { path = "../relayburn-reader", version = "0.0.0" } | ||
| relayburn-ledger = { path = "../relayburn-ledger", version = "0.0.0" } | ||
| relayburn-analyze = { path = "../relayburn-analyze", version = "0.0.0" } | ||
| relayburn-ingest = { path = "../relayburn-ingest", version = "0.0.0" } |
There was a problem hiding this comment.
These dependency constraints will break as soon as the workspace version is bumped above 0.0.x: Cargo treats "0.0.0" as a caret requirement (>=0.0.0, <0.0.1) and also validates the local path crate version against it, so a normal release bump (for example to 0.1.0 or 1.x) will make this manifest unsatisfiable and block cargo build/cargo publish. This makes the workspace not actually “publish-ready” beyond the initial 0.0.0 version unless every dependency spec is manually rewritten each release.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
crates/relayburn-sdk/tests/integration.rs (2)
256-259: ⚡ Quick winRestore
RELAYBURN_HOMEafter the test.This mutates process-global state and never puts the previous value back. That can leak into later tests in the same binary and make them order-dependent. Snapshot the old value and restore it with a drop guard around the ingest assertions.
🤖 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/tests/integration.rs` around lines 256 - 259, The test sets RELAYBURN_HOME via std::env::set_var("RELAYBURN_HOME", home.path()) but never restores the previous value; wrap the change so you snapshot the prior value using std::env::var_os before calling set_var and restore it after the ingest_all assertions (or use a RAII/drop guard) to reset RELAYBURN_HOME; ensure the guard covers the calls that invoke cleanup_stale_pending_stamps, load_config and ingest_all so process-global state is always restored even on test failure.
154-244: ⚡ Quick winAssert the free-function results instead of discarding them.
Line 154, Line 171, Line 188, Line 222, and Line 240 only prove that the free wrapper returns
Ok(_). A wrapper bug that opens the wrong ledger or maps the payload incorrectly would still pass here. Since this test is meant to validate both call surfaces, compare those free-function results to the handle-path results foroverhead,overhead_trim,hotspots,export_ledger, andexport_stamps.🤖 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/tests/integration.rs` around lines 154 - 244, The free-function results for overhead, overhead_trim, hotspots, export_ledger, and export_stamps are only checked for Ok(_) and discarded (variables _oh2, _trim2, _h2, _ledger_rows2, _stamp_rows2); bind those free-function returns to meaningful names and assert they match the corresponding handle-path results (e.g., compare overhead summaries/recommendations from overhead and _oh2, compare trim.recommendations and _trim2, ensure hotspots discriminated shape/refused flag matches h and _h2, compare exported ledger rows/JSONL contents between ledger_rows and _ledger_rows2 including presence of kind="turn" and SESSION_ID where applicable, and compare stamp_rows and _stamp_rows2 including kind="stamp"); update assertions to check equality or equivalent content/length checks rather than just Ok.
🤖 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.
Nitpick comments:
In `@crates/relayburn-sdk/tests/integration.rs`:
- Around line 256-259: The test sets RELAYBURN_HOME via
std::env::set_var("RELAYBURN_HOME", home.path()) but never restores the previous
value; wrap the change so you snapshot the prior value using std::env::var_os
before calling set_var and restore it after the ingest_all assertions (or use a
RAII/drop guard) to reset RELAYBURN_HOME; ensure the guard covers the calls that
invoke cleanup_stale_pending_stamps, load_config and ingest_all so
process-global state is always restored even on test failure.
- Around line 154-244: The free-function results for overhead, overhead_trim,
hotspots, export_ledger, and export_stamps are only checked for Ok(_) and
discarded (variables _oh2, _trim2, _h2, _ledger_rows2, _stamp_rows2); bind those
free-function returns to meaningful names and assert they match the
corresponding handle-path results (e.g., compare overhead
summaries/recommendations from overhead and _oh2, compare trim.recommendations
and _trim2, ensure hotspots discriminated shape/refused flag matches h and _h2,
compare exported ledger rows/JSONL contents between ledger_rows and
_ledger_rows2 including presence of kind="turn" and SESSION_ID where applicable,
and compare stamp_rows and _stamp_rows2 including kind="stamp"); update
assertions to check equality or equivalent content/length checks rather than
just Ok.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 4d23c869-29a6-4f67-8be8-ba1c6d8bafc5
📒 Files selected for processing (9)
crates/relayburn-analyze/Cargo.tomlcrates/relayburn-analyze/data/models.dev.jsoncrates/relayburn-analyze/src/pricing.rscrates/relayburn-ingest/Cargo.tomlcrates/relayburn-ledger/Cargo.tomlcrates/relayburn-reader/Cargo.tomlcrates/relayburn-sdk/Cargo.tomlcrates/relayburn-sdk/tests/integration.rsscripts/update-pricing.mjs
💤 Files with no reviewable changes (1)
- crates/relayburn-reader/Cargo.toml
7 tasks
willwashburn
added a commit
that referenced
this pull request
May 6, 2026
Absorbs `relayburn-{reader,ledger,analyze,ingest}` into `relayburn-sdk`
as `src/{reader,ledger,analyze,ingest}/` modules. The Rust workspace is
now three crates (`relayburn-sdk`, `relayburn-cli`, `relayburn-sdk-node`)
instead of seven. Only the SDK and CLI are published to crates.io; the
four absorbed modules become internal implementation details rather than
a public crates.io contract.
Why: keeping the lower crates separate forced a lockstep-publish
arrangement (the prep for which landed in #304) for what is really a
single implementation. The CLI's `relayburn-sdk` dep now exercises the
SDK's public surface as an external embedder would, which is the
property we actually wanted from the split.
Mechanics:
- `git mv` each lower crate's `src/` into the corresponding SDK
submodule, with each crate's old `lib.rs` becoming the module root.
- Cross-crate imports rewritten in two passes: first scope-rename
intra-module sibling refs (`crate::config::` → `crate::ledger::config::`),
then `relayburn_X::` → `crate::X::`.
- `models.dev.json` moves to `crates/relayburn-sdk/data/`; the
`include_str!` path and `scripts/update-pricing.mjs` follow.
- Ingest's four `tests/*.rs` files become in-crate `#[cfg(test)] mod`s
(they exercise crate-private items that the SDK doesn't re-export).
Their per-binary `static ENV_LOCK` / `GAP_LOCK` mutexes are
consolidated into shared `TEST_ENV_LOCK` / `TEST_GAP_LOCK` in
`crate::ingest`; without that, bundling them into one binary races
`$RELAYBURN_HOME` and gap-state mutations across modules.
- `relayburn-cli` and `relayburn-sdk-node` `Cargo.toml`s now depend
only on `relayburn-sdk`. The CLI's version requirement is `0.0`
(= `>=0.0.0, <0.1.0`) so it satisfies both the local 0.0.0 path dep
and the published `relayburn-sdk` 0.0.1 on crates.io.
- `AGENTS.md` (`CLAUDE.md`) Layout / When-in-doubt sections updated to
describe the three-crate shape and module-path pointers.
Verification: `cargo build --workspace` and `cargo test --workspace`
green (605 unit + 2 integration + 3 doctests, including all four
absorbed ingest test files); `cargo doc --no-deps -p relayburn-sdk`,
`cargo publish --dry-run -p relayburn-sdk --allow-dirty`, and
`cargo publish --dry-run -p relayburn-cli --allow-dirty` all succeed.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Final PR in the #246 series. Closes the remaining acceptance gates after #299/#300/#301/#302/#303 landed the scaffold, module split, and the nine verbs.
What changed
Integration test — `crates/relayburn-sdk/tests/integration.rs`. Opens a fixture ledger in a `TempDir`, seeds one turn / one content blob (with a unique FTS token) / one stamp, and exercises every one of the nine verbs in both forms (`LedgerHandle` method + free function):
Assertions are structural — turn count, FTS hit, exported `kind=turn` / `kind=stamp` rows in the JSONL streams, ingest scan-zero report. Goal is "the wrappers plumb through correctly."
Workspace publish-ready. `cargo publish --workspace --dry-run --allow-dirty` now succeeds end to end. To get there:
Per-crate vs workspace dry-run
`cargo publish --dry-run -p relayburn-sdk` (the literal acceptance criterion) requires the lower crates to already be on crates.io — once the path is dropped during packaging, cargo resolves `relayburn-analyze` etc. from the registry, which won't have them until the first publish. `cargo publish --workspace --dry-run` is the equivalent gate for local verification (uses a temp registry to chain the dependency order in one shot), and that's the gate this PR makes green.
Acceptance for #246
Closes #246.
Test plan
🤖 Generated with Claude Code