Skip to content

feat(querier): rfc 0025 green b — rendering distinguishes absent from empty - #381

Merged
jensholdgaard merged 2 commits into
mainfrom
rfc0025-green-rendering
Jul 6, 2026
Merged

feat(querier): rfc 0025 green b — rendering distinguishes absent from empty#381
jensholdgaard merged 2 commits into
mainfrom
rfc0025-green-rendering

Conversation

@jensholdgaard

@jensholdgaard jensholdgaard commented Jul 5, 2026

Copy link
Copy Markdown
Owner

Second RFC 0025 green slice, discharging RFC0025.3.

  • LogBody::Absent variant (additive — the enum is #[non_exhaustive]); render_log_body short-circuits on BodyKind::Absent before any template walk.
  • The HTTP row DTO's body becomes Option with skip_serializing_if: an absent-body row's JSON omits the key entirely — never "", which is a different legal record (per the OTel data model, Body is optional; absence and empty-string are distinct states).
  • Scenario .3 runs both rows through the full query path (real store → DataFusion → rendering); a server-side unit test pins the JSON omission in both directions via LogRow::from_record.

Verification: fmt clean, clippy clean (querier + server, RC checked), querier 21 suites / 160 tests, server 12 suites / 76 tests, all green.

Remaining for RFC 0025: slice c (sink quarantine + telemetry, .4/.5) — which closes #362.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Distinguish between an empty log body and a truly missing one in query results.
    • API responses now omit the body field when no body is present, instead of sending an empty value.
    • Rendering behavior for malformed structured entries remains unchanged.
  • Tests

    • Added coverage for empty-string vs absent-body rendering and response serialization.

… empty

LogBody gains an Absent variant (the enum is non_exhaustive, so
additive); render_log_body short-circuits on BodyKind::Absent; the
HTTP row DTO omits the body key entirely for absent rows (RFC 0025
§3.2) — never an empty string, which is a different legal record.
Discharges RFC0025.3, with a server-side DTO test pinning the JSON
omission in both directions.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@jensholdgaard
jensholdgaard requested a review from Copilot July 5, 2026 23:33
@coderabbitai

coderabbitai Bot commented Jul 5, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

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

Next review available in: 43 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

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 reviews.

How do review limits work?

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

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, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 34ea7e21-2222-43bf-ac4d-8f1699685983

📥 Commits

Reviewing files that changed from the base of the PR and between 6b2f73f and 5f700aa.

📒 Files selected for processing (1)
  • crates/ourios-querier/src/log_row.rs
📝 Walkthrough

Walkthrough

Adds an explicit LogBody::Absent variant to distinguish absent bodies from empty-string bodies in query rendering. render_log_body returns this variant early for absent bodies. The server's LogRowDto maps Absent to None, omitting the body JSON key. Tests cover both layers.

Changes

Absent-body rendering and DTO handling

Layer / File(s) Summary
LogBody::Absent variant and render_log_body
crates/ourios-querier/src/log_row.rs
Adds LogBody::Absent variant with updated docs; render_log_body returns it immediately for BodyKind::Absent records instead of falling through to rendering.
Querier integration test
crates/ourios-querier/tests/rfc0025_rendering.rs
Replaces an ignored test stub with an active test verifying empty-string bodies render empty lines while absent bodies yield LogBody::Absent.
Server DTO optional body field
crates/ourios-server/src/querier.rs
LogRowDto.body becomes Option<LogBodyDto> with skip_serializing_if; conversion maps LogBody::Absent to None.
Server DTO serialization tests
crates/ourios-server/src/querier.rs
Adds tests confirming JSON omits body for absent bodies but retains it for empty-string bodies.

Estimated code review effort: 2 (Simple) | ~12 minutes

Possibly related PRs

  • jensholdgaard/ourios#167: Earlier BodyKind::Absent handling in miner reconstruction aligns with this PR's rendering contract for absent bodies.
  • jensholdgaard/ourios#275: Introduced the initial LogBody/render_log_body contract that this PR extends with the Absent variant.
🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is relevant and detailed, but it does not follow the required Summary/Related/Checklist template. Add the required ## Summary, ## Related, and ## Checklist sections, including the RFC or issue link and checklist items.
Linked Issues check ⚠️ Warning The PR only updates querier/server rendering and DTO omission; it does not add the required schema support or sink quarantine for #362. Implement the on-disk BodyKind::Absent representation and the partition-buffer quarantine fix, then update the property test.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately summarizes the main change: distinguishing absent log bodies from empty ones.
Out of Scope Changes check ✅ Passed No clearly out-of-scope changes are evident; the tests and DTO updates support the absent-vs-empty rendering work.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch rfc0025-green-rendering

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

This PR implements RFC 0025 green slice .3 in the read/query path by ensuring the system distinguishes an absent log body from an empty-string body, and reflects that distinction in both querier rendering and server JSON output.

Changes:

  • Add LogBody::Absent and make render_log_body return it immediately for BodyKind::Absent.
  • Update the server’s HTTP row DTO to represent body as Option and omit the JSON key entirely when absent (skip_serializing_if).
  • Add coverage in both querier integration tests (full query path) and server unit tests (DTO JSON shape) to pin the absent-vs-empty behavior.

Reviewed changes

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

File Description
crates/ourios-server/src/querier.rs Makes body optional in the response DTO and omits it for absent-body rows; adds a unit test asserting the JSON key omission.
crates/ourios-querier/tests/rfc0025_rendering.rs Implements RFC0025.3 integration test to ensure query results distinguish empty-string bodies from absent bodies end-to-end.
crates/ourios-querier/src/log_row.rs Introduces LogBody::Absent and updates render_log_body to short-circuit for BodyKind::Absent.

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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-querier/src/log_row.rs`:
- Around line 147-158: Add a co-located unit test in the log_row.rs test module
to cover the new BodyKind::Absent early return in render_log_body. Reuse the
existing record(body_kind: BodyKind) helper already present in this file’s
tests, construct a record with BodyKind::Absent and a registry, and assert that
render_log_body returns LogBody::Absent. Keep the test next to render_log_body
so the branch is directly covered here rather than only by rfc0025_rendering.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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 13c8cb71-96c2-4a09-b0ea-3f2dc2ed30b1

📥 Commits

Reviewing files that changed from the base of the PR and between eb28c7c and 6b2f73f.

📒 Files selected for processing (3)
  • crates/ourios-querier/src/log_row.rs
  • crates/ourios-querier/tests/rfc0025_rendering.rs
  • crates/ourios-server/src/querier.rs

Comment thread crates/ourios-querier/src/log_row.rs
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 1971712 into main Jul 6, 2026
22 checks passed
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.

Absent-body OTLP records permanently wedge their partition buffer (UnsupportedAbsentBody)

2 participants