fix(querier)!: unspecified severity bypasses a minimum-severity floor (RFC0002.21) by jensholdgaard · Pull Request #641 · jensholdgaard/ourios

jensholdgaard · 2026-07-25T20:26:44Z

Summary

Ourios compiled severity ordering to a bare numeric comparison, so a floor like severity >= trace excluded records with SeverityNumber = 0. That is the inverse of the OTel Logs SDK:

If a log record's SeverityNumber is specified (i.e. not 0) and is less than the configured minimum_severity, the log record MUST be dropped … Log records with an unspecified severity (i.e. 0) are not affected by this parameter and therefore bypass minimum severity filtering.
— Logs SDK, LoggerConfig

And the data model sanctions the special case directly:

Special handling MAY be given to SeverityNumber=0 when it is used to represent an unspecified severity.
— Logs Data Model, Comparing Severity

Being the inverse of the reference SDK isn't defensible for a backend that presents itself as OTLP-native, so floors now admit unspecified records. Observed against real agent telemetry: severity >= trace returned zero rows, because every Claude Code GenAI event carries 0 and every row group pruned.

Adds the §6.1 amendment + RFC0002.21 to RFC 0002 — this is a DSL contract change, not a patch.

Two consequences that are easy to get wrong

1. Ceilings must exclude unspecified. With only the floor bypass added, an unspecified row matched both >= error and < error (0 < 17 is numerically true), breaking the partition property a query language can't give up. My first implementation had exactly this bug and its own test caught it — the fix compiles </<= as severity_number != 0 AND ….

2. It's a pruning-correctness issue, not just UX. The rule is compiled into the predicate as a disjunction, not applied after the scan. A row group whose severity range is entirely 0 is therefore no longer prunable by a floor. A post-filter would have left the old min/max pruning in place and silently skipped whole files of unspecified rows. There's a dedicated test for this.

An explicit threshold of 0 keeps ordinary semantics, so severity > 0 still means "has a specified severity" rather than absurdly matching rows that have none.

Breaking change

severity >= X / > X now match SeverityNumber = 0; severity < X / <= X no longer do. Queries relying on the old behaviour change results. Pre-1.0, so a minor bump.

Verification

3 new tests (rfc0002_21_unspecified_severity.rs): floor admits + ceiling excludes + partition holds; explicit-0 threshold; row-groups-not-pruned.
219 existing tests pass unchanged — notably RFC0018.6 (out-of-range monotonicity, 25/200 matching >= error) still holds. No existing test pinned 0 behaviour, so this is a semantics addition rather than a test rewrite.
cargo clippy --all-targets --all-features -- -D warnings clean; cargo fmt --check clean; mdbook build clean.

Checklist

cargo fmt clean
cargo clippy clean
Tests added (3), full suite green
RFC 0002 amended (§6.1 + RFC0002.21) — contract change documented
CHANGELOG.md — generated at release from the conventional-commit ! marker

Summary by CodeRabbit

New Features
- Updated severity filtering so unspecified severity values are included in minimum-severity comparisons.
- Preserved explicit threshold behavior: severity > 0 matches only records with a specified severity.
- Ensured records with unspecified severity remain discoverable during query optimization.
Documentation
- Documented the revised severity comparison semantics and row-group matching behavior.
Tests
- Added integration coverage for floor, ceiling, explicit threshold, and unspecified-severity queries.

… (RFC0002.21) Ourios compiled `severity` ordering to a bare numeric comparison, so a floor like `severity >= trace` EXCLUDED records with SeverityNumber 0. That is the inverse of the OTel Logs SDK, whose minimum_severity drops a record only when its SeverityNumber "is specified (i.e. not 0)" -- unspecified records "bypass minimum severity filtering". The data model sanctions the special case directly: "Special handling MAY be given to SeverityNumber=0 ... in less-than / greater-than comparisons". Being the inverse of the reference SDK is not defensible for a backend that presents itself as OTLP-native, so floors now admit unspecified records. Observed against real agent telemetry: `severity >= trace` returned zero rows because every GenAI event carries 0 and every row group pruned. Two consequences that are easy to get wrong, both pinned by tests: - Ceilings (`<`/`<=`) must EXCLUDE unspecified, or the bypass makes a row match both `>= error` and `< error` (0 < 17 is numerically true). The first implementation missed this and its own test caught it. A predicate and its negation must still partition. - The rule is compiled INTO the predicate, not applied after the scan, so a row group whose severity range is entirely 0 is no longer prunable by a floor. A post-filter would have left the old min/max pruning in place and silently skipped whole files -- a correctness bug, not just a UX one. An explicit threshold of 0 keeps ordinary semantics, so `severity > 0` still means "has a specified severity". BREAKING CHANGE: `severity >= X` and `severity > X` now match records with SeverityNumber 0 (unspecified); `severity < X` and `severity <= X` no longer match them. Queries relying on the previous behaviour change results. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qtny6z6cA74xPZa4qRhk4F Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>

