feat(miner): structured-body short-circuit + canonical render (RFC0001.9) - #167
Conversation
…1.9) Flips RFC0001.9 (`#[ignore]`/`todo!` → a real scenario test) and wires the §6.6 structured render against the just-pinned RFC 0001 §6.1 / RFC 0005 §3.3 "Ourios canonical body encoding" (#166). - RFC0001.9: ingest a `Body::Structured(KvlistValue)`, assert the §6.2 step-0 short-circuit — `body_kind = Structured`, a structured-template id keyed on `(severity, scope, Structured)` (same tuple reuses, a different tuple allocates), `body` round- trips via `canonical::decode_any_value`, `lossy_flag = false`, empty `params`/`separators`, `confidence == 1.0`. - Byte-shape gate in `otlp::canonical`: asserts the EXACT bytes per variant (int64-as-decimal-string, bytes-as-base64, lowerCamelCase, order-preserving kvlist `zeta,alpha,mid` — NOT sorted), locking the OTLP/JSON shape rather than only struct round-trip. - §6.6 structured `Reconstruction`: `render` no longer panics on non-String rows. `Structured` → canonical `body` bytes with `Reconstruction::Faithful` (the canonical encoding is the §3.3 round-trip; `lossy_flag = false`); `Absent` → empty bytes with `RetainedVerbatim`. String path unchanged. `Reconstruction` stays `#[non_exhaustive]`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@coderabbitai review |
|
Warning Review limit reached
More reviews will be available in 49 minutes and 3 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR extends structured-body handling in the OTLP canonical pipeline by refactoring the ChangesStructured Body Handling and Validation
Sequence Diagram(s)The changes do not meet the criteria for sequence diagram generation: they primarily consist of function refactoring, test additions, and validation logic without introducing new multi-component interactions or significant control-flow changes. 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)
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 |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Pull request overview
Implements RFC0001.9’s structured-body short-circuit end-to-end at the miner layer: adds scenario coverage, pins the canonical body encoding byte shape, and extends reader-side rendering to handle Structured/Absent bodies without panicking.
Changes:
- Turned the RFC0001.9 red-gate stub into a real scenario test validating structured-template-id semantics and canonical-body round-trip.
- Added a byte-for-byte “canonical body encoding” test to pin JSON field naming, int64-as-string, bytes base64, and order preservation.
- Updated
renderto handleBodyKind::Structured(return canonical bytes) andBodyKind::Absent(return empty bytes) and added corresponding unit tests.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| crates/ourios-miner/tests/rfc_internal.rs | Adds RFC0001.9 scenario test for structured-body short-circuit and structured-template-id semantics. |
| crates/ourios-miner/src/reconstruct.rs | Extends render for Structured/Absent bodies and adds unit tests for those branches. |
| crates/ourios-core/src/otlp.rs | Adds tests pinning the exact emitted bytes for the Ourios canonical body encoding. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/ourios-miner/tests/rfc_internal.rs (1)
639-673: ⚡ Quick winAssert structured invariants across all emitted rows, not only
emitted[0].Line 641 onward validates only the first drained record. Iterating all emitted rows (and matching expected template ids per scope) would make this scenario much harder to false-pass.
🤖 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/ourios-miner/tests/rfc_internal.rs` around lines 639 - 673, The test currently only checks invariants on emitted[0]; change it to iterate over all drained records from records.drain() and run the same assertions for each rec: assert rec.body_kind == BodyKind::Structured, rec.template_id matches the expected template id for that ingest scope (use id_a1 or the appropriate id variable per expected order), decode body with canonical::decode_any_value and compare to kvlist(), and assert rec.lossy_flag is false, rec.params.is_empty(), rec.separators.is_empty(), and rec.confidence equals 1.0 (within f32::EPSILON); apply these checks to every rec rather than only emitted[0] so all emitted rows validate the structured invariants.
🤖 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/ourios-miner/src/reconstruct.rs`:
- Around line 221-227: The code currently uses body_bytes_or_empty(record) for
both BodyKind::Structured and BodyKind::Absent and unconditionally labels
Structured as Reconstruction::Faithful and Absent as
Reconstruction::RetainedVerbatim; instead, check record.body: for
BodyKind::Structured only return Reconstruction::Faithful when
record.body.is_some() (otherwise mark it as a non-faithful reconstruction), and
for BodyKind::Absent do not surface stored bytes blindly — avoid using
body_bytes_or_empty(record) when record.body is None (return an empty body or
the appropriate "no-body" bytes and the correct non-faithful Reconstruction
variant); update the match arms for BodyKind::Structured and BodyKind::Absent
accordingly, referencing BodyKind::Structured, BodyKind::Absent,
body_bytes_or_empty(record), and Reconstruction::<variant> to ensure faithful vs
non-faithful labels follow the render contract.
---
Nitpick comments:
In `@crates/ourios-miner/tests/rfc_internal.rs`:
- Around line 639-673: The test currently only checks invariants on emitted[0];
change it to iterate over all drained records from records.drain() and run the
same assertions for each rec: assert rec.body_kind == BodyKind::Structured,
rec.template_id matches the expected template id for that ingest scope (use
id_a1 or the appropriate id variable per expected order), decode body with
canonical::decode_any_value and compare to kvlist(), and assert rec.lossy_flag
is false, rec.params.is_empty(), rec.separators.is_empty(), and rec.confidence
equals 1.0 (within f32::EPSILON); apply these checks to every rec rather than
only emitted[0] so all emitted rows validate the structured invariants.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d6941c7f-8e5d-483b-b448-b92929d44163
📒 Files selected for processing (3)
crates/ourios-core/src/otlp.rscrates/ourios-miner/src/reconstruct.rscrates/ourios-miner/tests/rfc_internal.rs
…ver-empty Structured with body=None now renders (empty, RetainedVerbatim) not Faithful (empty bytes are not a faithful render). Absent always renders empty (Vec::new()), never surfacing a stray stored body. Add guard tests for both + a doc caveat. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…rip (#176) Fixes the finite-f64 half of #130 — with a corrected diagnosis: the with-serde encoder was never lossy (it emits correct shortest-round-trip digits); the 1-2 ULP drift was serde_json's default approximate float PARSER. Enabling serde_json's float_roundtrip feature in ourios-core makes decode(encode(x)) bit-exact (200k-random-f64 proptest, zero drift; the #130 repro is a pinned regression case). Byte shapes unchanged — the #167 byte-shape test passes unmodified. Also pinned: non-finite doubles (NaN/±Inf) encode to {"doubleValue":null} and the decoder rejects it — a pre-existing gap, follow-up issue filed separately. RFC 0001 §6.1 carries a 2026-06-11 amendment relocating the loss to the decode side and declaring float_roundtrip load-bearing. Naming + stale-doc cleanups in ourios-core ride along. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
What
Implements the RFC0001.9 scenario (structured-body short-circuit) code slice against the just-merged RFC 0001 §6.1/§6.2 + RFC 0005 §3.3 "Ourios canonical body encoding" pin (#166). The encoder + producer already existed; this slice is tests + the §6.6 structured render, plus flipping the acceptance stub.
Three files:
crates/ourios-miner/tests/rfc_internal.rs— flipsrfc0001_9_…from#[ignore]+todo!()to a real scenario test. Ingests anOtlpLogRecordwhosebody = Body::Structured(KvlistValue)(keyszeta, alpha, so a real structured body that exercises order) and asserts the §6.2 step-0 short-circuit:body_kind == Structured;(severity_number=9, scope_name="lib.auth")get the same id; a record differing only in scope ("lib.payments") gets a different id — i.e. the id is keyed on(severity, scope, BodyKind::Structured)per §6.2 step 0;bodyisSome, andcanonical::decode_any_value(body.as_bytes())round-trips to the originalAnyValue;lossy_flag == false,paramsempty,separatorsempty,confidence == 1.0(the §6.1 sentinel).crates/ourios-core/src/otlp.rs(mod canonical) — a byte-shape verification test (encoder_emits_exact_canonical_bytes_per_variant): assertsencode_any_valueemits exactly these bytes (UTF-8 string compare, not just struct round-trip):int(-42)→{"intValue":"-42"},double(2.71)→{"doubleValue":2.71},bool(true)→{"boolValue":true},bytes(b"raw\0bytes")→{"bytesValue":"cmF3AGJ5dGVz"},array[string("a"),int(1)]→{"arrayValue":{"values":[{"stringValue":"a"},{"intValue":"1"}]}};zeta, alpha, midemits them in that order — proving order is preserved, not sorted (explicitly NOT RFC 8785 / JCS). This locks int64-as-decimal-string, base64, lowerCamelCase, and order-preservation.crates/ourios-miner/src/reconstruct.rs— wires the §6.6 structuredReconstruction.renderpreviouslyassert!dStringand panicked on Structured/Absent. Now:BodyKind::Structured→ canonicalbodybytes withReconstruction::Faithful(the canonical encoding is byte-deterministic and bidirectional,lossy_flag = false, so the stored body is the §3.3 round-trip — no template walk);BodyKind::Absent→ empty bytes withReconstruction::RetainedVerbatim(nothing reconstructed; the emptybodycolumn surfaced verbatim — the honest "not rebuilt from a template" signal);Stringpath unchanged (lossy/overflow/clean split intact).Reconstructionstays#[non_exhaustive]. The §6.6 amendment left the structured-renderReconstructionmapping "open … to be settled when structured-body rendering is wired" — this is that wiring.rendertests for a structured row (→ canonical bytes +Faithful) and an absent row (→ empty +RetainedVerbatim); refreshedreconstruct's now-stale "producer-side gap" doc (the producer populatesbodysince feat(miner): reader render emits body+RetainedVerbatim for lossy rows (H7.3) #163/docs(rfc-0001): pin the structured-body canonical encoding as an ourios-local rule #166).Invariant: §3.3 bit-identical body reconstruction
For structured rows, reconstruction faithfulness is the Ourios round-trip via the canonical encoding:
stored_bytes ↔ AnyValueis bidirectional and byte-deterministic (§6.1), andlossy_flagis unconditionallyfalse, so the storedbodycolumn is itself the faithful representation —renderreturns it withFaithfuland walks no template. The byte-shape test pins that encoding so a serialiser swap can't silently drift the column.Notes for the maintainer
Verification (local)
cargo test --all-features→ all green (full result line below)cargo fmt --all --check→ cleancargo clippy --all-targets --all-features -- -D warnings→ exit 0, cleanTop-level workspace result line:
test result: ok. 130 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out(ourios-miner lib), withrfc0001_9_…no longer ignored and passing across the workspace (0 failed everywhere).🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests