refactor(sdk): drop workspace-wide dead_code/unused_imports allows#429
Merged
Conversation
…ed-dead items Remove `#![allow(dead_code, unused_imports)]` from the four absorbed module roots (analyze, ingest, ledger, reader) and the per-mod `#[allow(unused_imports)]` on the lib.rs verb mods. Address the confirmed-dead candidates from issue #336: - Delete `provider_for_turn` / `resolve_turn_provider` aliases (`provider_for` is the canonical entry point and the aliases had no callers). - Inline the `severity_from_usd_pub` visibility-laundering wrapper — `severity_from_usd` is now `pub(crate)` and `tool_call_patterns` calls it directly. - Delete `Fidelity::classify`; its only callers were the file's own tests, which now use `classify_fidelity` directly. Follow-up fall-out triage from the blanket-allow removal is staged in a subsequent commit. https://claude.ai/code/session_01WavFVyZAAyiSmWaktaXsmD
After dropping `#![allow(dead_code, unused_imports)]` from the four
absorbed module roots, ~130 warnings surfaced. Resolve them by trimming
inner-module-root re-export chains down to what `lib.rs` actually
publishes, deleting items confirmed dead across SDK + CLI + node, and
gating test-only helpers with `#[cfg(test)]`. No change to the SDK's
external surface in `lib.rs`.
- Trimmed `pub use` lists in `analyze.rs`, `ingest.rs`, `ledger.rs`,
`reader.rs` to only the items re-exported by `lib.rs` or consumed by a
sibling submodule.
- Deleted dead helpers: `provider_for_model`, `flatten_value`,
`stringify_measured_content`, `merge_content_by_order`,
`ghost_findings_to_waste_findings`, `DetectGhostSurfaceOptions`,
`OpencodeStreamCursor`, `resolve_pending_stamps_for_session` wrapper,
`FIRST_PARTY_TABLES`, the unused `tokenizer` field on
`ParseOptions`/`ParseIncrementalOptions`, and `Cursors::{get,remove}`.
- Removed `reader/opencode_stream.rs` (1.6k lines, unused) and the bulk
of `ingest/reingest.rs`; retained `derive_codex_session_id` because
`ingest/ingest.rs` consumes it.
- Gated test-only items behind `#[cfg(test)]`: `attribute_claude_md`
scaffolding, `extend_default_rules`, `infer_outcome`,
`compute_one_shot_rate`, gap-warning reset helpers,
`ingest_claude_projects`, `LedgerLayout::under`, `turn_id_fingerprint`,
`parse_codex_session`, `parse_opencode_session`, etc.
- Swapped a handful of test imports onto direct module paths so the
facade `pub use` lines could go.
`cargo build --workspace` is warning-free; `cargo test --workspace`
passes (10 binaries, 775 tests).
https://claude.ai/code/session_01WavFVyZAAyiSmWaktaXsmD
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR implements systematic API hygiene cleanup across the relayburn-sdk by removing blanket compiler lint suppressions, narrowing public re-export surfaces, gating test-only code, and removing dead functions per issue ChangesRust Hygiene: Remove Lint Suppressions and Narrow Public APIs
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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 (1)
crates/relayburn-sdk/src/analyze/provider_reattribution.rs (1)
133-140:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
extend_default_rulesis#[cfg(test)]but documented as a public extension helper
crates/relayburn-sdk/src/analyze/provider_reattribution.rsmodule docs (lines 13-15) presentextend_default_rulesas the way to extend the default provider rules, but the function itself is gated behind#[cfg(test)](lines ~24-32 / decl ~134). In non-test builds the helper isn’t available, so downstream callers following the docs can’t compile (and the rustdoc reference is effectively invalid).Make
extend_default_rulesavailable in normal builds (remove/adjust#[cfg(test)]), or update the docs/examples to use an always-available alternative (e.g.,default_rules().to_vec()+extend(...)) instead.🤖 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/analyze/provider_reattribution.rs` around lines 133 - 140, The doc examples advertise extend_default_rules but the function is currently gated with #[cfg(test)] so it’s unavailable in non-test builds; either remove the #[cfg(test)] from extend_default_rules in provider_reattribution.rs so the helper is public for normal builds (keeping its signature: pub fn extend_default_rules<I>(extra: I) -> Vec<ProviderRule> where I: IntoIterator<Item = ProviderRule>), or update the module docs to show the always-available alternative using default_rules().to_vec() and Vec::extend instead; pick one fix and make the code/docs consistent (references: extend_default_rules, default_rules, ProviderRule in provider_reattribution.rs).
🤖 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/analyze/claude_md.rs`:
- Around line 310-313: The doc comment on attribute_claude_md_refs links to the
test-only function attribute_claude_md, which will be absent in non-test builds
and breaks rustdoc links; update the docs for attribute_claude_md_refs to remove
the intra-doc link and refer to "attribute_claude_md" as plain text (or wrap
that sentence in a cfg(test)/doc-specific cfg_attr so the link is emitted only
when attribute_claude_md exists). Locate the `attribute_claude_md_refs` doc
string and either replace `[`attribute_claude_md`]` with unlinked plain text or
conditionally emit the sentence using cfg_attr(docsrs, cfg(test)) as
appropriate.
In `@crates/relayburn-sdk/src/reader/claude.rs`:
- Around line 49-53: The evidence field is incorrectly hidden behind
#[cfg(test)], breaking downstream APIs like parse_claude_session and
reconcile_claude_session_relationships that expect ClaudeRelationshipEvidence;
remove the #[cfg(test)] gating from the pub evidence: ClaudeRelationshipEvidence
field(s) (and any other similarly guarded evidence fields around the same
struct, e.g., the second occurrence at lines ~121-122) so
ParseResult/ParseIncrementalResult exposes evidence in non-test builds and
callers can reconcile across files.
---
Outside diff comments:
In `@crates/relayburn-sdk/src/analyze/provider_reattribution.rs`:
- Around line 133-140: The doc examples advertise extend_default_rules but the
function is currently gated with #[cfg(test)] so it’s unavailable in non-test
builds; either remove the #[cfg(test)] from extend_default_rules in
provider_reattribution.rs so the helper is public for normal builds (keeping its
signature: pub fn extend_default_rules<I>(extra: I) -> Vec<ProviderRule> where
I: IntoIterator<Item = ProviderRule>), or update the module docs to show the
always-available alternative using default_rules().to_vec() and Vec::extend
instead; pick one fix and make the code/docs consistent (references:
extend_default_rules, default_rules, ProviderRule in provider_reattribution.rs).
🪄 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: 31ddf9ce-bd14-471e-be67-d6b6bc55b4a2
📒 Files selected for processing (31)
crates/relayburn-sdk/src/analyze.rscrates/relayburn-sdk/src/analyze/claude_md.rscrates/relayburn-sdk/src/analyze/fidelity.rscrates/relayburn-sdk/src/analyze/findings.rscrates/relayburn-sdk/src/analyze/ghost_surface.rscrates/relayburn-sdk/src/analyze/pricing.rscrates/relayburn-sdk/src/analyze/provider.rscrates/relayburn-sdk/src/analyze/provider_reattribution.rscrates/relayburn-sdk/src/analyze/quality.rscrates/relayburn-sdk/src/analyze/tool_call_patterns.rscrates/relayburn-sdk/src/analyze/tool_output_bloat.rscrates/relayburn-sdk/src/ingest.rscrates/relayburn-sdk/src/ingest/cursors.rscrates/relayburn-sdk/src/ingest/gap.rscrates/relayburn-sdk/src/ingest/gap_warning_tests.rscrates/relayburn-sdk/src/ingest/ingest.rscrates/relayburn-sdk/src/ingest/orchestration_tests.rscrates/relayburn-sdk/src/ingest/pending_stamps.rscrates/relayburn-sdk/src/ingest/reingest.rscrates/relayburn-sdk/src/ledger.rscrates/relayburn-sdk/src/ledger/fingerprint.rscrates/relayburn-sdk/src/ledger/schema.rscrates/relayburn-sdk/src/lib.rscrates/relayburn-sdk/src/reader.rscrates/relayburn-sdk/src/reader/claude.rscrates/relayburn-sdk/src/reader/codex.rscrates/relayburn-sdk/src/reader/fidelity.rscrates/relayburn-sdk/src/reader/opencode.rscrates/relayburn-sdk/src/reader/opencode_stream.rscrates/relayburn-sdk/src/reader/opencode_stream/tests.rscrates/relayburn-sdk/src/reader/user_turn.rs
💤 Files with no reviewable changes (9)
- crates/relayburn-sdk/src/reader/opencode_stream/tests.rs
- crates/relayburn-sdk/src/ledger/schema.rs
- crates/relayburn-sdk/src/lib.rs
- crates/relayburn-sdk/src/reader/user_turn.rs
- crates/relayburn-sdk/src/ingest/pending_stamps.rs
- crates/relayburn-sdk/src/analyze/pricing.rs
- crates/relayburn-sdk/src/analyze/ghost_surface.rs
- crates/relayburn-sdk/src/analyze/provider.rs
- crates/relayburn-sdk/src/reader/opencode_stream.rs
Comment on lines
+49
to
53
| /// Read by the in-crate test suite to verify the From<ParseIncrementalResult> | ||
| /// conversion preserves evidence. Production callers consume the incremental | ||
| /// result directly and access `evidence` from there. | ||
| #[cfg(test)] | ||
| pub evidence: ClaudeRelationshipEvidence, |
There was a problem hiding this comment.
Don't hide reconciliation evidence from release builds.
parse_claude_session is still part of the exported Claude surface in crates/relayburn-sdk/src/reader.rs (Lines 43-48), and reconcile_claude_session_relationships still takes ClaudeRelationshipEvidence. Gating ParseResult::evidence behind #[cfg(test)] means release callers can no longer do a full parse and then reconcile across files, while the in-crate tests keep passing because they still compile with the field present.
Suggested fix
pub struct ParseResult {
pub turns: Vec<TurnRecord>,
pub content: Vec<ContentRecord>,
pub events: Vec<CompactionEvent>,
pub relationships: Vec<SessionRelationshipRecord>,
pub tool_result_events: Vec<ToolResultEventRecord>,
pub user_turns: Vec<UserTurnRecord>,
- #[cfg(test)]
pub evidence: ClaudeRelationshipEvidence,
}
@@
impl From<ParseIncrementalResult> for ParseResult {
fn from(r: ParseIncrementalResult) -> Self {
Self {
turns: r.turns,
content: r.content,
events: r.events,
relationships: r.relationships,
tool_result_events: r.tool_result_events,
user_turns: r.user_turns,
- #[cfg(test)]
evidence: r.evidence,
}
}
}Also applies to: 121-122
🤖 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/reader/claude.rs` around lines 49 - 53, The evidence
field is incorrectly hidden behind #[cfg(test)], breaking downstream APIs like
parse_claude_session and reconcile_claude_session_relationships that expect
ClaudeRelationshipEvidence; remove the #[cfg(test)] gating from the pub
evidence: ClaudeRelationshipEvidence field(s) (and any other similarly guarded
evidence fields around the same struct, e.g., the second occurrence at lines
~121-122) so ParseResult/ParseIncrementalResult exposes evidence in non-test
builds and callers can reconcile across files.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
No issues found across 31 files
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
`attribute_claude_md` and `extend_default_rules` are now `#[cfg(test)]`, so their non-test consumers' intra-doc links resolve to nothing in a default `cargo doc` build. - `attribute_claude_md_refs` doc: drop the link; describe the function on its own terms. - `provider_reattribution` module doc: point at `default_rules` plus a manual `Vec` spread instead of the test-only convenience. https://claude.ai/code/session_01WavFVyZAAyiSmWaktaXsmD
willwashburn
pushed a commit
that referenced
this pull request
May 25, 2026
Main released 2.10.0 (hotspots MCP rollup #424), 2.10.1 (#427 + pending-stamp refactor #428), and the workspace-wide dead_code/ unused_imports cleanup #429. The latter removed ingest_claude_projects from the public re-export and switched orchestration_tests to `crate::ingest::ingest::*` deep imports. Conflict resolution: - crates/relayburn-sdk/src/ingest.rs: keep main's narrowed re-export (no ingest_claude_projects) and add ingest_claude_transcript_path. - crates/relayburn-sdk/src/ingest/orchestration_tests.rs: keep main's deep imports and add ingest_claude_transcript_path next to its sibling verbs. - CHANGELOG.md: slot this branch's items under [Unreleased] above main's new 2.10.0 release section. https://claude.ai/code/session_011ubB69Zxijqb1BsYVYL9iQ
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
2 participants
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
Closes #336.
#[allow(dead_code, unused_imports)]from the four absorbed module roots (analyze.rs,ingest.rs,ledger.rs,reader.rs) and the per-mod#[allow(unused_imports)]on the lib.rs verb mods. Resolves the ~130 warnings that surfaced by trimming inner-module-root re-export chains, deleting dead items, and#[cfg(test)]-gating test-only helpers.provider_for_turn/resolve_turn_provideraliases, inlines theseverity_from_usd_pubvisibility-laundering wrapper (nowpub(crate) severity_from_usd), and dropsFidelity::classify(tests useclassify_fidelitydirectly).state.rs run_resetandsummary.rs --by-*flag stubs called out in the issue are already implemented onmain, so no change there.reader/opencode_stream.rs(1.6k lines) and the bulk ofingest/reingest.rs.lib.rs(the published SDK surface) is unchanged.Test plan
cargo build --workspace— 0 warnings (down from 130 after the blanket allows were dropped)cargo test --workspace— 10 binaries, 775 tests passcargo check -p relayburn-sdk-node -p relayburn-cli— downstream consumers compile against the unchanged SDK surfacepnpm run test— Node SDK facade + MCP tests pass (napi-not-built skips are standard locally)https://claude.ai/code/session_01WavFVyZAAyiSmWaktaXsmD
Generated by Claude Code