feat: rfc0018.5 green — non-finite doubles round-trip; RFC 0018 green by jensholdgaard · Pull Request #271 · jensholdgaard/ourios

jensholdgaard · 2026-06-20T20:15:37Z

RFC 0018 green (6/6) — non-finite doubles round-trip; RFC 0018 fully green (RFC0018.5)

serde_json renders a non-finite f64 as JSON null — lossy (NaN/±Inf collapse to one shape), and opentelemetry-proto's deserializer rejects both null and the proto3 string form for an f64 field (verified). So a structured Body / attribute carrying NaN/Infinity/-Infinity silently dropped the value (RFC 0018 §3.4).

Fix — a guarded robust path (round-trip is the §3.3 "canonical" contract)

When and only when a non-finite double is present, ourios-core's canonical encode/decode walk the AnyValue tree and use the proto3-JSON string forms ("NaN"/"Infinity"/"-Infinity"), delegating finite/string/int/bytes leaves to opentelemetry-proto's exact serde. The finite case stays on the untouched #130-bit-exact fast path. Same for encode_attributes/decode_attributes. (No new serde direct-dep — custom errors via serde_json::Error::io.)

Verification

RFC0018.5 (ourios-core/tests): round-trips at top level, nested in a structured body (kvlist+array mixing the three non-finite with finite doubles and strings), and as an attribute value.
Overturns the pinned nonfinite_doubles_encode_to_the_null_shape test → …round_trip_via_proto3_strings (a §6.2 contract change the RFC authorizes).
clippy --all-targets, fmt, mdbook, semconv no-diff clean. NaN payloads canonicalise (proto3 doesn't preserve them) — value-equality is the contract.

RFC 0018 status flipped red → green — all six §5 scenarios pass.

⚠️ Merge order: merge #270 (.3) first, then this — so main never shows RFC 0018 green while .3 is still out. (.1/.2/.4/.6 already merged.)

🤖 Generated with Claude Code

coderabbitai · 2026-06-20T20:15:44Z

Warning

Review limit reached

@jensholdgaard, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 26 minutes and 14 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 8a2b548c-df7a-42fb-9911-70f59e30c0a7

📥 Commits

Reviewing files that changed from the base of the PR and between 5504e21 and 75d1580.

📒 Files selected for processing (3)

crates/ourios-core/src/otlp.rs
crates/ourios-core/tests/rfc0018_otlp_compliance.rs
docs/rfcs/0018-otlp-log-spec-compliance.md

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch feat/rfc0018-green-nonfinite

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.

❤️ Share

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

Copilot

Pull request overview

This PR completes RFC 0018 scenario .5 by making Ourios-canonical JSON round-trip NaN/Infinity/-Infinity for AnyValue and attribute values, and flips RFC 0018’s overall status to green.

Changes:

Add a guarded “robust” canonical JSON encode/decode path that uses proto3-JSON string forms for non-finite doubles while keeping the existing fast path for finite values.
Turn RFC0018.5 from an ignored red stub into a green acceptance test covering top-level, nested structured bodies, and attributes.
Update RFC 0018 document status from red to green.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
docs/rfcs/0018-otlp-log-spec-compliance.md	Marks RFC 0018 as `green` now that all acceptance scenarios pass.
crates/ourios-core/src/otlp.rs	Implements non-finite double handling in canonical JSON encode/decode (AnyValue + attributes), plus updates the pinned non-finite-double test.
crates/ourios-core/tests/rfc0018_otlp_compliance.rs	Adds the RFC0018.5 green test asserting non-finite doubles round-trip in multiple placements.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

serde_json renders a non-finite f64 as JSON null (lossy: NaN/±Inf collapse to one shape, and opentelemetry-proto's deserializer rejects both null and the proto3 string form for an f64 field), so a structured Body / attribute carrying NaN/Infinity/-Infinity didn't round-trip (RFC 0018 §3.4). Add a guarded robust path to ourios-core's canonical codec: when — and only when — a non-finite double is present, encode/decode walk the AnyValue tree and use the proto3-JSON string forms ("NaN"/"Infinity"/"-Infinity"), delegating finite/string/int/bytes leaves to opentelemetry-proto's exact serde. The finite case stays on the untouched #130-bit-exact fast path. encode_attributes/decode_attributes get the same treatment for attribute values. (No serde direct-dep: custom-error via serde_json::Error::io.) Greens RFC0018.5 (round-trip at top level, nested in a structured body, and as an attribute value) and overturns the pinned "encodes to null" test (§6.2 contract change). RFC 0018 status red → green — all six §5 scenarios pass (.1/.2/.3/.4/.5/.6). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Copilot: the module-level canonical-JSON doc and CanonicalJsonError doc still said non-finite doubles encode as JSON null and do not decode. That was the pre-RFC0018.5 behaviour; the canonical codec now round-trips them via the proto3-JSON string forms (NaN/Infinity/-Infinity) on a guarded path. Update both comments to match. Doc-only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

…anonical (#333) * docs(querier): finish #177's doc sweep — the codec is Ourios-canonical, not OTLP-canonical The last two stale mentions from #177's ride-along list: compile.rs still called the attributes-column encoding "OTLP-canonical-JSON". Post-#166 the codebase convention is "Ourios-canonical" — deliberately so, since the codec deviates from opentelemetry-proto's with-serde exactly where with-serde deviates from the proto3 JSON mapping (non-finite doubles as "NaN"/"Infinity"/"-Infinity" strings, RFC 0018 §3.4 — the #177 core bug, fixed in #271). Doc-comment-only change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: complete the repo-wide Ourios-canonical sweep (miner + parquet) Copilot caught that my "last remaining mentions" claim was wrong — I had grepped for cluster.rs / record_batch.rs under ourios-querier, where those files don't exist (they live in ourios-miner and ourios-parquet), and misread no-matches as already-clean. Repo-wide sweep now: - cluster.rs: rename the §3.3 encoding mention, and rewrite the genuinely stale test note that still described `body` as "the AnyValue's Debug form — an interim placeholder" awaiting a future canonicalisation PR (that PR landed long ago; the body is the Ourios-canonical JSON from `ingest_structured` → `canonical::encode_any_value`). - record_batch.rs: rename both mentions. The remaining "OTLP-canonical" hits in docs/ are historical amendment notes that *describe* the old name (e.g. RFC 0001/0005 "previously called the encoding…") and are correct as written. Comment-only; `cargo grep` for OTLP-canonical in crates/ is now empty. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

jensholdgaard requested a review from Copilot June 20, 2026 20:15

Copilot started reviewing on behalf of jensholdgaard June 20, 2026 20:16 View session

Copilot AI reviewed Jun 20, 2026

View reviewed changes

jensholdgaard force-pushed the feat/rfc0018-green-nonfinite branch from 4e49ca4 to e62a416 Compare June 20, 2026 21:33

jensholdgaard requested a review from Copilot June 20, 2026 21:33

Copilot started reviewing on behalf of jensholdgaard June 20, 2026 21:33 View session

Copilot AI reviewed Jun 20, 2026

View reviewed changes

Comment thread crates/ourios-core/src/otlp.rs

Comment thread crates/ourios-core/src/otlp.rs

jensholdgaard requested a review from Copilot June 20, 2026 21:44

Copilot started reviewing on behalf of jensholdgaard June 20, 2026 21:44 View session

Copilot AI reviewed Jun 20, 2026

View reviewed changes

jensholdgaard merged commit dcff67e into main Jun 20, 2026
21 checks passed

jensholdgaard deleted the feat/rfc0018-green-nonfinite branch June 20, 2026 21:49

jensholdgaard mentioned this pull request Jul 2, 2026

docs(querier): finish #177's doc sweep — Ourios-canonical, not OTLP-canonical #333

Merged

jensholdgaard mentioned this pull request Jul 2, 2026

non-finite doubles (NaN/±Inf) break the canonical round-trip — null emitted, decode rejects #177

Closed

Navigation Menu

feat: rfc0018.5 green — non-finite doubles round-trip; RFC 0018 green - #271

feat: rfc0018.5 green — non-finite doubles round-trip; RFC 0018 green#271
jensholdgaard merged 2 commits into
mainfrom
feat/rfc0018-green-nonfinite

jensholdgaard commented Jun 20, 2026

Uh oh!

coderabbitai Bot commented Jun 20, 2026 •

edited

Loading

Review limit reached

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

jensholdgaard commented Jun 20, 2026

RFC 0018 green (6/6) — non-finite doubles round-trip; RFC 0018 fully green (RFC0018.5)

Fix — a guarded robust path (round-trip is the §3.3 "canonical" contract)

Verification

Uh oh!

coderabbitai Bot commented Jun 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai Bot commented Jun 20, 2026 •

edited

Loading