coderabbitai · 2026-07-25T20:27:02Z

Warning

Review limit reached

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

Next review available in: 13 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: 75aa461a-52a7-4060-8117-d3389e35aa4a

📥 Commits

Reviewing files that changed from the base of the PR and between 1dd9411 and 57db7e8.

📒 Files selected for processing (3)

crates/ourios-bench/tests/rfc0024_calibration.rs
crates/ourios-querier/src/compile.rs
crates/ourios-querier/tests/it/rfc0024_properties.rs

📝 Walkthrough

Walkthrough

Severity predicate compilation now treats SeverityNumber = 0 explicitly for ordering comparisons. Integration tests verify matching, complement, explicit-zero, and row-group scanning behavior, and RFC0002.21 documents the updated semantics.

Changes

Unspecified severity semantics

Layer / File(s)	Summary
Compile unspecified severity comparisons `crates/ourios-querier/src/compile.rs`	Threshold-based ordering predicates now include unspecified values for floors above zero and exclude them for ceilings above zero, while preserving ordinary semantics for an explicit zero threshold.
Validate and document RFC0002.21 `crates/ourios-querier/tests/it/*`, `docs/rfcs/0002-query-dsl.md`	Integration tests cover predicate results and row-group scanning, and RFC0002.21 records the updated severity behavior.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

jensholdgaard/ourios#146: Adds related DSL-to-execution severity comparison compilation, including severity-name flooring.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise and accurately captures the main change: unspecified severity now bypasses minimum-severity floors.
Description check	✅ Passed	The description matches the template with Summary, Related, and Checklist sections filled in with relevant details.
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.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch rfc0002-21-unspecified-severity

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

Pull request overview

Adjusts the query DSL’s severity ordering semantics to align with the OpenTelemetry Logs SDK/data model treatment of SeverityNumber = 0 (“unspecified”), ensuring minimum-severity floors don’t incorrectly drop unspecified records and preventing pruning from silently excluding entire row groups.

Changes:

Compile severity >= X / severity > X (for X > 0) as a disjunction that admits severity_number = 0 (unspecified).
Compile severity < X / severity <= X (for X > 0) to explicitly exclude severity_number = 0 to preserve partitioning (p vs not p).
Add an integration test suite for the new RFC0002.21 semantics, including a pruning-correctness guard, and document the contract change in RFC 0002.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
docs/rfcs/0002-query-dsl.md	Documents the §6.1 amendment and adds RFC0002.21 acceptance criteria for unspecified-severity ordering semantics.
crates/ourios-querier/src/compile.rs	Updates severity predicate compilation to implement RFC0002.21 (floor bypass + ceiling exclusion) in the pushed-down predicate.
crates/ourios-querier/tests/it/rfc0002_21_unspecified_severity.rs	Adds coverage for floor/ceiling behavior, explicit `0` thresholds, and a pruning regression test.
crates/ourios-querier/tests/it/main.rs	Registers the new RFC0002.21 integration test module.

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

coderabbitai

Actionable comments posted: 2

🤖 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/compile.rs`:
- Around line 774-809: Add colocated unit tests beside compile_severity covering
predicate lowering for positive-threshold `>` and `<=` comparisons, plus the
zero-threshold fallback path. Assert the generated predicates preserve
unspecified-severity handling and the existing ord_expr behavior, using the
surrounding compile-test conventions.

In `@docs/rfcs/0002-query-dsl.md`:
- Around line 453-470: Reformat the RFC0002.21 acceptance scenario in the §5
acceptance-criteria section using the documented blockquote convention: add a `>
**Scenario …**` heading and prefix each Given/When/Then/And line with `>`. Keep
a blank separator from adjacent scenarios and preserve the scenario’s existing
behavior and wording.

🪄 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: 0f648993-b030-4eac-a2e5-8363790adf98

📥 Commits

Reviewing files that changed from the base of the PR and between a2e61e5 and 1dd9411.

📒 Files selected for processing (4)

crates/ourios-querier/src/compile.rs
crates/ourios-querier/tests/it/main.rs
crates/ourios-querier/tests/it/rfc0002_21_unspecified_severity.rs
docs/rfcs/0002-query-dsl.md

CodeRabbit on PR #641: the new semantics were covered only end-to-end, but CLAUDE.md §6.2 wants unit tests next to the code. Added two in compile.rs's own module, asserting the LOWERED predicate rather than query results, so a regression is caught at the compile step even if no fixture happens to hold a severity-0 row: floors gain the `= 0` disjunct (which is also what defeats min/max pruning), ceilings gain the `!= 0` conjunct, a 0 threshold lowers to a plain comparison, and band membership (`==`/`!=`) is untouched. Copilot: the constant's doc comment opened with SEVERITY_NUMBER_UNSPECIFIED while the constant is SEVERITY_UNSPECIFIED, reading as a copy/paste slip. Reworded so it is clear the long form is the OTel proto enum symbol. Also fixes a mistake in the first version of this commit: inserting the new tests between an existing `#[test]` and its function left the attribute stranded (duplicated on mine, absent on duration_nanos_covers_all_units, which silently stopped being a test). Both restored; verified it runs again. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qtny6z6cA74xPZa4qRhk4F Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Copilot on PR #641: the test claimed to cover band membership but passed SeverityValue::Number(17), which lowers through the exact-comparison path -- so the bare-name band (error => 17..=20), the form the DSL actually encourages, was never exercised. Now covers both forms, and additionally asserts `== error` really lowers to the 17..=20 range, so the test cannot silently degrade into an exact compare and keep passing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qtny6z6cA74xPZa4qRhk4F Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

crates/ourios-querier/src/compile.rs:1263

After switching the test to operate on Expr structurally (rather than strings), this 0-threshold check should also assert structurally that the top-level operator is not AND/OR, instead of checking for formatted substrings.

        // A `0` threshold is a question *about* unspecified, not a floor, so
        // `severity > 0` keeps meaning "has a specified severity".
        for op in [OrdOp::Ge, OrdOp::Gt, OrdOp::Lt, OrdOp::Le] {
            let e = lowered(op, SeverityValue::Number(0));
            assert!(

codecov · 2026-07-25T20:52:18Z

❌ 1 Tests Failed:

Tests completed	Failed	Passed	Skipped
1264	1	1263	0

View the top 1 failed test(s) by shortest run time

ourios-bench::rfc0024_calibration::umbrella::rfc0024_7_adversarial_mode_passes_the_full_property_set

Stack Traces | 105s run time

thread 'umbrella::rfc0024_7_adversarial_mode_passes_the_full_property_set' (5221) panicked at .../ourios-bench/tests/rfc0024_calibration.rs:392:5:
Test failed: assertion failed: `(left == right)` 
  left: `1`, 
 right: `0`: oracle disagreement on `severity >= 17` at .../ourios-bench/tests/rfc0024_calibration.rs:359.
minimal failing input: batch = [
    OtlpLogRecord {
        tenant_id: TenantId(
            "rfc0024-testgen",
        ),
        time_unix_nano: 1767225600000000000,
        observed_time_unix_nano: None,
        severity_number: 0,
        severity_text: None,
        scope_name: Some(
            "Ѩ",
        ),
        scope_version: Some(
            "",
        ),
        scope_attributes: [],
        resource_schema_url: None,
        scope_schema_url: Some(
            "",
        ),
        attributes: [
            KeyValue {
                key: "w0hph0kscff6qt0vh",
                value: Some(
                    AnyValue {
                        value: Some(
                            KvlistValue(
                                KeyValueList {
                                    values: [
                                        KeyValue {
                                            key: ".lkxm8hs4ai.w7v",
                                            value: Some(
                                                AnyValue {
                                                    value: Some(
                                                        KvlistValue(
                                                            KeyValueList {
                                                                values: [
                                                                    KeyValue {
                                                                        key: "g5d4eb9.d8w..pvy",
                                                                        value: Some(
                                                                            AnyValue {
                                                                                value: Some(
                                                                                    DoubleValue(
                                                                                        -1.580780072713666e-308,
                                                                                    ),
                                                                                ),
                                                                            },
                                                                        ),
                                                                        key_strindex: 0,
                                                                    },
                                                                    KeyValue {
                                                                        key: "8m",
                                                                        value: Some(
                                                                            AnyValue {
                                                                                value: Some(
                                                                                    DoubleValue(
                                                                                        -3.8582165892639153e306,
                                                                                    ),
                                                                                ),
                                                                            },
                                                                        ),
                                                                        key_strindex: 0,
                                                                    },
                                                                    KeyValue {
                                                                        key: "",
                                                                        value: Some(
                                                                            AnyValue {
                                                                                value: Some(
                                                                                    DoubleValue(
                                                                                        -2.6178596109780736e230,
                                                                                    ),
                                                                                ),
                                                                            },
                                                                        ),
                                                                        key_strindex: 0,
                                                                    },
                                                                ],
                                                            },
                                                        ),
                                                    ),
                                                },
                                            ),
                                            key_strindex: 0,
                                        },
                                        KeyValue {
                                            key: "b26e",
                                            value: None,
                                            key_strindex: 0,
                                        },
                                        KeyValue {
                                            key: "o.d.acr._ca8.rvad_p",
                                            value: None,
                                            key_strindex: 0,
                                        },
                                    ],
                                },
                            ),
                        ),
                    },
                ),
                key_strindex: 0,
            },
            KeyValue {
                key: "ke__q",
                value: Some(
                    AnyValue {
                        value: Some(
                            ArrayValue(
                                ArrayValue {
                                    values: [
                                        AnyValue {
                                            value: Some(
                                                ArrayValue(
                                                    ArrayValue {
                                                        values: [
                                                            AnyValue {
                                                                value: Some(
                                                                    BytesValue(
                                                                        [
                                                                            101,
                                                                            43,
                                                                            201,
                                                                            112,
                                                                            4,
                                                                            139,
                                                                            187,
                                                                            65,
                                                                            188,
                                                                            199,
                                                                            89,
                                                                            224,
                                                                            152,
                                                                            46,
                                                                            233,
                                                                            244,
                                                                            71,
                                                                            175,
                                                                            190,
                                                                            223,
                                                                            131,
                                                                            79,
                                                                            37,
                                                                            241,
                                                                            85,
                                                                            200,
                                                                            97,
                                                                            202,
                                                                        ],
                                                                    ),
                                                                ),
                                                            },
                                                        ],
                                                    },
                                                ),
                                            ),
                                        },
                                        AnyValue {
                                            value: Some(
                                                ArrayValue(
                                                    ArrayValue {
                                                        values: [
                                                            AnyValue {
                                                                value: Some(
                                                                    StringValue(
                                                                        "\u{f491}\u{8c}:\u{feff}\u{57edf}'`\u{6ba3b}\u{8a119}\u{bf64e}\u{b}<'¥?",
                                                                    ),
                                                                ),
                                                            },
                                                        ],
                                                    },
                                                ),
                                            ),
                                        },
                                        AnyValue {
                                            value: Some(
                                                KvlistValue(
                                                    KeyValueList {
                                                        values: [
                                                            KeyValue {
                                                                key: "G``L?¥/.",
                                                                value: None,
                                                                key_strindex: 0,
                                                            },
                                                        ],
                                                    },
                                                ),
                                            ),
                                        },
                                        AnyValue {
                                            value: Some(
                                                KvlistValue(
                                                    KeyValueList {
                                                        values: [
                                                            KeyValue {
                                                                key: "\r`u:/\u{10cb9b}\\",
                                                                value: None,
                                                                key_strindex: 0,
                                                            },
                                                            KeyValue {
                                                                key: "\t\tTP\u{fd88e}\u{103a89}",
                                                                value: Some(
                                                                    AnyValue {
                                                                        value: None,
                                                                    },
                                                                ),
                                                                key_strindex: 0,
                                                            },
                                                            KeyValue {
                                                                key: "аdmin.key",
                                                                value: None,
                                                                key_strindex: 0,
                                                            },
                                                            KeyValue {
                                                                key: "yus.0bvo1vv._7c",
                                                                value: Some(
                                                                    AnyValue {
                                                                        value: Some(
                                                                            StringValue(
                                                                                "?Ѩ\\\u{202e}\u{c26d2})/c\u{4aafb}\u{f01cb}J\u{59724}慛&\u{6b2dc}&𳄶\u{477a3}\u{e0a8b}?(\r",
                                                                            ),
                                                                        ),
                                                                    },
                                                                ),
                                                                key_strindex: 0,
                                                            },
                                                        ],
                                                    },
                                                ),
                                            ),
                                        },
                                    ],
                                },
                            ),
                        ),
                    },
                ),
                key_strindex: 0,
            },
        ],
        dropped_attributes_count: 0,
        resource_attributes: [],
        trace_id: None,
        span_id: Some(
            [
                81,
                83,
                23,
                138,
                50,
                213,
                1,
                239,
            ],
        ),
        flags: 0,
        event_name: None,
        body: Some(
            Structured(
                AnyValue {
                    value: Some(
                        ArrayValue(
                            ArrayValue {
                                values: [
                                    AnyValue {
                                        value: Some(
                                            KvlistValue(
                                                KeyValueList {
                                                    values: [],
                                                },
                                            ),
                                        ),
                                    },
                                    AnyValue {
                                        value: Some(
                                            IntValue(
                                                4882943992017001042,
                                            ),
                                        ),
                                    },
                                    AnyValue {
                                        value: Some(
                                            KvlistValue(
                                                KeyValueList {
                                                    values: [
                                                        KeyValue {
                                                            key: ".f5wbhfpri43_hqb",
                                                            value: Some(
                                                                AnyValue {
                                                                    value: Some(
                                                                        KvlistValue(
                                                                            KeyValueList {
                                                                                values: [
                                                                                    KeyValue {
                                                                                        key: "hpko_2_.uw",
                                                                                        value: None,
                                                                                        key_strindex: 0,
                                                                                    },
                                                                                    KeyValue {
                                                                                        key: "yf3afwzkr_dtqhdmtvhuwk",
                                                                                        value: None,
                                                                                        key_strindex: 0,
                                                                                    },
                                                                                ],
                                                                            },
                                                                        ),
                                                                    ),
                                                                },
                                                            ),
                                                            key_strindex: 0,
                                                        },
                                                    ],
                                                },
                                            ),
                                        ),
                                    },
                                ],
                            },
                        ),
                    ),
                },
            ),
        ),
    },
]
successes: 44
local rejects: 0
global rejects: 0

note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

To view more test analytics, go to the Test Analytics Dashboard
_{📋 Got 3 mins? Take this short survey to help us improve Test Analytics.}

…02.21) CI caught what the querier's own suite did not: two naive oracles encode the pre-amendment severity semantics, so RFC0002.21 made them assert behaviour we deliberately changed. - ourios-bench rfc0024_calibration: `severity >= 17` was mirrored as `r.severity_number >= 17`. Adversarial mode generates severity-0 records, so this is the arm that failed (rfc0024_7). - ourios-querier rfc0024_properties: `SeverityGe(n)` likewise. It did not fail because its generated data happens not to hit the case, which is precisely why it is worth correcting now rather than when it surfaces later. Both now encode the floor contract: a threshold above 0 also admits unspecified severity; an explicit 0 threshold keeps ordinary semantics. Per CLAUDE.md §6.2 this is a contract change, not a test weakened for convenience -- the amended contract is RFC0002.21 in this same PR, and the oracles are being brought up to it rather than relaxed. Neither assertion loses strength: both still compare the querier against an independent naive evaluation, now of the correct semantics. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qtny6z6cA74xPZa4qRhk4F Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>

Copilot

Pull request overview

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

…isplay Copilot on PR #641: the lowering tests asserted on `format!("{e}")` with substring checks like " OR " and "severity_number = Int64(0)". DataFusion's Display output is not a stable API, and this workspace actively tracks DataFusion upgrades (RFC 0021) -- so a pure formatting change upstream could fail these tests with no semantic change, which is the worst kind of red. Now matches the Expr tree: top-level Operator::{Or,And}, and a `severity_number <Eq|NotEq> Int64(0)` arm identified by walking the tree with TreeNode::apply. The band assertion likewise collects Int64 literals and looks for 17 and 20 rather than reading them out of a rendered string. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qtny6z6cA74xPZa4qRhk4F Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>

Copilot

Pull request overview

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

§3.4 opened as "needs a decision either way" and presented three options. It has since been decided and merged on its own (#641): floors admit unspecified severity, ceilings exclude it, the rule is compiled into the predicate so pruning agrees, and an explicit 0 threshold keeps ordinary semantics. Rewritten as a record of what shipped rather than an open question, with the §7 checklist item closed and the status banner noting that one finding landed independently. The rest of the RFC -- whether to build a plugin at all, and for which host -- is unchanged and still open. Keeping the section rather than deleting it: the spike is what surfaced the defect, and it is the clearest example of a class of bug only a dashboard client exposes, which is itself an argument the RFC makes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qtny6z6cA74xPZa4qRhk4F Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>

…erses) (#640) * docs(rfc): draft RFC 0041 — dashboard datasource plugins (Grafana / Perses) Ourios's query API is stable (RFC 0002 + 0016 green) and nothing consumes it but curl, MCP and the bench harness. The DSL was explicitly shaped for dashboard authors -- RFC 0002 §3.6 names Perses as the primary audience and RFC0002.10 property-tests YAML-embeddability -- but no dashboard tool can issue a query without a plugin. Drafted, not specified: §§1-4 and §§7-8 per the lifecycle, with §5/§6 left empty on purpose. The open question is whether this is worth doing and where, not how. §7 puts "is this worth doing now?" first and names the competing calls (RFC 0036 implementation, the D1/D2 soak cadence, agent-observability). Both hosts were spiked to a rendered dashboard against the live querier before drafting, so §3.1's numbers are measured: 1-2 days for Grafana, 3-5 for Perses. The Ourios-side mapping is identical across them and ported in ~20 minutes; effectively all cost is each host's plugin system. One finding is Ourios-side and wants deciding either way (§3.4): a natural `severity >= trace` query returns zero rows against real agent telemetry, because GenAI events carry severity_number 0 and every row group prunes. Faithful storage, but a dashboard makes it look like a broken datasource. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qtny6z6cA74xPZa4qRhk4F Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org> * docs(rfc): ground RFC 0041's severity question in the OTel logs spec Consulted the OTel spec on SeverityNumber=0 rather than leaving §3.4 as "this wants a decision". The spec addresses it directly and turns an open worry into three options with upstream precedent: - Logs Data Model, Comparing Severity: "Special handling MAY be given to SeverityNumber=0 when it is used to represent an unspecified severity" -- the spec anticipates exactly this case in >= comparisons. - Logs SDK LoggerConfig: records with unspecified severity "bypass minimum severity filtering". OTel's own filter INCLUDES them; ours excludes them, so today's behaviour is the inverse of the SDK's. - Logs Data Model, Severity Fields: a backend "may interpret" missing severity as INFO(9) -- explicitly permitted, but invents a value the source did not send, so it is defensible at query time and not at storage time. - The Collector's attributesprocessor exposes match_undefined, precedent for making the choice explicit rather than implicit. Also records that this is a pruning-correctness question, not only UX: whichever semantics win, row-group pruning must agree or a filter will skip files containing matching rows. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qtny6z6cA74xPZa4qRhk4F Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org> * docs(rfc): record RFC 0041's severity finding as resolved by RFC0002.21 §3.4 opened as "needs a decision either way" and presented three options. It has since been decided and merged on its own (#641): floors admit unspecified severity, ceilings exclude it, the rule is compiled into the predicate so pruning agrees, and an explicit 0 threshold keeps ordinary semantics. Rewritten as a record of what shipped rather than an open question, with the §7 checklist item closed and the status banner noting that one finding landed independently. The rest of the RFC -- whether to build a plugin at all, and for which host -- is unchanged and still open. Keeping the section rather than deleting it: the spike is what surfaced the defect, and it is the clearest example of a class of bug only a dashboard client exposes, which is itself an argument the RFC makes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qtny6z6cA74xPZa4qRhk4F Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org> * docs(rfc): cite the half-open range contract in RFC 0041; normalise CUE CodeRabbit asked for the `range(...)` boundary contract to be documented, on the premise that RFC 0002 leaves it unspecified. It does not -- §6.2 already fixes `range(from, to)` as `from <= effective < to`, matching RFC 0010's [from, to). But the useful half of the point stands: RFC 0041 never cited it, and both pickers hand over an inclusive-looking `to`, so a plugin author would otherwise have to infer that a row exactly on the upper bound is excluded. Now stated in §3.2 alongside the existing-range-wins rule, so the two plugins cannot drift apart on it. Copilot: the document spelled the schema language both "Cue" and "Cuelang". Normalised to CUE throughout. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qtny6z6cA74xPZa4qRhk4F Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org> * docs(rfc): reconcile RFC 0041's Perses plugin count with its scope Copilot on PR #640: the §3.1 table said Perses time series "needs a third plugin" one row above "Plugins to write: 2", which is internally inconsistent as written. Both figures were true at different scopes, which is exactly the confusion. Split the row: logs parity is 1 plugin on Grafana and 2 on Perses; logs AND time series is still 1 on Grafana and 3 on Perses. Added the underlying asymmetry in prose, since it is the sharpest structural difference between the hosts -- a Grafana datasource picks its frame from the response shape, while Perses splits LogQuery from TimeSeriesQuery. Also scoped the effort figures (§1 and the table row) to log parity, so 1-2 vs 3-5 days is not read as covering equal capability: Grafana's number already includes time series, Perses's does not. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qtny6z6cA74xPZa4qRhk4F Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org> * docs(rfc): fix an unresolvable reference and a stale priority in RFC 0041 §3.4 cited a private note slug that exists nowhere in the repo, so the fidelity rationale was unresolvable for any reader; point at RFC 0018's faithful-witness wording instead. §7 listed RFC 0036's implementation as a competing call on time. RFC 0036 is accepted with its §5 criteria green — the claim was inherited from a stale roadmap entry (docs/roadmap.md, tracked separately). Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org> --------- Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

* docs(roadmap): refresh §3 through RFC 0041 (closes #642) The RFC 0036 bullet under "What's actually open" described it as `specified` with design review gating `red`, months after #593 flipped it to `accepted` and #592/#594/#595 landed the implementation. The roadmap is what a contributor reads to pick up work, so the entry actively misdirected — it already did, on #640. Checking the neighbours for the same drift found the file had gone stale wholesale against its own §6 cadence rule (refresh whenever a merged PR materially changes §3): - ladder stopped at RFC 0036; adds 0037 (`green`), 0038/0039/0040 (`green`, the self-observability arc) and 0041 (`drafted`) - "all ten product crates" predates `-config`, `-df-otel` and `-testgen`; now twelve product crates plus one dev-only - the `-telemetry` bullet described a metrics-only export surface, which stopped being true at RFC 0038/0039 - §5's Perses row still read "prerequisite is clear, left for after RFC 0031" with no pointer to RFC 0041, which now works that question up for both hosts Also records the unreleased breaking change (#641) sitting on `main` behind v0.5.0, so whoever cuts the next tag doesn't call it a patch. Prior banner entries are left as written — they are point-in-time records; the 2026-07-21 entry gains a one-line note that its arc closed. Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org> * docs(roadmap): reconcile §5's telemetry row with the §3 refresh Review caught the exact failure this PR exists to fix: §3 now records RFC 0038/0039/0040 as green while §5's telemetry row still read "Traces deliberately deferred", leaving two contradictory statuses in one file. Also tightens the ourios-df-otel dependency claim — datafusion + opentelemetry are its *runtime* deps; it does carry dev-dependencies (opentelemetry_sdk, chrono, criterion). The load-bearing property is that no ourios-* crate is among them, so say that instead. Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org> --------- Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>

jensholdgaard requested a review from Copilot July 25, 2026 20:26

Copilot started reviewing on behalf of jensholdgaard July 25, 2026 20:27 View session