relayburn: enum wire_str()/Display; drop serde_json::Value round-trips#350
relayburn: enum wire_str()/Display; drop serde_json::Value round-trips#350willwashburn merged 1 commit intomainfrom
Conversation
Add `wire_str() -> &'static str` and `impl Display` to `SourceKind`,
`FidelityClass`, `UsageGranularity`, `RelationshipSourceKind`, and
`RelationshipType` in `crates/relayburn-sdk/src/reader/types.rs`.
Replace round-trip sites with direct enum-method calls:
SDK (per-record alloc):
- `ledger/fingerprint.rs` — drop the `serde_plain` helper; the five
fingerprint fns now call `wire_str()` directly. Avoids per-record
`serde_json::Value` materialization on every ingest.
- `ledger/reader.rs::relationship_passes` — compare via `wire_str()`
instead of two per-row `to_value` allocations.
- `ledger/writer.rs` — drop the `source_str` helper; ten call sites
now pass `wire_str()` directly to `params!`.
- `analyze/findings.rs` — replace the `match serde_json::to_value(...)`
block in the edit-heavy detail string.
- `analyze/overhead.rs::describe_applies_to` — replace three `to_value`
round-trips per call with a one-line `iter().map(SourceKind::wire_str)`.
CLI (parallel hand-written converters now redundant):
- `commands/compare.rs::fidelity_class_str` — deleted.
- `commands/summary.rs::{fidelity_class_key,granularity_key}` — deleted.
- `commands/hotspots.rs::{source_label,granularity_label}` — deleted.
Scope expansion noted: the directive named three enums but the
fingerprint helper handles five (including the relationship enums).
Adding `wire_str` to `RelationshipSourceKind` and `RelationshipType`
was required to fully delete `serde_plain` and is the same shape of
change. No variants or names touched.
Tests: 729 passed, 0 failed (golden snapshots verify byte-identical
output). Clippy warning count unchanged at 17.
Closes #327.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR unifies enum-to-string representation across the Relayburn SDK and CLI by introducing ChangesWire String Representation Refactoring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/relayburn-sdk/src/reader/types.rs (1)
27-45: ⚡ Quick winAdd serde↔wire parity tests for all enum variants.
These
wire_str()/Displaymappings are now canonical in hashing/output paths, so a typo in any literal can silently drift behavior. Please add table-driven tests assertingwire_str(),Display, and serde serialization stay identical per variant.Proposed test pattern
+#[test] +fn source_kind_wire_and_serde_match() { + let cases = [ + (SourceKind::ClaudeCode, "claude-code"), + (SourceKind::Codex, "codex"), + (SourceKind::Opencode, "opencode"), + (SourceKind::AnthropicApi, "anthropic-api"), + (SourceKind::OpenaiApi, "openai-api"), + (SourceKind::GeminiApi, "gemini-api"), + ]; + for (v, label) in cases { + assert_eq!(v.wire_str(), label); + assert_eq!(v.to_string(), label); + assert_eq!(serde_json::to_string(&v).unwrap(), format!("\"{label}\"")); + } +}Also applies to: 61-82, 153-169, 234-251, 361-377
🤖 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/types.rs` around lines 27 - 45, Add a table-driven unit test that iterates every SourceKind variant and for each asserts three things are identical: the wire_str() return, the Display output via fmt::Display (format!("{}", variant)), and the serde serialization string (serialize the enum and compare to the expected kebab-case token). Target the SourceKind enum and its wire_str()/fmt::Display implementations so the test fails if any literal drifts; mirror this pattern for the other enums in the same file by creating analogous table-driven tests covering all their variants and serde output.
🤖 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/src/reader/types.rs`:
- Around line 27-45: Add a table-driven unit test that iterates every SourceKind
variant and for each asserts three things are identical: the wire_str() return,
the Display output via fmt::Display (format!("{}", variant)), and the serde
serialization string (serialize the enum and compare to the expected kebab-case
token). Target the SourceKind enum and its wire_str()/fmt::Display
implementations so the test fails if any literal drifts; mirror this pattern for
the other enums in the same file by creating analogous table-driven tests
covering all their variants and serde output.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: df3c8ff9-725b-4637-87f1-f263db0d9e82
📒 Files selected for processing (9)
crates/relayburn-cli/src/commands/compare.rscrates/relayburn-cli/src/commands/hotspots.rscrates/relayburn-cli/src/commands/summary.rscrates/relayburn-sdk/src/analyze/findings.rscrates/relayburn-sdk/src/analyze/overhead.rscrates/relayburn-sdk/src/ledger/fingerprint.rscrates/relayburn-sdk/src/ledger/reader.rscrates/relayburn-sdk/src/ledger/writer.rscrates/relayburn-sdk/src/reader/types.rs
Summary
Add
wire_str() -> &'static strandimpl DisplaytoSourceKind,FidelityClass,UsageGranularity,RelationshipSourceKind,RelationshipTypeincrates/relayburn-sdk/src/reader/types.rs. Replaceserde_json::Valueround-trip sites in the SDK and the parallel hand-written converters in the CLI with direct enum-method calls.SDK (per-record alloc removed):
ledger/fingerprint.rs— drop theserde_plainhelper; five fingerprint fns now callwire_str().ledger/reader.rs::relationship_passes— compare viawire_str()instead of two per-rowto_valueallocs.ledger/writer.rs— dropsource_str; ten callsites passwire_str()toparams!.analyze/findings.rs— replacematch serde_json::to_value(...)block.analyze/overhead.rs::describe_applies_to— replace 3to_valueround-trips per call.CLI (5 hand-written converters deleted):
compare.rs::fidelity_class_str,summary.rs::{fidelity_class_key,granularity_key},hotspots.rs::{source_label,granularity_label}.Sets up the foundation for #324 (the ledger
Query.sourcecomparison no longer needs a per-row Value alloc).Closes #327.
Test plan
cargo build --workspace --all-targetscleancargo test --workspace— 729 passed (golden snapshots verify byte-identical output)cargo clippy --workspace --all-targets— no new warnings🤖 Generated with Claude Code