Skip to content

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

Merged
jensholdgaard merged 2 commits into
mainfrom
docs-177-canonical-naming
Jul 2, 2026
Merged

docs(querier): finish #177's doc sweep — Ourios-canonical, not OTLP-canonical#333
jensholdgaard merged 2 commits into
mainfrom
docs-177-canonical-naming

Conversation

@jensholdgaard

@jensholdgaard jensholdgaard commented Jul 2, 2026

Copy link
Copy Markdown
Owner

What

Closes the residue of #177. The core bug — non-finite doubles (NaNInf)
encoding to {"doubleValue":null} and failing decode — was fixed in #271
(RFC 0018.5): the canonical codec now emits and decodes the proto3-JSON string
forms "NaN"/"Infinity"/"-Infinity", pinned by
nonfinite_doubles_round_trip_via_proto3_strings (green on main).

Verified against the OTel spec (via the OpenTelemetry MCP) before closing:
OTLP/JSON is the proto3 JSON Mapping plus four explicit deviations (hex IDs,
int enums, unknown-field tolerance, lowerCamelCase) — non-finite doubles are
not among them, so the proto3 quoted-string rule applies and the landed fix is
the spec-faithful form (with-serde's null is the deviation). The Collector
recently aligned its own Value.AsString() the same way.

This PR is the last item on #177's list — the ride-along doc sweep, now
repo-wide (the first commit only caught compile.rs; Copilot flagged the
rest):

  • compile.rs ×2, record_batch.rs ×2, cluster.rs ×1: rename
    "OTLP-canonical" → Ourios-canonical (the convention — deliberately so:
    our codec is spec-faithful exactly where with-serde is not).
  • cluster.rs test note: rewrite the genuinely stale text that still described
    body as "the AnyValue's Debug form — an interim placeholder" awaiting a
    future canonicalisation PR (landed long ago; the body is the Ourios-canonical
    JSON from ingest_structuredcanonical::encode_any_value).

grep -rn "OTLP-canonical" crates/ is now empty. The remaining docs/ hits are
historical amendment notes that describe the old name (e.g. RFC 0001/0005
"previously called the encoding…") and are correct as written.

Verification

cargo clippy -p ourios-querier -p ourios-miner -p ourios-parquet --all-targets --all-features -- -D warnings, cargo fmt --all --check, cargo test -p ourios-miner (the edited test module) — all clean. Comment-only changes.

🤖 Generated with Claude Code

…l, 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>
@jensholdgaard
jensholdgaard requested a review from Copilot July 2, 2026 20:20
@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Two comments in compile.rs were updated to reference "Ourios-canonical-JSON" instead of "OTLP-canonical-JSON" for describing attribute JSON matching. No functional or logic changes were made.

Changes

Documentation terminology correction

Layer / File(s) Summary
Comment terminology update
crates/ourios-querier/src/compile.rs
Two comments (module-level and doc comment) are updated to reference Ourios-canonical-JSON instead of OTLP-canonical-JSON, with no logic changes.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Related Issues: None specified

Related PRs: None specified

Suggested labels: documentation

Suggested reviewers: None specified

Poem
A rabbit hopped through lines of code,
Fixed a word along the way it strode,
From OTLP to Ourios true,
Just comments changed, nothing new,
Hop, hop, hop — the docs now glowed. 🐰

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is detailed, but it does not follow the template: it lacks explicit Summary, Related, and Checklist sections. Rename 'What' to Summary, add a Related section linking #177/#271, and include the checklist items with their status.
✅ Passed checks (4 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title is concise and accurately summarizes the doc-only cleanup from OTLP-canonical to Ourios-canonical.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch docs-177-canonical-naming

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.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates querier compiler documentation to use the project’s “Ourios-canonical” naming for the JSON encoding of attribute columns, aligning the docs with the canonical codec’s intended convention (spec-faithful where upstream with-serde is not).

Changes:

  • Rename “OTLP-canonical-JSON” → “Ourios-canonical-JSON” in compile.rs module-level docs.
  • Rename “OTLP-canonical-JSON” → “Ourios-canonical-JSON” in attr_match doc comment.

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

Comment thread crates/ourios-querier/src/compile.rs
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>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

@jensholdgaard
jensholdgaard merged commit e15a0e8 into main Jul 2, 2026
22 checks passed
@jensholdgaard
jensholdgaard deleted the docs-177-canonical-naming branch July 2, 2026 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants