Skip to content

docs: add verification process spec - #1

Merged
jensholdgaard merged 2 commits into
mainfrom
docs/verification-spec
Apr 28, 2026
Merged

docs: add verification process spec#1
jensholdgaard merged 2 commits into
mainfrom
docs/verification-spec

Conversation

@jensholdgaard

Copy link
Copy Markdown
Owner

Summary

  • Adds docs/verification.md — the process spec connecting CLAUDE.md §3 invariants and hazards.md H-x hazards to RFC acceptance criteria, red/green tests, and Validated thesis-gates.
  • Defines the five-stage RFC maturity model (drafted | specified | red | green | validated) with accepted | rejected | superseded as terminals.
  • Specifies the scenario id grammar (H<n>.<m>, §3.<n>.<m>, RFC<NNNN>.<m>) and the greppability contract between scenarios in RFCs and #[test] doc comments.
  • §3.2 Outer loop vs. inner loop — clarifies the model is BDD/ATDD on the outside; classic Beck-style red→minimal→triangulate→refactor is the recommended inner loop inside the Red→Green transition, not mandated.
  • §6 Worked example — traces CLAUDE.md §3.1 No silent template merges + hazards.md H1 through three numbered scenarios (H1.1 / H1.2 / H1.3), crates/ourios-miner/tests/ red stubs, green tests, and benchmarks.md C2 as the Validated thesis-gate.
  • docs/SUMMARY.md gains a Verification entry under Architecture so the doc is reachable from the mdbook nav as soon as it lands.

The two Proposed amendment sections at the bottom of docs/verification.md (docs/rfcs/README.md and CLAUDE.md §5.6) are tracked in a follow-up PR — this PR only adds the spec and lights it up in the nav.

Invariants and hazards touched

The spec describes the process by which invariants and hazards become tested promises; it does not modify any §3 invariant or H-x hazard text. No mitigation is weakened.

Test plan

  • mdbook build — clean exit
  • Live mdbook serve — rebuilds cleanly through every edit
  • Reviewer reads §1–§6 end to end and confirms the worked example is faithful to RFC 0001 and hazards.md H1
  • Reviewer confirms the §3.2 outer/inner-loop split matches the team's intended methodology

🤖 Generated with Claude Code

Describes the path from a CLAUDE.md §3 invariant or hazards.md H-x
hazard to a passing test: scenarios in RFC §5, scenario id grammar
and greppability contract, the five-stage RFC maturity model
(drafted/specified/red/green/validated) with the Specified gate as
the most valuable, BDD/ATDD outer loop with Beck-style TDD as the
recommended inner loop, regression handling after Validated, and a
worked example tracing CLAUDE.md §3.1 / hazards.md H1 through to
benchmarks.md C2 against RFC 0001.

The proposed amendments at the bottom of the file (rfcs/README.md
and CLAUDE.md §5.6) land in a separate PR.

SUMMARY.md gains a Verification entry under Architecture so the doc
is reachable from the mdbook nav as soon as it lands.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@jensholdgaard
jensholdgaard merged commit f054047 into main Apr 28, 2026
7 checks passed
@jensholdgaard
jensholdgaard deleted the docs/verification-spec branch April 28, 2026 13:55
jensholdgaard added a commit that referenced this pull request Apr 28, 2026
* docs: add verification process spec

Describes the path from a CLAUDE.md §3 invariant or hazards.md H-x
hazard to a passing test: scenarios in RFC §5, scenario id grammar
and greppability contract, the five-stage RFC maturity model
(drafted/specified/red/green/validated) with the Specified gate as
the most valuable, BDD/ATDD outer loop with Beck-style TDD as the
recommended inner loop, regression handling after Validated, and a
worked example tracing CLAUDE.md §3.1 / hazards.md H1 through to
benchmarks.md C2 against RFC 0001.

The proposed amendments at the bottom of the file (rfcs/README.md
and CLAUDE.md §5.6) land in a separate PR.

SUMMARY.md gains a Verification entry under Architecture so the doc
is reachable from the mdbook nav as soon as it lands.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs: apply RFC maturity-model amendments

Applies the amendments proposed at the bottom of docs/verification.md
(landed in #1).

docs/rfcs/README.md
- status: value list expanded to drafted | specified | red | green |
  validated | accepted | rejected | superseded.
- New §5 Acceptance criteria in Required sections, with §5 Testing
  strategy → §6, Open questions → §7, References → §8.
- Lifecycle rewritten as the five-stage maturity model with superseded
  and rejected as terminals reachable from any stage.

CLAUDE.md
- New §5.6 Verification process — three-line cross-reference to the
  spec.

docs/rfcs/0001-template-miner.md, docs/rfcs/0002-query-dsl.md
- status: draft → drafted, applying the renamed maturity stage. No
  body changes; RFC 0001 picks up its §5 Acceptance criteria in a
  follow-up PR per docs/verification.md §6.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jensholdgaard added a commit that referenced this pull request May 10, 2026
Two §5 invariant scenarios go from #[ignore] + todo!() to real
assertions in the same commit, jumping the §5 count from 4/29
to 6/29. Pattern matches PR #11 (MinerConfig flipped 3 stubs).

This PR ships the multi-tenancy *shape* — TenantId, MinerCluster
with one TenantState per tenant, lazy per-tenant allocation —
but explicitly NOT the Drain tree. Per-tenant state is a
HashMap<Vec<String>, u64> keyed on the masked-token sequence
(exact-match templating). Future PRs replace the HashMap with
simSeq + the depth-bounded tree + widening (RFC 0001 §6.2 steps
3–5). The §3.7 isolation invariant is testable at this layer
because isolation is about *who owns which store*, not about how
the store clusters.

Implementation:

ourios-core::tenant::TenantId
  String-backed newtype (deferred u64 representation per the
  plan's design call #1: operator-facing slugs / UUIDs are
  String-shaped; column-store efficiency is a downstream concern
  the future ourios-parquet RFC will own). No validation yet —
  accept any string, future try_new can layer on top.
  AsRef<str> + Display so it composes with log lines + metric
  labels without ceremony.

ourios-miner::cluster::MinerCluster
  Public type holding a HashMap<TenantId, TenantState> and a
  *cluster-wide* template_id allocator. Tenant state allocated
  lazily on first ingest. Public API: new, config, ingest,
  template_count, templates_for. The latter two are test
  helpers but kept public per the plan's design call #4 (future
  operator-console-style tooling will want them; pub(crate)
  tightening is easy if we change our minds).

ourios-miner::cluster::TenantState
  Private struct holding only the templates HashMap. Future
  PRs swap this for the real tree.

Why the template_id allocator is cluster-wide, not per-tenant:

RFC 0001 §6.1 uses the phrase "per-tenant monotonic" but ALSO
requires that "two tenants emitting the structurally identical
template will have different template_ids", and §5 §3.7.2
requires "no template_id is shared across tenants." A truly
per-tenant allocator gives both tenants id=1 for their first
template and silently violates §3.7.2 (the test caught this on
first run — id_a == id_b == 1). Reconciliation: the id *space*
is cluster-wide, but each tenant's slice of that space is
monotonic with respect to that tenant's allocation order. Both
phrases hold:

  - "per-tenant monotonic" — given a tenant, the sequence of ids
    allocated *to* that tenant strictly increases over time
  - "different template_ids across tenants" — the shared
    allocator never hands out the same id twice

A code comment on next_template_id documents this so future
readers don't try to "fix" the cluster-wide allocator back into
per-tenant.

Tests:

§5 stub flips (AAA-structured per the new policy):

- §3.7.1 — Two tenants emit different shapes; interleaved
  ingest. Asserts on token-set membership: A's tree contains
  A-shape tokens, B's contains B-shape tokens, neither contains
  the other's. Cross-pollination would mean either set
  contained tokens that originated in the other tenant's input.
- §3.7.2 — Two tenants emit the structurally identical line.
  Asserts id_a != id_b (the bug the cluster-wide allocator
  fixes).

Cluster unit tests (in cluster.rs, AAA-structured):

- ingest_returns_same_template_id_for_repeat_shape — exact-
  match templating gives one id for "user 42 logged in" and
  "user 17 logged in" since both mask to the same shape.
- ingest_returns_distinct_template_ids_for_distinct_shapes —
  same tenant, different shapes → different ids.
- template_count_is_zero_for_unseen_tenant — unseen tenants
  return 0 / [], no panic.
- ingest_lazily_allocates_per_tenant_state — first ingest
  materialises the tenant state; before that, count is 0.

Cargo dependency change: ourios-miner promotes ourios-core from
[dev-dependencies] to [dependencies] — the cluster module now
imports ourios_core::config::MinerConfig and ourios_core::
tenant::TenantId from non-test code. The single dep entry covers
both production code and the integration tests in
tests/invariants.rs.

Lifecycle (per docs/verification.md §3 two-loop spec):

- Outer loop (cargo test --all-features):
    21 passed (was 15: + 4 cluster unit tests + 2 newly green
    §5 scenarios), 23 ignored (was 25, − 2 flipped)
- Inner loop (cargo test --no-fail-fast -- --ignored):
    23 failed, was 25
- §5 scenario count toward Green: 4/29 → 6/29

RFC 0001 stays at status: red (23 stubs to go).

What this PR is NOT:

- Not Drain — no simSeq, no depth-bounded tree, no widening.
  Future PR.
- No audit events, telemetry, body retention, lossy_flag.
  Future PR(s).
- No Parquet record emission. ourios-parquet's problem.
- No tenant lifecycle (TenantPaused, TenantDeleted, eviction).
  RFC §9 deferral, future PR.

Verification (CLAUDE.md §6.6): cargo fmt clean, cargo clippy
clean (-D warnings, --all-targets --all-features), cargo test
passing (21 / 23 split), mdbook build clean.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
jensholdgaard added a commit that referenced this pull request May 10, 2026
* feat(cluster): add MinerCluster — flips §3.7.1, §3.7.2

Two §5 invariant scenarios go from #[ignore] + todo!() to real
assertions in the same commit, jumping the §5 count from 4/29
to 6/29. Pattern matches PR #11 (MinerConfig flipped 3 stubs).

This PR ships the multi-tenancy *shape* — TenantId, MinerCluster
with one TenantState per tenant, lazy per-tenant allocation —
but explicitly NOT the Drain tree. Per-tenant state is a
HashMap<Vec<String>, u64> keyed on the masked-token sequence
(exact-match templating). Future PRs replace the HashMap with
simSeq + the depth-bounded tree + widening (RFC 0001 §6.2 steps
3–5). The §3.7 isolation invariant is testable at this layer
because isolation is about *who owns which store*, not about how
the store clusters.

Implementation:

ourios-core::tenant::TenantId
  String-backed newtype (deferred u64 representation per the
  plan's design call #1: operator-facing slugs / UUIDs are
  String-shaped; column-store efficiency is a downstream concern
  the future ourios-parquet RFC will own). No validation yet —
  accept any string, future try_new can layer on top.
  AsRef<str> + Display so it composes with log lines + metric
  labels without ceremony.

ourios-miner::cluster::MinerCluster
  Public type holding a HashMap<TenantId, TenantState> and a
  *cluster-wide* template_id allocator. Tenant state allocated
  lazily on first ingest. Public API: new, config, ingest,
  template_count, templates_for. The latter two are test
  helpers but kept public per the plan's design call #4 (future
  operator-console-style tooling will want them; pub(crate)
  tightening is easy if we change our minds).

ourios-miner::cluster::TenantState
  Private struct holding only the templates HashMap. Future
  PRs swap this for the real tree.

Why the template_id allocator is cluster-wide, not per-tenant:

RFC 0001 §6.1 uses the phrase "per-tenant monotonic" but ALSO
requires that "two tenants emitting the structurally identical
template will have different template_ids", and §5 §3.7.2
requires "no template_id is shared across tenants." A truly
per-tenant allocator gives both tenants id=1 for their first
template and silently violates §3.7.2 (the test caught this on
first run — id_a == id_b == 1). Reconciliation: the id *space*
is cluster-wide, but each tenant's slice of that space is
monotonic with respect to that tenant's allocation order. Both
phrases hold:

  - "per-tenant monotonic" — given a tenant, the sequence of ids
    allocated *to* that tenant strictly increases over time
  - "different template_ids across tenants" — the shared
    allocator never hands out the same id twice

A code comment on next_template_id documents this so future
readers don't try to "fix" the cluster-wide allocator back into
per-tenant.

Tests:

§5 stub flips (AAA-structured per the new policy):

- §3.7.1 — Two tenants emit different shapes; interleaved
  ingest. Asserts on token-set membership: A's tree contains
  A-shape tokens, B's contains B-shape tokens, neither contains
  the other's. Cross-pollination would mean either set
  contained tokens that originated in the other tenant's input.
- §3.7.2 — Two tenants emit the structurally identical line.
  Asserts id_a != id_b (the bug the cluster-wide allocator
  fixes).

Cluster unit tests (in cluster.rs, AAA-structured):

- ingest_returns_same_template_id_for_repeat_shape — exact-
  match templating gives one id for "user 42 logged in" and
  "user 17 logged in" since both mask to the same shape.
- ingest_returns_distinct_template_ids_for_distinct_shapes —
  same tenant, different shapes → different ids.
- template_count_is_zero_for_unseen_tenant — unseen tenants
  return 0 / [], no panic.
- ingest_lazily_allocates_per_tenant_state — first ingest
  materialises the tenant state; before that, count is 0.

Cargo dependency change: ourios-miner promotes ourios-core from
[dev-dependencies] to [dependencies] — the cluster module now
imports ourios_core::config::MinerConfig and ourios_core::
tenant::TenantId from non-test code. The single dep entry covers
both production code and the integration tests in
tests/invariants.rs.

Lifecycle (per docs/verification.md §3 two-loop spec):

- Outer loop (cargo test --all-features):
    21 passed (was 15: + 4 cluster unit tests + 2 newly green
    §5 scenarios), 23 ignored (was 25, − 2 flipped)
- Inner loop (cargo test --no-fail-fast -- --ignored):
    23 failed, was 25
- §5 scenario count toward Green: 4/29 → 6/29

RFC 0001 stays at status: red (23 stubs to go).

What this PR is NOT:

- Not Drain — no simSeq, no depth-bounded tree, no widening.
  Future PR.
- No audit events, telemetry, body retention, lossy_flag.
  Future PR(s).
- No Parquet record emission. ourios-parquet's problem.
- No tenant lifecycle (TenantPaused, TenantDeleted, eviction).
  RFC §9 deferral, future PR.

Verification (CLAUDE.md §6.6): cargo fmt clean, cargo clippy
clean (-D warnings, --all-targets --all-features), cargo test
passing (21 / 23 split), mdbook build clean.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* docs(cluster): address PR #13 review — fix per-tenant-allocator drift

Two Copilot hits — both real doc/comment drift caused by my
mid-execution switch from per-tenant to cluster-wide template_id
allocator. The implementation went cluster-wide (rightly, per
team verdict + PR #14's RFC clarification) but two prose
artefacts kept describing the old per-tenant rationale:

- C1 (cluster.rs module docs): said "no shared template_id
  allocator" and "RFC 0001 §6.1's per-tenant monotonic
  template_id falls out of construction." The cluster-wide
  next_template_id field directly contradicts both clauses.
  Rewrite the opening paragraph to say what the code actually
  does: per-tenant template *stores* are isolated (no template
  ever crosses tenants), but the template_id allocator is
  cluster-wide so the same u64 never refers to two leaves; each
  tenant sees a monotonic *subsequence* of the shared id space.

- C2 (invariants.rs §3.7.2 test assertion comment): said
  "RFC 0001 §6.1's per-tenant template_id allocator gives each
  tenant its own monotonic id space, so the two ids are distinct
  by construction (each starts at 1)." This rationale is wrong
  for the cluster-wide allocator — the ids are distinct because
  the allocator never reuses values (id_a = 1, id_b = 2),
  not because each tenant has its own space starting at 1.
  Replace with the correct rationale: the second call pulls the
  *next* monotonic id rather than reusing the first tenant's id.

Both fixes are pure prose. No behaviour change. The tests
themselves still pass (the assertion logic was correct; only the
explanatory comment was stale).

Verification (CLAUDE.md §6.6): cargo fmt clean, cargo clippy
clean, cargo test passing (21 / 23 split unchanged).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
jensholdgaard added a commit that referenced this pull request May 14, 2026
Wires `MinerCluster::ingest` to the prefix tree skeleton from
PR #17. `MinerCluster::TenantState`'s `HashMap<Vec<String>, u64>`
placeholder is gone; per-tenant templates now live in a
`Tree`, walked via the read-only `Tree::descend` for the
existence-check phase of an ingest and `Tree::descend_mut` for
the allocate-and-insert path.

Attach decision: **exact-match only** — a candidate line attaches
to a leaf when `sim_seq(line, leaf.template) == 1.0`; anything
else creates a new leaf in the same `(length, prefix)` bucket.
The §6.2 step-5 widening branch and its §3.1 audit-event
invariant are deferred to the next PR; widening + audits land
together because §3.1 forbids merges without an audit event,
so they cannot be split.

Hazard #1 invariant story: this PR does no merges. Every line
either matches an existing template exactly or creates a fresh
leaf. §3.1 "no silent template merges" is preserved vacuously.
The new locking test
`ingest_creates_separate_leaves_for_near_match_under_same_parent`
pins this no-widening behaviour explicitly so the next PR's
review surfaces the contract change rather than silently editing
it away (CLAUDE.md §6.2 "Tests are specifications").

Tree-module additions (alongside the existing `descend_mut`):

- `Tree::descend(&self, masked, depth) -> Option<&PrefixNode>` —
  read-only counterpart for the lookup phase of an ingest.
- `Tree::leaf_count(&self) -> usize` — backs cluster
  `template_count`.
- `Tree::collect_leaves(&self) -> Vec<&Leaf>` — backs cluster
  `templates_for`.

Module docs on both `tree.rs` and `cluster.rs` updated to reflect
the new state (tree no longer described as "skeleton with no
caller"; cluster no longer described as "exact-match HashMap
placeholder").

`docs/roadmap.md` §3 refreshed: 6/29 §5 scenarios green; the
miner now has tree + descend + cluster integration; the C2
blocker is reduced to "widen + best-candidate selection," the
tree itself is done.

§5 scenarios flipped: 0 (no new acceptance criteria pass; this
PR is foundational wiring). Outer-loop test count: 40 → 48
passed (8 new — 7 tree, 1 cluster locking test); 23 ignored
unchanged.

Verification:
- `cargo fmt --all --check` — clean
- `cargo clippy --all-targets --all-features -- -D warnings` — clean
- `cargo test --all-features` — 48 passed / 23 ignored / 0 failed
- `mdbook build` — clean (`docs/roadmap.md` touched)
jensholdgaard added a commit that referenced this pull request May 14, 2026
…tch (#19)

* feat(miner): route MinerCluster through Drain tree + sim_seq exact-match

Wires `MinerCluster::ingest` to the prefix tree skeleton from
PR #17. `MinerCluster::TenantState`'s `HashMap<Vec<String>, u64>`
placeholder is gone; per-tenant templates now live in a
`Tree`, walked via the read-only `Tree::descend` for the
existence-check phase of an ingest and `Tree::descend_mut` for
the allocate-and-insert path.

Attach decision: **exact-match only** — a candidate line attaches
to a leaf when `sim_seq(line, leaf.template) == 1.0`; anything
else creates a new leaf in the same `(length, prefix)` bucket.
The §6.2 step-5 widening branch and its §3.1 audit-event
invariant are deferred to the next PR; widening + audits land
together because §3.1 forbids merges without an audit event,
so they cannot be split.

Hazard #1 invariant story: this PR does no merges. Every line
either matches an existing template exactly or creates a fresh
leaf. §3.1 "no silent template merges" is preserved vacuously.
The new locking test
`ingest_creates_separate_leaves_for_near_match_under_same_parent`
pins this no-widening behaviour explicitly so the next PR's
review surfaces the contract change rather than silently editing
it away (CLAUDE.md §6.2 "Tests are specifications").

Tree-module additions (alongside the existing `descend_mut`):

- `Tree::descend(&self, masked, depth) -> Option<&PrefixNode>` —
  read-only counterpart for the lookup phase of an ingest.
- `Tree::leaf_count(&self) -> usize` — backs cluster
  `template_count`.
- `Tree::collect_leaves(&self) -> Vec<&Leaf>` — backs cluster
  `templates_for`.

Module docs on both `tree.rs` and `cluster.rs` updated to reflect
the new state (tree no longer described as "skeleton with no
caller"; cluster no longer described as "exact-match HashMap
placeholder").

`docs/roadmap.md` §3 refreshed: 6/29 §5 scenarios green; the
miner now has tree + descend + cluster integration; the C2
blocker is reduced to "widen + best-candidate selection," the
tree itself is done.

§5 scenarios flipped: 0 (no new acceptance criteria pass; this
PR is foundational wiring). Outer-loop test count: 40 → 48
passed (8 new — 7 tree, 1 cluster locking test); 23 ignored
unchanged.

Verification:
- `cargo fmt --all --check` — clean
- `cargo clippy --all-targets --all-features -- -D warnings` — clean
- `cargo test --all-features` — 48 passed / 23 ignored / 0 failed
- `mdbook build` — clean (`docs/roadmap.md` touched)

* refactor(miner): flatten ingest lookup into Option/Iterator chain

The Phase-1 read-only lookup in `MinerCluster::ingest` was
nested three deep (`if let Some(state)` → `if let Some(parent)`
→ `for leaf in &parent.leaves`). Replace with an
`Option::and_then` + `Iterator::find` + `Option::map` chain so
the data flow reads top-to-bottom and the conditional return
sits at the same level as the rest of the function.

Behaviour-preserving and zero-cost: `and_then`, `find`, and
`map` are all `#[inline]`-able std methods, and rustc inlines
them with the closures in release builds — the chain compiles
to the same control flow as the original imperative form (match
on Option discriminant → match on Option discriminant → loop
with break on first match → field access). No allocation, no
dyn dispatch, no extra Option construction that survives.

Verification: cargo fmt clean, clippy clean, 48 passed / 23
ignored / 0 failed (counts unchanged).

* fix(miner): address PR #19 review — empty input + O(1) template_count

Three substantive Copilot inline comments on the cluster/tree
integration:

1. **Empty-input panic (cluster.rs:121,122 + tree.rs:181).**
   `tokenize` documents and tests inputs with no tokens
   (`""`, whitespace-only); `mask` preserves length, so a public
   `MinerCluster::ingest` call with such a line tripped
   `Tree::descend`/`descend_mut`'s `N ≥ 1` precondition and
   panicked. `ingest` now short-circuits on empty masked tokens
   and returns the new `NO_TEMPLATE` sentinel (`0`) — the value
   `MinerCluster::new` already reserved for "no template
   allocated" per its existing `next_template_id: 1` comment.
   Placeholder for the `parse_failures_total` metric path that
   lands with the §6.3 confidence-zone branching PR.

2. **`template_count` O(1) regression (cluster.rs:172).** The
   pre-tree `HashMap::len()` was O(1); the post-tree
   `Tree::leaf_count()` walks the whole tree on every call.
   Add a `template_count: usize` cache to `TenantState`,
   incremented after each leaf push in Phase 2. Reading from
   the cache restores O(1). `Tree::leaf_count` stays for
   tree-level introspection (still used by `tree::tests`); the
   cluster simply doesn't use it on the read path.

3. **Misleading panic message (tree.rs:181).** "tokenize+mask
   guarantee N ≥ 1" was factually wrong — tokenize explicitly
   supports zero-token inputs. Trimmed to the precondition
   statement alone (`should_panic` test expectations still
   match — they assert the substring "must be non-empty").

Two new tests:

- `ingest_returns_no_template_sentinel_for_empty_input` — pins
  the `""` and `"   \t\n"` paths against `NO_TEMPLATE` and
  confirms `template_count` stays 0.
- `template_count_grows_with_each_distinct_template` — pins
  the cache invariant: 3 ingests, 2 distinct shapes, count == 2.

Verification: cargo fmt clean, clippy clean, 50 passed
(was 48; +2 new cluster tests) / 23 ignored / 0 failed.
jensholdgaard added a commit that referenced this pull request May 19, 2026
Thirteen new substantive comments. Stale carry-overs #1 and #2
folded into the same commit since the summary's MinedRecord
reference and §3.4's tenant_id= naming were the lingering hooks.

3, 12. §3.8 vs §3.9 internal contradiction: §3.8 rule 1 forbids
   REQUIRED additive columns, but §3.9 defaulted them. Dropped the
   REQUIRED-added-in-amendment bullet from §3.9 with an explicit
   cross-reference to §3.8 rule 1.

4. URL-encoding spec was "URL-encoded" without a normative variant.
   Pinned to RFC 3986 percent-encoding with explicit overrides:
   UTF-8 byte input, no Unicode normalisation, escape everything
   outside the unreserved set (including `/`, `=`, `%`).
   Malformed-escape decoding is a hard read error.

5, 13. UUIDv7 conflict: §3.4 normatively required UUIDv7 but §7
   listed it as an open question. Resolved by pinning UUIDv7 in §3.4
   (matters for partition-lexsort = creation-order) and removing the
   open question.

6. RFC0005.7 referenced `TemplateWidened` (the Rust variant name)
   while RFC 0001 uses `template_widened` (snake_case event_type).
   Rewrote the Given clause to use the RFC 0001 string and
   parenthetically map to the Rust variant.

7. "§3.2 cardinality invariant" was ambiguous — §3.2 in this RFC is
   the schema, the invariant lives in CLAUDE.md §3.2 / hazards H2.
   Qualified all four cardinality-invariant references with
   `CLAUDE.md` §3.2.

8. TIMESTAMP logical type used `isAdjusted=true` (not a Parquet
   spec field). Replaced with `isAdjustedToUTC=true` (Parquet's
   actual flag name); two occurrences fixed via replace-all.

9. trace_id/span_id listed `BYTES` as Parquet logical type, which
   isn't a real Parquet logical type. Switched trace_id to `UUID`
   (matches 16-byte fixed-len) and span_id to "no logical type"
   (Parquet has no 8-byte opaque-id annotation; physical type alone
   is the contract).

10. §3.7 audit schema was missing RFC 0001 §6.4 fields. Added:
    - `triggering_line_hash` (FIXED_LEN_BYTE_ARRAY(16), blake3 of
      L_raw, REQUIRED)
    - `triggering_line_sample` (STRING, first 256 B of L_raw,
      OPTIONAL)
    - `slots_expanded` (LIST<STRUCT<slot_index, types_added:
      LIST<INT32>>>, REQUIRED) — replaces the singleton
      `type_added`/`slot_index` columns since RFC 0001 specifies
      `Vec<SlotExpansion>` (a single attach can grow multiple slots)

11. RFC 0001 §9 names `event_type` (snake_case STRING) as the
    drift-query predicate-pushdown column. RFC 0005 had replaced
    that with `event_kind` ordinal. Resolution: store BOTH — the
    INT32 ordinal for writer/reader internal use, and the STRING for
    RFC 0001's contract. Writer must keep them in sync per the
    mapping table.

14. RFC0005.7 said "full §3.7 payload" then enumerated a subset.
    Rephrased to "every row-level column declared in §3.7's audit-
    schema table" with explicit NULL-on-OPTIONAL semantics.

15. RFC0005.8's "file size doesn't balloon proportionally" clause
    wasn't testable. Replaced with a concrete assertion: the body
    column chunk's `dictionary_page_offset` is unset — no
    dictionary page exists on disk for the column.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jensholdgaard added a commit that referenced this pull request May 20, 2026
Thirteen new substantive comments. Stale carry-overs #1 and #2
folded into the same commit since the summary's MinedRecord
reference and §3.4's tenant_id= naming were the lingering hooks.

3, 12. §3.8 vs §3.9 internal contradiction: §3.8 rule 1 forbids
   REQUIRED additive columns, but §3.9 defaulted them. Dropped the
   REQUIRED-added-in-amendment bullet from §3.9 with an explicit
   cross-reference to §3.8 rule 1.

4. URL-encoding spec was "URL-encoded" without a normative variant.
   Pinned to RFC 3986 percent-encoding with explicit overrides:
   UTF-8 byte input, no Unicode normalisation, escape everything
   outside the unreserved set (including `/`, `=`, `%`).
   Malformed-escape decoding is a hard read error.

5, 13. UUIDv7 conflict: §3.4 normatively required UUIDv7 but §7
   listed it as an open question. Resolved by pinning UUIDv7 in §3.4
   (matters for partition-lexsort = creation-order) and removing the
   open question.

6. RFC0005.7 referenced `TemplateWidened` (the Rust variant name)
   while RFC 0001 uses `template_widened` (snake_case event_type).
   Rewrote the Given clause to use the RFC 0001 string and
   parenthetically map to the Rust variant.

7. "§3.2 cardinality invariant" was ambiguous — §3.2 in this RFC is
   the schema, the invariant lives in CLAUDE.md §3.2 / hazards H2.
   Qualified all four cardinality-invariant references with
   `CLAUDE.md` §3.2.

8. TIMESTAMP logical type used `isAdjusted=true` (not a Parquet
   spec field). Replaced with `isAdjustedToUTC=true` (Parquet's
   actual flag name); two occurrences fixed via replace-all.

9. trace_id/span_id listed `BYTES` as Parquet logical type, which
   isn't a real Parquet logical type. Switched trace_id to `UUID`
   (matches 16-byte fixed-len) and span_id to "no logical type"
   (Parquet has no 8-byte opaque-id annotation; physical type alone
   is the contract).

10. §3.7 audit schema was missing RFC 0001 §6.4 fields. Added:
    - `triggering_line_hash` (FIXED_LEN_BYTE_ARRAY(16), blake3 of
      L_raw, REQUIRED)
    - `triggering_line_sample` (STRING, first 256 B of L_raw,
      OPTIONAL)
    - `slots_expanded` (LIST<STRUCT<slot_index, types_added:
      LIST<INT32>>>, REQUIRED) — replaces the singleton
      `type_added`/`slot_index` columns since RFC 0001 specifies
      `Vec<SlotExpansion>` (a single attach can grow multiple slots)

11. RFC 0001 §9 names `event_type` (snake_case STRING) as the
    drift-query predicate-pushdown column. RFC 0005 had replaced
    that with `event_kind` ordinal. Resolution: store BOTH — the
    INT32 ordinal for writer/reader internal use, and the STRING for
    RFC 0001's contract. Writer must keep them in sync per the
    mapping table.

14. RFC0005.7 said "full §3.7 payload" then enumerated a subset.
    Rephrased to "every row-level column declared in §3.7's audit-
    schema table" with explicit NULL-on-OPTIONAL semantics.

15. RFC0005.8's "file size doesn't balloon proportionally" clause
    wasn't testable. Replaced with a concrete assertion: the body
    column chunk's `dictionary_page_offset` is unset — no
    dictionary page exists on disk for the column.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jensholdgaard added a commit that referenced this pull request May 20, 2026
…am) (#41)

* docs(rfc): add RFC 0005 — Parquet storage (schema, writer, audit stream)

Opens Phase 2 with the on-disk schema RFC the CLAUDE.md §3.5 invariant
requires before any ourios-parquet code lands. Pins:

- The data-file Parquet schema (column-by-column mapping of RFC 0001
  §6.1's MinedRecord onto Parquet types, with tenant + time as
  Hive-style partition keys).
- The AnyValue encoding rule (OTLP-canonical JSON in a BYTE_ARRAY,
  not a recursive typed STRUCT — rejected for MVP because Parquet's
  flat-nested model can't represent the discriminated union's
  recursion faithfully without capping depth).
- The audit-event file schema — the cross-RFC contract from RFC 0001
  §9 that ties §6.4 widening events to a separate file series.
- The writer's row-group / file-size targets per hazards.md H4, the
  compression codec (ZSTD-3), and the per-column encoding policy
  (notably no dictionary on the body column — the §3.2 cardinality
  invariant forbids it).
- The reader's forward- and backward-compatibility contract (unknown
  columns ignored, missing OPTIONAL columns surface as None, missing
  baseline REQUIRED columns error).
- The schema-evolution rules anchored to CLAUDE.md §3.5.

Status: drafted. §5 acceptance criteria are written so reviewers can
flip to specified if the criteria pass review; a follow-on code PR
will add test stubs and move the status to red, then implementation
PRs move it to green.

Doc-only — per the split-doc-spec convention, no scaffolding or code
rides this commit.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(rfc): address Copilot review feedback on RFC 0005

Four substantive comments on PR #41, all addressed:

1. tenant_id was REQUIRED but also "not stored inside row groups" —
   contradictory. Now documented as a Hive partition column (not a
   row-level column), synthesised by the reader from the partition
   path. The §3.8 schema-evolution rules explicitly do not apply to
   partition columns; §3.4 pins the partition contract separately.
   Same treatment in §3.7's audit schema.

2. params and separators were OPTIONAL but described as always
   present (mirroring RFC 0001's Vec<...> fields). Now REQUIRED with
   the explicit rule that NULL is not a valid encoding; the list may
   be empty.

3. The §3.5 "Compression codec" bullet conflated compression (ZSTD)
   with encoding (RLE, dictionary). Now states the codec policy
   cleanly (ZSTD-3 across the board) and explicitly defers per-column
   encoding to §3.6.

4. positions_widened was OPTIONAL but described as "empty list for
   variants that don't widen positions" — same issue as #2. Now
   REQUIRED with documented empty-list semantics for
   TemplateTypeExpanded and TemplateWideningRejectedDegenerate, plus
   NULL-handling clarified for the other audit columns.

RFC0005.1 updated to scope the round-trip property to row-level
columns; partition-column synthesis becomes an "And" clause
delegating the partition contract to RFC0005.5.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(rfc): address Copilot review round 2 on RFC 0005 (9 comments)

Nine new comments on the previous fix commit (8543543), all
addressed:

5. Added §3.0 Terminology note explicitly defining MinedRecord as
   the planned Rust type name for RFC 0001 §6.1's record schema.
   Avoids cross-RFC ambiguity until a 0001 patch adopts the name.

6. Renamed partition path segment from `tenant=` to `tenant_id=`
   throughout, matching the Hive convention (path key = column
   name). Stale references in RFC0005.5 and RFC0005.7 caught and
   fixed too.

7. Added §3.7 event-kind mapping table linking the INT32 ordinal
   stored on disk, RFC 0001 §6.4's snake_case `event_type` string,
   and the Rust variant name. The three surfaces now have a
   single normative correspondence.

8. Rewrote RFC0005.1 — the "out of scope but synthesised" contradiction
   is replaced with a clear row-level-only equality clause; the
   row-vs-path validation moves to a new RFC0005.11 (its own
   scenario + test).

9. Standardised on MiB/GiB throughout (§3.5, RFC0005.6, testing
   notes) and noted that the RFC chose binary units because
   Parquet metadata's byte counts are unprefixed binary bytes.

10. Split RFC0005.8 — compression codec (`ZSTD`) and encoding
    (no `PLAIN_DICTIONARY`/`RLE_DICTIONARY`) are now two distinct
    "Then" clauses naming the distinct Parquet-metadata fields.

11. Dropped the "CI invokes it as a nightly job" claim for
    RFC0005.6 (the workflow has no `schedule:` trigger today).
    Replaced with "run manually via cargo test --ignored" + a new
    §7 open question committing the scheduled-CI workflow PR as
    a separate follow-up.

12. Rewrote the "memory-captured rule" line in §3.10 as a self-
    contained rationale: pre-abstracting before the second
    consumer is visible picks the wrong axis. No external memory
    reference.

13. The big one — reconciled with RFC 0001 §6.1 and docs/talks/
    0001-template-miner.md ("tenant_id is present on every row …
    we trust the row"). tenant_id is now a REQUIRED row-level
    column (data + audit) and is **also** replicated in the
    partition path; the row is authoritative; the reader
    validates row-vs-path and errors on mismatch. §3.9 gains a
    third bullet pinning that contract; RFC0005.11 covers it as
    a test.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(rfc): address Copilot review round 3 on RFC 0005 (13 comments)

Thirteen new substantive comments. Stale carry-overs #1 and #2
folded into the same commit since the summary's MinedRecord
reference and §3.4's tenant_id= naming were the lingering hooks.

3, 12. §3.8 vs §3.9 internal contradiction: §3.8 rule 1 forbids
   REQUIRED additive columns, but §3.9 defaulted them. Dropped the
   REQUIRED-added-in-amendment bullet from §3.9 with an explicit
   cross-reference to §3.8 rule 1.

4. URL-encoding spec was "URL-encoded" without a normative variant.
   Pinned to RFC 3986 percent-encoding with explicit overrides:
   UTF-8 byte input, no Unicode normalisation, escape everything
   outside the unreserved set (including `/`, `=`, `%`).
   Malformed-escape decoding is a hard read error.

5, 13. UUIDv7 conflict: §3.4 normatively required UUIDv7 but §7
   listed it as an open question. Resolved by pinning UUIDv7 in §3.4
   (matters for partition-lexsort = creation-order) and removing the
   open question.

6. RFC0005.7 referenced `TemplateWidened` (the Rust variant name)
   while RFC 0001 uses `template_widened` (snake_case event_type).
   Rewrote the Given clause to use the RFC 0001 string and
   parenthetically map to the Rust variant.

7. "§3.2 cardinality invariant" was ambiguous — §3.2 in this RFC is
   the schema, the invariant lives in CLAUDE.md §3.2 / hazards H2.
   Qualified all four cardinality-invariant references with
   `CLAUDE.md` §3.2.

8. TIMESTAMP logical type used `isAdjusted=true` (not a Parquet
   spec field). Replaced with `isAdjustedToUTC=true` (Parquet's
   actual flag name); two occurrences fixed via replace-all.

9. trace_id/span_id listed `BYTES` as Parquet logical type, which
   isn't a real Parquet logical type. Switched trace_id to `UUID`
   (matches 16-byte fixed-len) and span_id to "no logical type"
   (Parquet has no 8-byte opaque-id annotation; physical type alone
   is the contract).

10. §3.7 audit schema was missing RFC 0001 §6.4 fields. Added:
    - `triggering_line_hash` (FIXED_LEN_BYTE_ARRAY(16), blake3 of
      L_raw, REQUIRED)
    - `triggering_line_sample` (STRING, first 256 B of L_raw,
      OPTIONAL)
    - `slots_expanded` (LIST<STRUCT<slot_index, types_added:
      LIST<INT32>>>, REQUIRED) — replaces the singleton
      `type_added`/`slot_index` columns since RFC 0001 specifies
      `Vec<SlotExpansion>` (a single attach can grow multiple slots)

11. RFC 0001 §9 names `event_type` (snake_case STRING) as the
    drift-query predicate-pushdown column. RFC 0005 had replaced
    that with `event_kind` ordinal. Resolution: store BOTH — the
    INT32 ordinal for writer/reader internal use, and the STRING for
    RFC 0001's contract. Writer must keep them in sync per the
    mapping table.

14. RFC0005.7 said "full §3.7 payload" then enumerated a subset.
    Rephrased to "every row-level column declared in §3.7's audit-
    schema table" with explicit NULL-on-OPTIONAL semantics.

15. RFC0005.8's "file size doesn't balloon proportionally" clause
    wasn't testable. Replaced with a concrete assertion: the body
    column chunk's `dictionary_page_offset` is unset — no
    dictionary page exists on disk for the column.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fixup! docs(rfc): RFC 0005 — address Copilot review round 2 (9 comments)

* fixup! docs(rfc): add RFC 0005 — Parquet storage (schema, writer, audit stream)

fixup! attributes/resource_attributes REQUIRED with empty-Vec ↔ [] mapping

* fixup! docs(rfc): add RFC 0005 — Parquet storage (schema, writer, audit stream)

* fixup! docs(rfc): add RFC 0005 — Parquet storage (schema, writer, audit stream)

* fixup! docs(rfc): add RFC 0005 — Parquet storage (schema, writer, audit stream)

* fixup! docs(rfc): add RFC 0005 — Parquet storage (schema, writer, audit stream)

* fixup! docs(rfc): add RFC 0005 — Parquet storage (schema, writer, audit stream)

* fixup! docs(rfc): add RFC 0005 — Parquet storage (schema, writer, audit stream)

* fixup! docs(rfc): add RFC 0005 — Parquet storage (schema, writer, audit stream)

* fixup! docs(rfc): add RFC 0005 — Parquet storage (schema, writer, audit stream)

* fixup! docs(rfc): add RFC 0005 — Parquet storage (schema, writer, audit stream)

* fixup! docs(rfc): add RFC 0005 — Parquet storage (schema, writer, audit stream)

* fixup! docs(rfc): add RFC 0005 — Parquet storage (schema, writer, audit stream)

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jensholdgaard added a commit that referenced this pull request Jun 5, 2026
- §6.5: replace "dedup mechanism not yet specified" with the §9 #1
  resolution (at-least-once with duplicates per OTLP spec; any future
  dedup is additive). Removes the §6.5↔§9 internal inconsistency.
- RFC0003.2: broaden the kill-point description from "the §6.5 step
  5/6 gap" to "anywhere in the step-4-through-6 window," since the
  duplicate-on-retry contract holds across both the 4/5 and 5/6 gaps
  once the records are durable.
- RFC0003.4 / RFC0003.12: drop `wal_syncs_total` / `wal_unflushed_bytes`
  metric-name references from acceptance criteria; reframe in terms of
  observable WAL state (frame count, segment offsets, append/sync call
  counts via a test wrapper). Acceptance criteria should not pin
  telemetry identifiers.
- §5: reference `docs/rfcs/README.md` *Required sections* by heading
  name, not the malformed `§Required-sections` token.

The scenario-id grammar question (RFC0003.<m> vs §3.<n>.<m> for
invariant-tagged scenarios) is left for the maintainer — see the PR
reply on that thread.
jensholdgaard added a commit that referenced this pull request Jun 6, 2026
…ed → specified (#127)

* docs(rfc-0003): specify §5 G/W/T scenarios + OTel-spec enrichments — drafted → specified

Fills the §5 acceptance-criteria stub on RFC 0003 (OTLP receiver) with
15 Given/When/Then scenarios — the 11 originally sketched plus four
enrichments surfaced by the OpenTelemetry transport-spec citations the
maintainer pulled in (empty request → success, identity+gzip MUST,
default /v1/logs path + override, concurrent Export calls).

§5.2 (Crash-before-ack) lands as **at-least-once with retry
tolerance**: the OTLP spec's *duplicate-data* section explicitly
accepts client-retry duplicates as the right tradeoff for telemetry
data, and the Collector's own WAL guidance carries the same caveat.
This resolves §9 open question #1 (dedup) — the receiver implements no
de-duplication in this RFC; any future dedup mechanism is additive.
§5.13 (compression) resolves §9 open question #9 — identity + gzip are
both required acceptance criteria; zstd/br stay out of scope (HTTP 415).

§8 is filled to map each scenario id to its test technique (proptest
for wire-decode equivalence, child-process SIGKILL harness mirroring
PR #126 for crash-before-ack, table-driven for the transport-error and
edge-OTLP arms, criterion for the latency/throughput benches).
Frontmatter status flips drafted → specified per the maturity ladder
(docs/rfcs/README.md §Lifecycle); §10 references unchanged
(the OTLP spec link already covers the cited sub-sections).

No code touched — per the split-doc-from-code-PRs convention, the §5
test stubs (red gate per docs/verification.md §3) ride a follow-up
implementation PR.

* docs(rfc-0003): address Copilot review feedback on §5 / §6.5

- §6.5: replace "dedup mechanism not yet specified" with the §9 #1
  resolution (at-least-once with duplicates per OTLP spec; any future
  dedup is additive). Removes the §6.5↔§9 internal inconsistency.
- RFC0003.2: broaden the kill-point description from "the §6.5 step
  5/6 gap" to "anywhere in the step-4-through-6 window," since the
  duplicate-on-retry contract holds across both the 4/5 and 5/6 gaps
  once the records are durable.
- RFC0003.4 / RFC0003.12: drop `wal_syncs_total` / `wal_unflushed_bytes`
  metric-name references from acceptance criteria; reframe in terms of
  observable WAL state (frame count, segment offsets, append/sync call
  counts via a test wrapper). Acceptance criteria should not pin
  telemetry identifiers.
- §5: reference `docs/rfcs/README.md` *Required sections* by heading
  name, not the malformed `§Required-sections` token.

The scenario-id grammar question (RFC0003.<m> vs §3.<n>.<m> for
invariant-tagged scenarios) is left for the maintainer — see the PR
reply on that thread.

* docs(rfc-0003): align WAL framing with RFC 0008 (per-batch, not per-record)

Copilot's second review pass caught a real contract bug: RFC 0003's §5
scenarios and §6.5 step sequence asserted per-record WAL appends, but
RFC 0008 §4 / §7.3 specifies `FrameKind::OtlpBatch` — one frame per
`ExportLogsServiceRequest`, carrying the verbatim protobuf bytes. RFC
0008 §7.3 explicitly rejects per-record framing as the alternative.

Fixes:

- §5.1, §5.2, §5.15: assertions now reference the `OtlpBatch` frame and
  its payload bytes round-tripping to the input request, not "every
  record in the WAL." The retry-duplicate assertion is now "a second
  `OtlpBatch` frame whose payload bytes equal the first."
- §6.1 step 5, §6.5 preamble, §6.5 step 3: durability language switched
  from "every record durably written" / "appends every record" to
  "batch's `OtlpBatch` frame is durably written" / "appends the encoded
  request as a single `OtlpBatch` frame."
- §8 testing-strategy crash-before-ack: asserts the same `OtlpBatch`
  payload bytes appear twice (two frames, one per export attempt).
- §5.11: drop the `max_request_bytes` config-key reference; phrase as
  "configured request-size limit" since the knob isn't specified in this
  RFC and pinning a name in acceptance criteria is premature.

The miner still consumes records per-record (§6.1 step 4–5, §9 open
questions); only the WAL frame is per-batch.

* docs(rfc-0003): close §6.5 ack-conditions and greppability gaps

Copilot's third review pass:

- §6.5: qualify the "ack only after `OtlpBatch` is durably written"
  contract as applying to non-empty batches; explicitly point at
  RFC0003.12 as the empty-batch exception (which returns success with
  no WAL write).
- RFC0003.1, RFC0003.15: lock in the §6.5 step-5 miner-acceptance
  condition alongside `Wal::sync` return. Both scenarios now assert
  ack-after-(sync AND miner-accept), per-call for RFC0003.15. RFC0003.1
  uses an instrumented `MinerCluster` stub; RFC0003.15's per-call probe
  records both orderings.
- §5 intro, §8 testing-strategy: clarify the greppability convention —
  the verbatim id reference lives in each test's leading doc comment
  (`/// Scenario RFC0003.1 — ...`) per `docs/verification.md` §2.3, not
  in the Rust function name. The `#[test] fn rfc0003_1_*` example was
  misleading.

* docs(rfc 0003): fix RFC 0008 cross-ref §4 -> §3.2 + §6.2.3

§4 of RFC 0008 is "Background — existing Rust durability ecosystem";
the FrameKind::OtlpBatch payload contract lives in §3.2 ("What goes
into the WAL") with the frame/payload-encoding details in §6.2.2 /
§6.2.3.

* docs(rfc 0003): tighten singular-frame, §6.3 error wording, decouple miner internals

Three Copilot nits:

1. RFC0003.1 said `Wal::sync` "covering the batch's frames" — singular
   per RFC 0008 §3.2 (one `OtlpBatch` frame per export batch).
2. §6.3 said the tenant-resolution error names "the failing Resource";
   RFC0003.4 names "the failing `ResourceLogs` index". Aligned both on
   the index + attribute key.
3. RFC0003.8's last `And` reached into miner internals (RFC 0001 §6.2
   step-0 short-circuit). The receiver's contract is the byte-for-byte
   pass-through; how the miner subsequently routes it is the miner's
   business. Asserted via an instrumented `MinerCluster` stub instead.

* docs(rfc 0003): broaden RFC0003.12 to all three "empty" shapes

"Empty" in OTLP can mean any of: empty resource_logs, empty
scope_logs per ResourceLogs, or empty log_records per ScopeLogs.
RFC0003.12 only covered the first shape; the fast-path contract
("zero records => success without WAL write") applies to all three.
All three are now explicit in the Given and tested.

* docs(rfc 0003): broaden unknown-fields rule, own the wire-0→None rule

1. RFC0003.6's unknown-fields clause restricted forward-compatibility
   to *top-level* fields; the OTLP/proto3 rule is "anywhere in the
   message". Reworded to "anywhere in the request body (top-level,
   nested, repeated)".
2. RFC0003.9 asserted observed_time_unix_nano=0 -> None but the rule
   was nowhere written down — RFC 0001 §6.1 just types the field as
   Option<u64> without defining the conversion. Made the contract
   explicit: the receiver owns the wire-0→None mapping, severity_number
   stays 0 because UNSPECIFIED is a valid OTLP value (not absence).

* docs(rfc 0003): respect WAL single-writer contract in replay assertions

RFC0003.1 / .4 / .15 each verified WAL state by opening a "fresh
Wal::open" while the receiver was still alive. The Wal contract
is "opened by exactly one writer" (ourios-wal/src/lib.rs §6.2 —
O_APPEND single-writer handle), so a second open while the
receiver's handle is live violates the contract and would race
with replay's heal/truncate path.

Spelled out in all three scenarios: shut the receiver down (which
drops its Wal handle) before opening a *second* Wal and replaying.

* docs(rfc 0003): use OtlpBatch frame terminology in WAL absence assertions

Three spots described "no record appended to the WAL" / "no partial
record appended" — but the WAL persistence unit is per-export
FrameKind::OtlpBatch frames, not per-record entries. Reworded
RFC0003.4, RFC0003.11, and §8's transport-errors testing bullet
to assert "no OtlpBatch frame appended" so the spec is unambiguous
about what the WAL stores.

* docs(rfc 0003): retry frames must be semantically equal, not byte-equal

RFC0003.2 and §8's crash-before-ack bullet both required the retry's
OtlpBatch frame to be byte-identical to the first. That over-constrains
clients: a retry can legitimately re-encode (JSON ordering, whitespace,
switched encoding/compression) and remain semantically the same export.

Both now assert that the second frame's payload decodes (via prost)
to an ExportLogsServiceRequest semantically equivalent to the first.

Also retitled RFC0003.13 to "Compression over HTTP: identity and gzip
MUST be supported" — the scenario is HTTP-only (Content-Encoding),
matching §9's wording.

* docs(rfc 0003): tidy §8 — Body-fork grammar + drop PR-M2 jargon

1. §8's Body-fork bullet had awkward grammar ("variants assert
   `Body::from_any_value` routes…"). Reworded as "each asserting
   that …" so the subject of the assertion is unambiguous.
2. Dropped the "PR-M2" label from the two-loop red-gate note; the
   label is internal milestone shorthand that's not defined in this
   RFC. Now refers to the two-loop pattern itself (and to RFC 0008
   §5 as the precedent) so the note is self-contained.

* docs(rfc 0003): tighten payload-equality wording in RFC0003.1 + .2

RFC0003.1's last `And` claimed "payload bytes equal the encoded
`ExportLogsServiceRequest`" but the named verification only checks a
`prost` round-trip. Protobuf has multiple wire encodings for the same
message, so the claim was strictly stronger than what the test asserts.
Relax to "payload decodes (via `prost`) to the input request" and
cross-link the RFC0003.2 explanation of why byte equality is the wrong
shape of contract here.

RFC0003.2's first `Then` said "decodes byte-for-byte" — internally
inconsistent (decoding yields a message value, not bytes). Rephrase to
"decodes (via `prost`) to an `ExportLogsServiceRequest` semantically
equivalent to the killed process's input," matching the second `And`
which already had the right shape after round 10.

Both Copilot comments (PR #127, comments 3367201831, 3367201836).

* docs(rfc 0003): disentangle ordering vs content check in RFC0003.1

Copilot caught that the last `And` clause bundled two distinct
assertions — "frame exists with the right payload" and "before the ack
fires" — but only the former is verified by the named technique
(post-shutdown replay via `Wal::replay`). Replay establishes
existence + content post-response; it can't establish pre-ack timing,
because by the time the second `Wal` is opened the response has
already shipped.

The before-the-ack ordering is what the `AtomicBool` probe in the
preceding `Then` + `And` clauses is for (set after `Wal::sync`
returns; asserted `true` by the response-writer; asserted `false` at
every pre-sync stage). That's the timing contract. The replay is the
durability/content contract. Rewording makes the split explicit so a
reader doesn't infer the replay proves the ordering.

PR #127, comment 3367208364.

* docs(rfc 0003): align §6.5 step 3 with RFC0003.1/.2 wire-encoding stance

Copilot caught that §6.5 step 3 said the WAL payload is stored as
"verbatim `ExportLogsServiceRequest` protobuf bytes" — strictly
stronger than what RFC0003.1's acceptance criteria require (rounds 12+13
relaxed those to "decodes via `prost` to the input request"). The
mismatch is most obvious on the HTTP/JSON path: there are no incoming
protobuf bytes to be "verbatim" — the receiver must encode the decoded
message to protobuf for WAL storage.

Reword step 3 to require the payload be a protobuf-encoded
`ExportLogsServiceRequest` decodable via `prost`, semantically
equivalent to the input. The wire-bytes-verbatim option remains
explicitly permitted (it is the obvious zero-copy choice for the
protobuf paths), but it is no longer the *contract*.

PR #127, comment 3367215593.

* docs(rfc 0003): cite RFC 0008 §3.1 for single-writer (not source file §6.2)

Copilot caught (three identical comments on lines 260, 340, 534) that
`crates/ourios-wal/src/lib.rs` §6.2 is a confusing cross-reference:
the file is Rust source, not a sectioned doc. The §6.2 the source-file
comment at line 162 itself cites is RFC 0008 §6.2 (segment layout),
which is where the append-only property lives — but the actual
*single-writer architecture* statement RFC 0003 is leaning on is
RFC 0008 §3.1 ("single-writer single-node component").

Rephrase all three sites to:

  per RFC 0008 §3.1's single-writer architecture
  (enforced by `crates/ourios-wal/src/lib.rs:162`)

— RFC-first as the contract; source file kept as the concrete
enforcement point. Source file gets a line-number cite, which is the
right shape for a Rust file.

PR #127, comments 3367230544, 3367230558, 3367230561.
jensholdgaard added a commit that referenced this pull request Jun 20, 2026
… §3.1 cite

Copilot review:
- RFC0018.1: schema_url is on ScopeLogs, not InstrumentationScope (which
  carries name/version/attributes). Reword the scenario accordingly.
- §8: include the -Infinity proto3-JSON string form alongside NaN/Infinity.
- §7: disambiguate the bare (§3.1 hazard) cite (this RFC's §3.1 is scope
  URLs) to CLAUDE.md §3.1 / docs/hazards.md #1 (template cardinality).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jensholdgaard added a commit that referenced this pull request Jun 20, 2026
…ty, §3.1 cite)

Copilot review:
- RFC0018.1: schema_url is on ScopeLogs, not InstrumentationScope (which
  carries name/version/attributes). Reword the scenario accordingly.
- §8: include the -Infinity proto3-JSON string form alongside NaN/Infinity.
- §7: disambiguate the bare (§3.1 hazard) cite (this RFC's §3.1 is scope
  URLs) to CLAUDE.md §3.1 / docs/hazards.md #1 (template cardinality).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jensholdgaard added a commit that referenced this pull request Jun 20, 2026
* docs(rfc-0018): add OTLP log-spec compliance amendments (specified)

One compliance push closing the six OTLP-fidelity gaps from the 2026-06-20
audit, graded against the spec via the OTel knowledge base. Governing
principle (§3.0): the backend is a faithful witness, not a corrector —
preserve what arrived (up to storage invariants), surface violations as
observable anomalies, never silently correct or reject; producing
spec-valid telemetry is the upstream's contract.

Six fixes spanning three green RFCs (0002, 0003, 0005):

1. persist InstrumentationScope.attributes + resource/scope schema_url
   (the flat MUST — dropped at the receiver today) — RFC 0003 + RFC 0005
2. map transient ingest failures to retryable gRPC/HTTP codes, not
   non-retryable INTERNAL/500 (clients currently drop data) — RFC 0003
3. event_name (and scope_version) as first-class DSL filters — RFC 0002
4. round-trip non-finite doubles via the proto3-JSON string forms
   ("NaN"/"Infinity"/"-Infinity") — RFC 0005
5. preserve out-of-range SeverityNumber + flag it, overturning the current
   silent clamp-to-0 (severity_to_u8); u8 column retained — RFC 0003
6. correct the body-column doc (UTF-8 JSON for Structured) — RFC 0005

Decisions baked in: severity preserve+flag (not clamp/reject); u8 column;
scope attributes retained + queryable but out of the template key. §5
scenarios RFC0018.1-.6. Spec fidelity outranks downstream API stability
(pre-release), so the implementation takes the resulting type/schema
changes. status: specified.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(rfc-0018): fix OTLP-accuracy nits (ScopeLogs.schema_url, -Infinity, §3.1 cite)

Copilot review:
- RFC0018.1: schema_url is on ScopeLogs, not InstrumentationScope (which
  carries name/version/attributes). Reword the scenario accordingly.
- §8: include the -Infinity proto3-JSON string form alongside NaN/Infinity.
- §7: disambiguate the bare (§3.1 hazard) cite (this RFC's §3.1 is scope
  URLs) to CLAUDE.md §3.1 / docs/hazards.md #1 (template cardinality).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
jensholdgaard added a commit that referenced this pull request Jun 29, 2026
…302)

Round 3 on #312 — close the empty-body window fully: a mined record must
never become query-visible before its template's audit event is durable,
under concurrency and the inline size trigger (strengthens `CLAUDE.md` §3.3
to hold on every publication path).

#4 — `emit` no longer blocks behind flush I/O. The audit sink's flush now
drains the buffer under the lock, releases it, does the `AuditWriter` store
I/O **unlocked**, then re-locks only to settle counters and requeue a
transient failure's events ahead of anything `emit` buffered meanwhile
(mirrors the record sink's documented "drain under the lock, I/O unlocked,
re-lock to settle"). A slow flush can't stall the request path.

#3 — the buffer is hard-bounded. The soft ceiling still signals an eager
off-runtime flush; a new hard cap (`AUDIT_SINK_MAX_EVENTS`, well above the
ceiling) is the OOM backstop: at the cap `emit` drops (counted via the new
`ourios.audit_sink.dropped` metric, logged once) rather than grow without
bound under sustained store-unavailability. Dropped template events degrade
those templates to retained/empty bodies until the WAL re-mines them on
restart — bounded memory is the deliberate trade.

#1/#2 — publication is audit-ordered and race-free via snapshot-then-
ordered-write. A new `PublishCoordinator` (ourios-ingester) drains both
sink buffers into owned batches under the pipeline's miner lock (a
microsecond memory move, no I/O — atomic w.r.t. `ingest`, closing the
cadence TOCTOU race), then writes off-lock: the audit batch to durability
first, the record partitions only after. A transient audit failure holds
the records (requeued, retried next cadence); a permanent audit failure
drops the audit batch and still publishes the records (the documented
degraded case). The receiver's age-sweep now publishes through it. The
inline size/ceiling trigger routes through a new record-sink audit barrier
(`ParquetRecordSink::with_audit_barrier`) that flushes the audit sink to
durability before the partition is put — race-free because that publish
runs under the miner lock. Rotation/shutdown already drain audit-before-
record under the miner lock (`flush_then_snapshot`), unchanged.

The record sink gains a drain/publish/requeue split (`drain_aged` /
`drain_all` / `requeue` / `publish_owned`) so the coordinator can move the
encode+put off the lock; its existing RFC 0014 emit/flush behavior and
tests are intact.

Tests: the coordinator holds records when the audit write fails transiently
(no data partition published though the data store is healthy); the size
trigger flushes audit-before-publish and is skipped when audit can't drain;
the hard cap drops + bounds; transient retains vs permanent drops; the
metrics export; plus all round-1/2 tests and the #302 regression stay green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jensholdgaard added a commit that referenced this pull request Jun 29, 2026
) (#312)

* fix(server): wire a buffering audit sink into the receiver (#302)

The OTLP receiver wired the template miner with a record sink but no
audit sink, so the miner's `template_created` / `template_widened` /
`template_type_expanded` events never reached the RFC 0005 audit Parquet
stream. The querier's read-time registry (RFC 0017
`derive_template_registry`) was therefore empty and `render_log_body`
fell back to the row's retained `body` — empty for clean, high-confidence
rows — so queries over freshly-ingested clean logs returned empty body
text, breaking `CLAUDE.md` §3.3.

Mirror the RFC 0014 record sink rather than wiring `ParquetAuditSink`
directly (which does a blocking per-event store write — request-path
stall + one tiny file per event, hazard #4):

- `BufferingAuditSink` / `SharedParquetAuditSink` (ourios-ingester):
  `emit` buffers cheaply on the request path; `flush` drains the buffer,
  groups events by audit partition, and writes each partition's batch
  with one `AuditWriter` (open_in → append_events → close) — few files,
  not one-per-event. A failed partition write retains its events (the WAL
  is the durability of record); an empty-buffer flush is a no-op.
- The receiver constructs the sink on the same `Store`, wires it via
  `MinerCluster::with_audit_sink(...).with_record_sink(...)` before
  recovery (so replay re-emits template events), and flushes it off the
  async runtime at the same cadence + rotation + shutdown points as the
  record sink — audit *before* records (durable no later than the rows it
  describes), with the snapshot gated on both sinks draining.
- Expose `derive_audit_partition` from ourios-parquet for the grouping.

Tests: unit tests for the buffering sink (per-partition batching round
trip + empty-buffer no-op); an in-process receiver test that ingests
clean logs, drains, derives the registry, and asserts every clean row
reconstructs `Faithful` from its template rather than empty. The RFC0019
`.3`/`.5` localstack scenarios now also assert the returned body text.
`rfc0013_6` scopes its data round-trip to `data/` so the new `audit/`
files aren't read with the data schema.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(parquet): drop intra-doc link to a private item

derive_audit_partition is now pub; its doc referenced the private
audit_partition_matches via an intra-doc link, which trips
rustdoc::private-intra-doc-links under cargo doc -D warnings. Use plain
backticks (the recurring private-item-doc convention).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(server): audit-sink metrics, error classification, bounded buffer (#302)

Address PR #312 review feedback on the receiver audit sink.

OTel metrics (§6.3): add `ourios.audit_sink.*` instruments mirroring the
record sink's `SinkMetrics` — `buffer.usage` (observable gauge of buffered
events), `flushes`, `flush.events`, `flush.errors` (split transient vs
permanent via the new `ourios.audit_sink.flush.outcome` attribute), and
`derive.errors`. Names go through the weaver registry
(`semconv/registry/{metrics,attributes}.yaml`); the generated
`ourios-semconv` constants are regenerated, not hand-written. Instruments
resolve through the global meter (no-op without a provider). A dedicated
test binary asserts the stream exports (separate process — `init_in_memory`
installs the global provider).

Data integrity: classify a failed partition flush. A store-`Io` error is
transient → retain + retry (the WAL is the durability of record); a
`Batch` / `Parquet` / `PartitionMismatch` / `Poisoned` error is permanent →
drop + count, so one malformed event can't requeue forever and wedge every
newer good event for that tenant/day behind it.

§3.3 flush gating: in both the age-sweep and `flush_then_snapshot`, flush
the audit sink first and skip the record flush this cycle if it didn't fully
drain — a non-empty buffer means a transient store error (permanents drop),
so the record flush to the same store would fail anyway, and flushing it
would expose a clean row before its template event is durable.

Bounded buffer: `emit` stays non-blocking but enforces a soft event ceiling
(default 100k); reaching it signals a `tokio::sync::Notify` the age-sweep
selects on, so adversarial template churn flushes promptly off the runtime
rather than growing the buffer until OOM. Signal-to-flush, never drop.

Tests: poison-pill (permanent drops + counts, does not requeue; transient
retains), flush-gating (record flush skipped while audit retains), bounding
(emit past the ceiling fires the notify), plus the metrics-export test.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(server): audit-ordered publication, non-blocking flush, hard cap (#302)

Round 3 on #312 — close the empty-body window fully: a mined record must
never become query-visible before its template's audit event is durable,
under concurrency and the inline size trigger (strengthens `CLAUDE.md` §3.3
to hold on every publication path).

#4 — `emit` no longer blocks behind flush I/O. The audit sink's flush now
drains the buffer under the lock, releases it, does the `AuditWriter` store
I/O **unlocked**, then re-locks only to settle counters and requeue a
transient failure's events ahead of anything `emit` buffered meanwhile
(mirrors the record sink's documented "drain under the lock, I/O unlocked,
re-lock to settle"). A slow flush can't stall the request path.

#3 — the buffer is hard-bounded. The soft ceiling still signals an eager
off-runtime flush; a new hard cap (`AUDIT_SINK_MAX_EVENTS`, well above the
ceiling) is the OOM backstop: at the cap `emit` drops (counted via the new
`ourios.audit_sink.dropped` metric, logged once) rather than grow without
bound under sustained store-unavailability. Dropped template events degrade
those templates to retained/empty bodies until the WAL re-mines them on
restart — bounded memory is the deliberate trade.

#1/#2 — publication is audit-ordered and race-free via snapshot-then-
ordered-write. A new `PublishCoordinator` (ourios-ingester) drains both
sink buffers into owned batches under the pipeline's miner lock (a
microsecond memory move, no I/O — atomic w.r.t. `ingest`, closing the
cadence TOCTOU race), then writes off-lock: the audit batch to durability
first, the record partitions only after. A transient audit failure holds
the records (requeued, retried next cadence); a permanent audit failure
drops the audit batch and still publishes the records (the documented
degraded case). The receiver's age-sweep now publishes through it. The
inline size/ceiling trigger routes through a new record-sink audit barrier
(`ParquetRecordSink::with_audit_barrier`) that flushes the audit sink to
durability before the partition is put — race-free because that publish
runs under the miner lock. Rotation/shutdown already drain audit-before-
record under the miner lock (`flush_then_snapshot`), unchanged.

The record sink gains a drain/publish/requeue split (`drain_aged` /
`drain_all` / `requeue` / `publish_owned`) so the coordinator can move the
encode+put off the lock; its existing RFC 0014 emit/flush behavior and
tests are intact.

Tests: the coordinator holds records when the audit write fails transiently
(no data partition published though the data store is healthy); the size
trigger flushes audit-before-publish and is skipped when audit can't drain;
the hard cap drops + bounds; transient retains vs permanent drops; the
metrics export; plus all round-1/2 tests and the #302 regression stay green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(ingester): keep requeued partitions aged for prompt retry

PublishCoordinator.requeue re-buffers a transient-failed batch ahead of records
emit added during the off-lock publish, but left PartitionBuffer.oldest at the
newer records' timestamp — so the already-aged requeued records could miss the
next age-sweep and retry late. Pin oldest to the age threshold (min with the
existing oldest) on requeue so the next sweep re-drains promptly. Test:
requeue_keeps_the_partition_aged_for_prompt_retry.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(ingester): audit buffer retains, never drops (reverse the hard cap) (#302)

Round 5 on #312.

Item 1: reword the audit_sink module-doc typo "The Drain miner emits…" →
"The template miner emits…".

Item 2 (CodeRabbit Major — the round-2 hard-cap drop could lose data):
the hard cap dropped audit events at `AUDIT_SINK_MAX_EVENTS`, which is
unsafe. A dropped event isn't counted by `buffered_events()`, so the
no-loss snapshot gate (`flush_then_snapshot`) doesn't see it, the miner
snapshot advances past that line's WAL position, and on restart the
template event is never re-mined → those clean rows become permanently
unreconstructable (a §3.3 violation), not merely degraded-until-restart.

Adopt the record sink's posture (follow the reference, §5.4): under
sustained store-unavailability the audit buffer is RETAINED and may
transiently exceed the ceiling — never dropped. The WAL is the durability
of record; the snapshot gate prevents loss because it won't advance while
the buffer is non-empty.

- Delete `AUDIT_SINK_MAX_EVENTS` and the drop branch in `buffer_event`;
  `emit` always buffers. The soft ceiling still fires the `Notify` for an
  eager off-runtime flush — the bound for the realistic (healthy-store)
  case. `requeue_ahead` no longer caps/drops.
- Remove the `ourios.audit_sink.dropped` metric: reverted its
  `semconv/registry/metrics.yaml` entry, regenerated `ourios-semconv`
  (the const is gone; weaver no-diff verified), and dropped its use.
- Replace the `hard_cap_drops_and_bounds_the_buffer` test with
  `persistent_store_failure_retains_every_event_never_drops`: under a
  persistently failing store, repeated emit + flush retains every event
  (buffer grows past the ceiling) and drops nothing.
- Module docs state the posture explicitly (healthy store → ceiling +
  Notify bound it; sustained outage → retained, like the record sink; OOM
  under a total outage is the same accepted failure mode the record sink
  carries).

The transient-vs-permanent flush classification (Io retain / Batch +
Parquet + PartitionMismatch + Poisoned drop+count) is unchanged — that's
about un-writable content, unrelated to the memory bound.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
jensholdgaard added a commit that referenced this pull request Jul 11, 2026
A v8-shaped miniature fixture (INFO-dominated + one WARN, zero ERROR)
pins that the picker selects the WARN band — the exact path run #1
surfaced.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jensholdgaard added a commit that referenced this pull request Jul 11, 2026
…gs) (#475)

* fix(bench): generalize the pair picker — v8 carries no ERROR logs

Run #1 of the indicative comparative (29160983634) failed fast (12 s,
before any container) in exactly the designed way: the corpus scan found
ZERO severity>=17 rows in the whole 4.9 M-record otel-demo v8 capture —
the calibration manifest confirms the corpus's complete severity
distribution is INFO x2.19M, "Information" x2.76M, WARN x4 (the demo's
adFailure/paymentFailure flags surface in traces/metrics, not logs) —
and the picker refused to fabricate a pair.

pick_error_pair → pick_selective_pair: instead of a hardcoded ERROR
band, scan per-service (severity_number, severity_text) row counts and
pick any (service, threshold T, text t) where EVERY row with number>=T
carries the single text t and the count is 1..=4000. The consistency
requirement still makes DSL `severity >= T` and LogQL
`severity_text="t"` the same question. On v8 this selects the WARN band
(>=13, 4 rows — an extremely selective honest L2); on error-bearing
corpora it selects ERROR exactly as before, which the fixture unit test
now pins (threshold 17, text ERROR).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(bench): reverse consistency + scan efficiency in the pair picker

Copilot review, all three:
- REVERSE consistency (real equivalence hole): the check guaranteed all
  num>=T rows share text t, but not that no LOWER-severity rows carry t
  — LogQL's text filter would return those too and break equivalence at
  container time. A candidate now also requires the service's total
  count for text t to equal the selected rows.
- Nested map (service -> number -> text -> count): thresholds are the
  deduped number keys (no recomputation per (num,text) pair).
- Severity texts clone only on FIRST occurrence per (service, number)
  via get_mut-then-insert — not one clone per record across millions.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* refactor(bench): split pair-candidate selection (clippy too_many_lines)

pick_selective_pair hit 106/100 after the reverse-consistency addition;
extract select_pair_candidates. No behavior change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(bench): safe-charset guard for pair names + threshold doc

Copilot review:
- pair.service / pair.text are interpolated into quoted DSL and LogQL
  string literals; a `"` or `\` (legal in OTLP attributes) would break or
  change either query. Rather than implement escaping for two query
  languages, a candidate whose names fall outside a conservative charset
  is simply skipped.
- Doc: candidate thresholds are the OBSERVED severity numbers, which is
  complete — a gap threshold selects exactly the same rows as the next
  observed number above it, adding no new candidates.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test(bench): cover the no-ERROR generalization path (Copilot review)

A v8-shaped miniature fixture (INFO-dominated + one WARN, zero ERROR)
pins that the picker selects the WARN band — the exact path run #1
surfaced.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(bench): exclude zero-timestamp records from candidate bands

Copilot review: a zero-ts record can't be returned by either side's
time-windowed query, so counting it into a band would fail the pair's
expected-count check at run time. Skip it for bands (still counted in
the total-records report figure).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs(bench): precise justification for the zero-ts band exclusion

Copilot review: the effective-timestamp fallback means BOTH systems can
return a time=0/observed-set record (Ourios windows the RFC 0005 §3.2
effective column; Loki's OTLP ingest falls back to observed) — but with
DIFFERENT answer timestamps (Ourios keeps time_unix_nano=0, Loki stamps
observed), so their LineKeys can never match. Exclusion is required for
key identity, not window reachability; the comment now says so.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jensholdgaard added a commit that referenced this pull request Jul 15, 2026
…u32::MAX bound

PR review findings #1 and #3. The structured JSON surface's group_term
accepted {resource|attr} field objects in a by-list, but the string DSL's
group_term = field production (§7 v1.1) is bare-field-only — so the
structured surface could express count/aggregate-by queries the string
grammar cannot, which then failed in planning instead of at validation.

RawGroupTerm::into_ir now rejects a {resource|attr} object with a clean
DslError, and the schema gains a bare_field $defs entry so schema
validation itself rejects the shape instead of only the runtime
converter. The schema's param integer also gets an explicit maximum
(u32::MAX) so an out-of-range param slot fails schema validation
cleanly rather than succeeding the schema and then failing Rust
deserialization.

Adds schema instance-list cases (resource/attr group term, param past
u32::MAX) and a structured.rs unit test for the runtime rejection.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
jensholdgaard added a commit that referenced this pull request Jul 15, 2026
…bucket(w) (#533)

* feat(querier): rfc 0002 green — count-by execution with param(n) and bucket(w)

The aggregation-execution slice of the RFC 0002 amendment 2026-07-15
(RFC 0031 L4): `count [by …]` now executes end-to-end, discharging §5
scenarios RFC0002.12/.13/.15/.16. RFC0002.14 (the grammar/compile error
contracts) stays an ignored red stub for its own slice.

Surface (§7 v1.1 / §6.4 amendment):
- IR: the aggregation `by`-list widens from `Vec<Field>` to
  `Vec<GroupTerm>` (field | `param(n)` | `bucket(duration)`).
- Parser: `group_list`/`group_term` productions, confined to `by`-lists;
  positive + negative parse tests per production.
- Structured surface: `{"param": n}` / `{"bucket": "<duration>"}`
  by-elements (widths validated by the string-DSL lexer, RFC0002.2);
  `structured_query.schema.json` gains the additive `group_term` def
  (snapshot-gated by RFC0002.11, which also gains instances).
- Serializer: group terms round-trip (corpus + proptest generator).

Compile (§6.3/§6.5 amendment):
- `compile::validate` lifts the `count` rejection ONLY — sum/min/max/avg,
  sort, project, render keep the explicit rejection. Enforces the
  single-template pinning rule for `param(n)` (top-conjunctive
  `template_id == N`, all naming one N; `resolves_to` does not pin),
  positive bucket widths, and the duplicate-term rules.
- Group terms lower as expressions inside the existing Aggregate row:
  `param(n)` = `array_element(params, n+1).value` (stored string form,
  no type promotion); `bucket(w)` = floor division of the effective
  timestamp (with the §3.9 `time_unix_nano` fallback) into half-open
  epoch-aligned UTC windows; `service` = the RFC 0022 promoted column.

Execute:
- One grouped-count scan per aggregation query (Filter → Aggregate,
  the drift precedent) with a row-level `tenant_id` guard mirroring
  drift's (§3.7 — group values are row contents). `rows` stays the
  total matching count, derived from the same scan.
- Short/NULL `param(n)` rows are EXCLUDED from every group (no synthetic
  absent key) and tallied on the new `QueryStats.rows_excluded`,
  surfaced on the RFC 0016 stats DTO (RFC0002.15).
- Result carrier: `QueryResult.aggregate: Option<Vec<AggregateGroup>>`
  (`key: Vec<String>` per by-term in query order — bucket keys RFC 3339
  UTC window starts — sorted, engine-free per hazard §4.6); the RFC 0016
  response gains the additive `aggregate` field so the HTTP surface
  cannot silently drop the map.
- RFC0002.16 honest bytes: the total is the group-column scan alone —
  zero row materialization, zero template-map acquisition (the RFC 0033
  acquisition was already lazy; the aggregation path never renders).

Invariants: hazard §4.6 (no DataFusion/arrow/SQL crosses the surface —
plain strings/ints only); §3.7 multi-tenancy (partition scope + the new
row-level tenant filter on the aggregation plan). Contract changes
sanctioned by the maintainer-merged amendment (#531): the `count` case
moves out of rfc0002_6_unsupported_stage_rejected (RFC0002.12 names the
lift), and rfc0005_14's error-precedence probe switches from `count` to
the still-rejected `render`.

Verified: cargo fmt --check; workspace clippy --all-targets
--all-features -D warnings; strict rustdoc (ourios-querier); full
cargo nextest run (1107 passed); .12/.13/.15/.16 force-run green,
.14 still ignored-failing.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y

* fix(querier): structured by-list rejects resource/attr paths + param u32::MAX bound

PR review findings #1 and #3. The structured JSON surface's group_term
accepted {resource|attr} field objects in a by-list, but the string DSL's
group_term = field production (§7 v1.1) is bare-field-only — so the
structured surface could express count/aggregate-by queries the string
grammar cannot, which then failed in planning instead of at validation.

RawGroupTerm::into_ir now rejects a {resource|attr} object with a clean
DslError, and the schema gains a bare_field $defs entry so schema
validation itself rejects the shape instead of only the runtime
converter. The schema's param integer also gets an explicit maximum
(u32::MAX) so an out-of-range param slot fails schema validation
cleanly rather than succeeding the schema and then failing Rust
deserialization.

Adds schema instance-list cases (resource/attr group term, param past
u32::MAX) and a structured.rs unit test for the runtime rejection.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(querier): reject count+limit; property-test the §6.3 planner invariants

PR review findings #2 and #4.

#2: `count [by …] | limit n` silently dropped the `limit` — execution
terminates in `Terminal::Aggregate`, which never consults `plan.limit`
(the aggregation map is the whole result; group-limiting semantics
aren't implemented). `validate()` now rejects the combination with a
clear QueryError::InvalidQuery instead of quietly returning the wrong
thing.

#4: pin detection (top-conjunctive `template_id == N`; `or`/`not`/
`resolves_to` don't pin), param-position duplication (at most one
`param(n)` per distinct n), and bucket constraints (positive width, at
most one `bucket(...)`) were covered only by hand-picked examples.
Adds a proptest generating arbitrary predicates and by-lists, checked
against an independently tracked ground truth (ground truth recorded
alongside generation, not derived from the code under test), covering
both `pinned_template_id` and `validate()`'s accept/reject decision.
The hand-picked examples stay as-is (CLAUDE.md §6.2).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* test(querier): event_name in group-term generator; tenant + NULL-param regressions

PR review findings #5, #6, #7.

#5: the RFC0002.7 round-trip generator's `bare_field()` — shared by
`path_field()`, `group_term()`, and the `project` field list — omitted
`Field::EventName`, so grouped-query round-trips never covered
`count by event_name`. It's a valid bare field everywhere the real
grammar's `bare_field` (parse.rs) allows it, so fixed in place.

#6: `rfc0002_12_count_by_matches_naive_oracle`'s foreign-tenant fixture
is written via `write_all`, which partitions by the record's own
`tenant_id` — so the "b" row lands under tenant "b"'s own directory and
the row-level `tenant_id == tenant` backstop in `execute_aggregate`
(CLAUDE.md §3.7) is never exercised, only directory-level scoping. Adds
`rfc0002_12_aggregation_tenant_backstop_excludes_misplaced_row`, which
plants a tenant "b" row *inside* tenant "a"'s partition directory (the
shape a partitioning bug or on-disk corruption would produce — the
`ourios-parquet` writer's RFC 0005 §3.9 row-vs-path contract refuses a
mismatched tenant_id at write time, so the row is written honestly then
relocated) and asserts the backstop filter, not partitioning, keeps it
out of both the count and the group map. Manually verified this test
fails without the backstop filter, confirming it exercises the guard.

#7: RFC0002.15 covered a `params` list shorter than `n + 1`, but not the
distinct case of a list that HAS slot n whose own `value` decodes as
Parquet-level NULL (the field is nullable — RFC 0005 §3.2 — even though
`Param.value` is a non-`Option` Rust `String`, so only a raw/corrupted
writer can produce it). Adds
`rfc0002_15_present_but_null_param_slot_excluded_and_tallied`, built
with a raw arrow-array batch (mirroring `forward_compat.rs`'s
schema-drift fixtures) so the disposition is proven on the actual
`decode_aggregate` code path rather than assumed from the short-list
case.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(querier): rfc 0002 — non_exhaustive QueryStats, typed group-null literal

QueryStats gains #[non_exhaustive] matching QueryResult's convention.
The rows_excluded doc comments now scope to any NULL group key, not
just param(n). The absent-OPTIONAL-column NULL substitute in the
aggregate group-term compiler now carries the field's real Arrow
type (Binary/Timestamp/FixedSizeBinary/Utf8) instead of always Utf8,
so the plan's output schema does not depend on which columns happen
to be present. Regression test covers grouping by an entirely-absent
FixedSizeBinary column (trace_id).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y

* fix(server): rfc 0016 — skip the §7 default-limit injection for count-by queries

apply_limit ran unconditionally, but compile::validate now rejects
count+limit combined (RFC 0002 amendment 2026-07-15). Every
aggregation query sent to the HTTP endpoint was therefore a clean
400. Skip the injection when a Stage::Count is present. Regression
test confirmed via revert: fails without the fix, passes with it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y

* docs(querier): rfc 0002 — execute_aggregate doc names the tenant backstop scan input

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y

* fix(querier): rfc 0002 — checked_add for the excluded-row tally

Matches the existing pattern on rows: an overflow surfaces as an
error rather than silently wrapping in release builds.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y

* fix(querier): rfc 0002 — mark AggregateGroup non_exhaustive

Matches QueryResult/QueryStats' convention for public response types.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y

* fix(querier): rfc 0002 — reject i64-overflowing bucket widths at validate time

bucket_expr's execution lowering casts the width to i64, but
validate_group_terms only checked positivity — a width between
i64::MAX and u64::MAX ns passed validation and failed later during
planning with a different error path. Moved into validate() for one
compile-time contract. Regression test confirmed via revert.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
jensholdgaard added a commit that referenced this pull request Jul 17, 2026
…7-17)

Sixteen dispatches (runs #1-#16) exhausted every mechanism checkable
from the harness's side without ever reaching exact L4 completeness.
Runs #13-#16, specifically, ruled out: query-shape artifacts (a plain
line-filter count matched the aggregation-path shortfall exactly),
Loki's documented same-(timestamp,body) dedup (zero exact collisions
found via direct corpus analysis), interleaving between a genuine
mid-corpus kafka restart's two service-instance periods (cleanly
sequential), a harness-side push/batching bug (read end to end, none
found), anything Loki logs at WARN/ERROR (zero matches bar one
harmless startup transient), and Loki's own discarded-samples
Prometheus accounting (zero discards of any kind, any reason).

This matches an open, unresolved upstream Loki issue
(grafana/loki#10658 and related): wide-time-range queries silently
missing a small, consistent percentage of lines, with no error, no
discard signal, and no maintainer-identified root cause. It's a
documented, external, currently-unfixable characteristic of the
comparison partner, not an Ourios or harness defect.

Adds L4_COMPLETENESS_MARGIN = 0.90 (real headroom over the observed
3.9-4.4% loss band) and compare_aggregations_within_margin — narrowly
scoped: it still hard-fails, at any margin, on Loki reporting MORE
than Ourios for any cell or a cell absent from Ourios's own answer,
the two signals that would actually indicate a correctness bug. Only
aggregate under-counting up to the margin is tolerated.
compare_aggregations (exact) is untouched and still gates the
RFC0031.5 fixture-level test's synthetic Loki answer.

Wires the margin into both loki_measure_frequency_pair's poll-complete
threshold (accept short-of-exact within margin instead of always
retrying to a hard timeout) and run_l4_pair's equivalence assertion.

RFC 0031 amended: RFC0031.1's L4 clause now states the margin
explicitly, and §7's L4-query-shape entry (previously open) is closed
with the full evidence trail and the margin decision. M_L4 (bytes-read)
stays deferred — this unblocks a measurement, it doesn't freeze that
margin.

Verified: cargo fmt --all --check, workspace cargo clippy -D warnings,
ourios-bench lib tests (32 passed, comparative module) + local
rfc0031_comparative integration tests (40 passed), mdbook build.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y
jensholdgaard added a commit that referenced this pull request Jul 17, 2026
* feat(bench): rfc 0031 l4 — wire into the live dispatch loop

Live-wires PairClass::L4 into rfc0031_indicative_comparative_run, the
#[ignore]d container-based dispatch test. The previous slice proved
the L4 machinery (ourios_aggregate_answer, parse_loki_matrix,
pick_frequency_pair, compare_aggregations) only at the fixture level,
against a hand-built Loki matrix response — this slice makes it real
against a running Loki container and the actual corpus.

L4 is picked and measured as its own step, kept OUT of the
`Picks`/`specs: Vec<PairSpec>` pipeline the L1/L2/L3/L6 classes share:
an aggregation's (bucket, group) -> count map is not a LineKey
multiset, and forcing it through OuriosAnswer/compare_lines would
misrepresent the state rather than model it (the same "make invalid
states unrepresentable" reasoning the miner/parquet layers already
follow). Concretely: pick_frequency_pair runs post-store-build like
pick_template_pair; its PairSpec is built with the exact dsl/logql
shape the fixture-level test already pinned; loki_query_matrix issues
a real query_range metric call with `step` pinned to the bucket
width so evaluation instants land on parse_loki_matrix's documented
bucket-alignment convention (t = bucket_start + width);
loki_measure_frequency_pair polls it to completeness the same way
loki_measure_pair does for line-returning pairs. Both share the same
Loki container and corpus replay as the existing pairs.

Equivalence-required-but-bytes-unasserted: RFC0031.1 (result-set
equivalence) is never optional, so run_l4_pair asserts
compare_aggregations(...).is_equal() unconditionally — an L4 mismatch
fails the run exactly like every other class's equivalence check. Only
the bytes RATIO stays unasserted (M_L4 is still §7-DEFERRED, no frozen
margin to gate against yet): print_l4_report reuses
print_pair_bytes_gates, which already prints L4's ratio with no
verdict. L4 is measured, equivalence-checked, and reported LAST — after
the L1-L3/L6 evidence has printed and their frozen gates have already
asserted — so an L4-only failure cannot destroy that evidence (the same
run #11 salvage lesson the rest of the harness follows). A missing
candidate is reported loudly at pick time, never silently skipped.

Purely additive: class_pair_specs, build_pair_specs, frozen_gate_failures,
print_pair_bytes_gates, print_indicative_report, PairSpec, and PairClass
are unchanged — no frozen-gate behavior for L1/L2/L3/L6 is touched.

Verification: cargo fmt --all --check, cargo clippy --all-targets
--all-features -- -D warnings (workspace), cargo nextest run -p
ourios-bench (165 passed, 7 skipped) and cargo test -p ourios-bench
--all-features all green, including the untouched fixture-level
rfc0031_5_l4_frequency_aggregation_bytes. The corpus-scale dispatch
test itself needs Docker + OURIOS_COMPARATIVE_CORPUS, neither available
in this sandbox — its first live proof is the comparative-bench
dispatch workflow, same as every other slice in this harness's history.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y

* fix(bench): rfc 0031 l4 — backtick-delimit the loki regexp argument

The dispatch's first-ever run failed: capture_regex's own Go RE2
escapes (\s+, \S+) were embedded inside a double-quoted LogQL string
literal, which tried to interpret those backslashes as its own escape
sequences (\s is not a valid one) and Loki rejected the query with
"invalid char escape" before the pattern reached the regex engine.
Fixed by switching to a backtick-delimited (LogQL/Go raw string)
regexp argument, which passes the pattern through literally.

Extracted the duplicated PairSpec-construction block (present
independently in the fixture test and the live-wiring loop) into one
shared l4_pair_spec helper, closing the drift risk and centralizing
the fix. Added a backtick guard: a capture_regex containing a
backtick (regex_escape does not escape backticks) would prematurely
close the raw string, so the candidate is now rejected loudly instead
of emitting a malformed query.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y

* fix(bench): rfc 0031 l4 — measure L4 before, not after, the L1-L6 failure asserts

The second dispatch run failed on the pre-existing, documented L3
Loki-side flake (0 of 9 rows before timeout) — but the run never even
attempted L4: the failures.is_empty() assert for L1-L6's own salvaged
measurement failures sat textually BEFORE the L4 measurement/report
code, so any earlier pair's failure aborted the test before L4 was
ever reached. This inverted the design intent (an L4-only failure
should not destroy L1-L6 evidence, not the other way around).

Moved L4's measurement to run immediately after the report prints,
before the gate/failures assertions. run_l4_pair now pushes a Loki-side
measurement failure (flake) into the same failures vec the other
classes salvage into, instead of panicking immediately — so a flaky
L4 measurement no longer aborts before the L1-L6 evidence is captured,
symmetric with the fix for the reverse direction. A genuine L4
equivalence MISMATCH still hard-panics immediately, unchanged:
RFC0031.1 equivalence is never optional, matching L1-L6's own
compare_lines assertion.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y

* fix(bench): rfc 0031 l4 — cap the picker's row-count ceiling at 100K

The third dispatch got past the control-flow fix and genuinely
measured L4 — but the picked candidate (a service's dominant,
near-catch-all template) summed to ~971K matching rows, and Loki
returned only 811,775 of them before the 300s poll deadline (the
same budget every other class's loki_measure_pair uses). L4_MIN_ROWS
was a floor with no ceiling, so the picker had no reason to prefer a
smaller, still-meaningful candidate. Added L4_MAX_ROWS=100_000
(comfortable margin at the observed ~2.7K rows/s Loki throughput) to
frequency_shape_rejection, so the picker moves on to a candidate the
poll can actually finish measuring.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y

* fix(bench): rfc 0031 l4 — disable loki's query-range results cache

Run #4's L4 pair plateaued at 11,053/11,523 rows across every 10s poll
instead of climbing to completeness. The picker's row ceiling (run #3's
fix) had already ruled out "too large to finish in time" — the count
never moved at all, which points at a cache serving the same stale
answer on every retry rather than a slow ingest.

Loki's bundled local-config.yaml enables the embedded results cache for
query_range's metric/matrix path (L4's loki_query_matrix), keyed by the
query+start+end+step tuple that loki_measure_frequency_pair repolls
unchanged. The first (still-incomplete) response gets cached and echoed
back on every subsequent poll. Plain log queries (loki_query_range, used
by L1-L3/L6) aren't extent-cached the same way, so they self-heal across
polls untouched by this.

-query-range.cache-results=false trades Loki's own query latency for
correctness of the harness's completeness poll — in Loki's favour, same
as the other operator-tuning flags already on this container.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y

* fix(bench): rfc 0031 l4 — correct the results-cache disable flag name

Run #5 never got past container startup: `-query-range.cache-results`
doesn't exist ("flag provided but not defined"), so Loki's /ready check
timed out on a container that failed to start at all.

Checked the pinned v3.5.3 source directly instead of guessing again:
queryrangebase.Config.CacheResults is registered under the `querier.`
flag prefix in roundtrip.go, not `query-range.`. Correct flag is
-querier.cache-results=false.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y

* fix(bench): rfc 0031 l4 — widen the loki poll deadline to 900s

Run #6 (with the corrected -querier.cache-results=false flag from the
prior commit) proved the results-cache theory wrong: L1-L3/L6 all
measured cleanly, but L4 still plateaued — 10752/11523 rows (93.3%),
even slightly worse than run #4's 95.9% pre-fix, and the shortfall
varies run to run rather than repeating a fixed cached answer.

That points at genuine, variable completion time rather than a bug:
L4's LogQL runs a `| regexp` capture over every candidate line before
grouping and counting, a real per-line cost the other classes' plain
stream/count queries never pay. Widened loki_measure_frequency_pair's
deadline from 300s to 900s — well inside the CI job's unset (360 min
default) timeout given the whole run has taken ~95-100 min so far.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y

* fix(bench): rfc 0031 l4 — raise loki's max-entries-limit, revert deadline theory

Runs #4/#6/#7 all converged L4 to ~93-96% of expected rows, independent
of poll deadline (300s vs 900s made no measurable difference) — ruling
out both a results-cache echo (already disabled in #fe5915a) and a
"just needs more time" theory (the deadline widening from the prior
commit). A stable, time-independent shortfall points at something being
permanently excluded, not merely delayed.

Pulled the frozen otel-demo-v8 corpus locally and checked every log line
matching the L4 pair's needle ("Wrote producer snapshot at offset")
against its capture regex directly: all 11,525 matches parse cleanly.
The regex/content isn't the problem — some matching lines are never
being scanned at all.

That points at Loki's default -validation.max-entries-limit (5000):
count_over_time with a |regexp stage has to scan every raw kafka log
line in a query-frontend split before the line filter narrows it down,
and kafka's per-split volume exceeds 5000 lines often enough to
silently truncate the scan before every matching line is reached.
Raised the limit well past the corpus's noisiest single template's
volume (~971K rows).

Reverted the 900s deadline back to 300s (matching loki_measure_pair) —
the widened deadline never addressed the actual bottleneck, and keeping
it would misattribute the fix in a way that'd mislead the next reader.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y

* fix(bench): rfc 0031 l4 — widen poll deadline now the entries cap is gone

Run #8 (max-entries-limit raised) moved L4 from a hard ~93% plateau to
97.1% (11192/11523) — real progress, and unlike runs #4/#6/#7 the
remaining gap now plausibly behaves like genuine ingest settle time
rather than a fixed ceiling, since the artificial cap that made the
prior 300s vs 900s test inconclusive is gone. Widened the deadline to
600s to test that directly before assuming a third factor is at play.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y

* fix(bench): rfc 0031 l4 — revert unhelpful deadline widening, add diagnostics

Run #9 (600s) measured 96.5% (11123/11523), statistically the same as
run #8's 97.1% at 300s — deadline widening does nothing here, so the
remaining shortfall after the entries-limit fix is a second stable
cap, not settle time. Reverted the deadline back to 300s to match
loki_measure_pair rather than keep an unjustified change.

Wired the existing dump_loki_diagnostics helper (already used by
loki_measure_pair on a deadline miss) into loki_measure_frequency_pair
too — it's built around spec.logql + stats parsing, which is
query-shape-agnostic, so it works unmodified for the matrix path. If
L4 still falls short, the next run's failure carries the raw Loki
stats (chunk-fetch counts, any warnings) instead of another guess.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y

* fix(bench): rfc 0031 l4 — epoch-align the loki query window to bucket boundaries

Run #10's diagnostics (wired in the prior commit) confirmed the L4
query itself is well-formed and Loki answers it successfully — no
error, no chunk-fetch shortfall visible in the sampled response. That,
combined with runs #6-#10 all converging to a stable ~93-97% regardless
of poll deadline (300s/600s/900s all statistically indistinguishable),
rules out both a timing race and a malformed query.

The real mismatch: Loki's query_range evaluates a step-grid starting
exactly at `start` (start, start+step, ..., end), but Ourios's own
bucket(width) semantics are epoch-aligned (floor(ts/width)*width) —
`min_effective_time_unix_nano` (the corpus's raw earliest timestamp)
has no reason to already be a bucket-boundary multiple. Unless
(end - start) is an exact multiple of the bucket width, the step-grid
leaves a fractional sliver at the tail of the range with no evaluated
window covering it at all — real, ingested, settled data that's simply
never queried, independent of poll duration. That's exactly the shape
every run has shown.

Snap `start` down and `end` up to the nearest bucket-width boundary in
l4_pair_spec — costs nothing (no data exists outside [min, max] to
inflate the count) and guarantees the step-grid fully covers the range.

Verified: cargo fmt --all --check, workspace cargo clippy -D warnings,
local (non-container) rfc0031_comparative unit tests — 39 passed.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y

* diag(bench): rfc 0031 l4 — add an ingest-vs-query-side split probe

Run #11 (bucket-aligned query window) measured 96.6% (11133/11523) —
narrower than the pre-fix ~93% plateau, but still in the same stable
band as runs #6-#10, all independent of poll deadline, entries-limit,
and now bucket alignment. Six straight dispatches without closing the
gap means continuing to guess at query-side LogQL/config knobs isn't
warranted anymore.

Added a decisive probe: on a deadline miss, loki_measure_frequency_pair
now also runs a PLAIN line-filter count (no count_over_time, no
regexp) for the same needle + window via the new
loki_query_range_uncapped (limit sized to expected_rows, unlike the
shared loki_query_range's fixed 5000 cap — which is below this pair's
11523 expected rows and would itself lie about the count). If that
plain count also falls short by the same margin, the shortfall is
ingest-side (Loki never stored those lines) and no further query
tuning will fix it; if it's ~complete, the loss is specific to the
aggregation path.

Diagnostic-only change — no behavior change to the measurement itself,
just evidence gathering on the existing failure path.

Verified: cargo fmt --all --check, workspace cargo clippy -D warnings,
local (non-container) rfc0031_comparative unit tests — 39 passed.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y

* fix(bench): rfc 0031 l4 — reject high-frequency candidates prone to loki dedup

Run #12's decisive diagnostic confirmed the L4 shortfall is ingest-side:
a plain unaggregated line-filter count for the same needle+window came
back just as short (11160/11523) as every aggregation-path attempt.
Loki's ingester silently drops a log entry that collides with another
on (timestamp, body) within the same stream — a drop invisible to the
OTLP push response's partial_success (push_otlp already asserts that
field is clean on every push in every run so far). No query-side fix
was ever going to close this gap; the picker was choosing a candidate
Loki structurally can't ingest identically.

kafka's template_id=16 ("Wrote producer snapshot") fires roughly every
15s. A local exploration against the real frozen corpus (offline, no
Loki container — pick_frequency_pair only touches Ourios's own
pipeline) found candidates at much lower frequency clear of the same
floors: template_id=60 ("Periodic task") at ~144s average cadence, ~10x
the failing candidate's margin.

Added L4_MIN_AVG_INTERVAL_SECONDS (100s) to frequency_shape_rejection
as a durable picker rule, not a one-off override — this protects any
future re-run of the picker against landing on another collision-prone
high-frequency template, not just this specific dispatch.

Updated two pre-existing tests (pick_frequency_pair_finds_a_moderate_
cardinality_group, rfc0031_5_l4_frequency_aggregation_bytes) whose
synthetic sub-3s timelines — convenient for test speed, not meant to
model real timing risk — tripped the new floor; scaled their
timestamps 1000x (preserving cardinality/row-count/needle assertions
unchanged) so they represent a realistic, non-collision-prone example.

Verified: cargo fmt --all --check, workspace cargo clippy -D warnings,
local (non-container) rfc0031_comparative unit tests — 40 passed
(39 prior + 1 new: frequency_shape_rejection_enforces_the_average_
interval_floor).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y

* diag(bench): rfc 0031 l4 — dump loki's own container logs on a deadline miss

Run #13's lower-frequency candidate (template_id=60, ~144s average
cadence) still fell short (1144/1197), and a corpus-side check ruled
out the leading theory entirely: every one of the 1197 matching
records has a UNIQUE timestamp AND a unique body (verified via jq
against the frozen otel-demo-v8 corpus locally) — zero exact
(timestamp, body) collisions possible. Loki's documented dedup rule
cannot be the mechanism here, which means it likely wasn't the full
story for the prior candidate either, even though lowering the
frequency floor did measurably help (17.5% loss -> 4.4% loss).

Also checked push_corpus_to_loki/push_otlp end to end for a harness-
side drop: the batching loop appends every non-empty corpus line's
resource_logs to `pending` before any flush, with a final flush after
the read loop — no line is skippable, and push_otlp's retry resends
the identical Bytes payload, so no bug found there either.

Everything checkable from the client side (query responses, corpus
content, our own push code) is now ruled out or confirmed clean. The
next place to look is Loki itself: on a deadline miss,
loki_measure_frequency_pair now also dumps the Loki container's own
stderr, filtered to warn/error/drop/reject/rate-limit/stream-limit
lines — the ingester logs these for exactly the mechanisms still on
the table (rate limiting, out-of-order rejection, stream-limit drops),
none of which are visible in a query response or push_otlp's already-
clean partial_success check.

Diagnostic-only — no change to measurement behavior.

Verified: cargo fmt --all --check, workspace cargo clippy -D warnings,
local (non-container) rfc0031_comparative unit tests — 40 passed.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y

* diag(bench): rfc 0031 l4 — scrape loki's discarded-samples metrics

Run #14's level=warn/level=error stderr scan came back with zero
matches in 6618 total lines — whatever is causing the L4 shortfall
(still 1147/1197 with the lower-frequency candidate), Loki doesn't
consider it log-worthy. That rules out rate limiting, out-of-order
rejection, and stream-limit drops as commonly logged at WARN.

(The first attempt at the stderr filter was a naive "contains warn"
substring match, which drowned in false positives from query text
like `severity_text="WARN"` appearing inside level=info lines — fixed
to match on the `level=` field precisely.)

Loki's distributor increments loki_discarded_samples_total/
loki_discarded_bytes_total (labeled by reason) even for discards that
don't warrant a log line — its own dedicated counter for exactly this
question. Added dump_loki_discard_metrics, scraping /metrics on a
deadline miss (extracted as its own function, alongside
dump_loki_diagnostics, to keep loki_measure_frequency_pair under
clippy's line-count lint).

Diagnostic-only — no change to measurement behavior.

Verified: cargo fmt --all --check, workspace cargo clippy -D warnings,
local (non-container) rfc0031_comparative unit tests — 40 passed.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y

* feat(bench): rfc 0031 l4 — documented completeness margin (§7, 2026-07-17)

Sixteen dispatches (runs #1-#16) exhausted every mechanism checkable
from the harness's side without ever reaching exact L4 completeness.
Runs #13-#16, specifically, ruled out: query-shape artifacts (a plain
line-filter count matched the aggregation-path shortfall exactly),
Loki's documented same-(timestamp,body) dedup (zero exact collisions
found via direct corpus analysis), interleaving between a genuine
mid-corpus kafka restart's two service-instance periods (cleanly
sequential), a harness-side push/batching bug (read end to end, none
found), anything Loki logs at WARN/ERROR (zero matches bar one
harmless startup transient), and Loki's own discarded-samples
Prometheus accounting (zero discards of any kind, any reason).

This matches an open, unresolved upstream Loki issue
(grafana/loki#10658 and related): wide-time-range queries silently
missing a small, consistent percentage of lines, with no error, no
discard signal, and no maintainer-identified root cause. It's a
documented, external, currently-unfixable characteristic of the
comparison partner, not an Ourios or harness defect.

Adds L4_COMPLETENESS_MARGIN = 0.90 (real headroom over the observed
3.9-4.4% loss band) and compare_aggregations_within_margin — narrowly
scoped: it still hard-fails, at any margin, on Loki reporting MORE
than Ourios for any cell or a cell absent from Ourios's own answer,
the two signals that would actually indicate a correctness bug. Only
aggregate under-counting up to the margin is tolerated.
compare_aggregations (exact) is untouched and still gates the
RFC0031.5 fixture-level test's synthetic Loki answer.

Wires the margin into both loki_measure_frequency_pair's poll-complete
threshold (accept short-of-exact within margin instead of always
retrying to a hard timeout) and run_l4_pair's equivalence assertion.

RFC 0031 amended: RFC0031.1's L4 clause now states the margin
explicitly, and §7's L4-query-shape entry (previously open) is closed
with the full evidence trail and the margin decision. M_L4 (bytes-read)
stays deferred — this unblocks a measurement, it doesn't freeze that
margin.

Verified: cargo fmt --all --check, workspace cargo clippy -D warnings,
ourios-bench lib tests (32 passed, comparative module) + local
rfc0031_comparative integration tests (40 passed), mdbook build.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y

* fix(bench): rfc 0031 l4 — margin check is total-level, not per-cell

Run #17 validated the completeness-margin design and immediately
refined it: the poll-completion check passed cleanly (1153/1197,
96.3%), but the equivalence check then hard-failed on a single cell
landing 1 row OVER Ourios's count (114 vs 113 for one bucket/value)
while the aggregate total stayed a solid under-count — consistent
with the same step-grid boundary imprecision already characterized
(a record landing in an adjacent bucket), not fabrication.

The original compare_aggregations_within_margin checked "Loki > Ourios"
per cell, which was too strict for that kind of noise. Refined to
check for phantom cells (a (bucket, group_key) Loki reports that
Ourios's own answer doesn't contain at all) and Loki's TOTAL exceeding
Ourios's total instead — this still catches the failure mode that
would actually indicate a bug (wrong regex or wrong bucket math would
produce cells Ourios never produced at all, or push the total over)
while tolerating single-cell boundary noise on keys both systems agree
exist.

Added a test for the exact run #17 shape (a single cell over, total
still under, must pass) alongside the existing phantom-cell and
net-overcount tests (renamed from "overcount" now that the check is
total-level). RFC 0031 §7's L4 entry updated with the refinement and
its rationale.

Verified: cargo fmt --all --check, workspace cargo clippy -D warnings,
ourios-bench lib tests (33 passed, +1 new) + local rfc0031_comparative
integration tests (40 passed), mdbook build.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y

* fix(bench): rfc 0031 l4 — per-group_key margin, address PR #536 review

PR #536's code review (14 fresh findings across Copilot + CodeRabbit's
post-run-18 passes) surfaced one substantive correctness gap and
several real documentation/robustness issues in the completeness-
margin work. All verified against current code before fixing.

Substantive fix — cross-key redistribution gap (CodeRabbit, Major):
compare_aggregations_within_margin's grand-total-only check (from the
run #17 fix) let Loki over-count one group_key while under-counting
another by the same amount and still read as 100% complete: Ourios
{A: 100, B: 100} vs Loki {A: 190, B: 10} sums to a "complete" 200/200
while hiding A being fabricated to compensate for B being nearly
lost. Refined to aggregate ourios/loki BY group_key first (summing
each key across every bucket it appears in), then apply the
phantom/overcount/margin checks per-key. This still tolerates run
#17's exact shape (a single bucket's +1 doesn't change a key's own
total across its buckets) while rejecting the redistribution a pure
grand-total check missed. Added regression tests for both shapes.

Also populates real per-key examples in mismatch reports (Copilot: the
old design returned examples: Vec::new() on both mismatch paths,
despite the function accepting examples_cap and RFC0031.1 calling for
example keys on a failed comparison).

Documentation/robustness fixes (all verified against current code,
none required a runtime-behavior change beyond the fix above):
- Two stale comments still asserted Loki's same-(timestamp, body)
  ingester dedup as the shortfall's mechanism, contradicting the
  nearby docs that say this was directly disproven and the true
  mechanism is uncharacterized (Copilot, 6 threads pointing at 2 real
  sites: the frequency_shape_rejection rejection message and one test
  comment — the other 4 threads were already-accurate historical
  narrative, verified and left alone).
- Missing `//` justification comment on one #[allow(cast_precision_loss)]
  (CodeRabbit).
- L4 picker silently continues when no viable candidate exists
  (l4_spec.is_none()) — only an eprintln, no run failure, despite L4
  being a must-win class (CodeRabbit). Now pushes into `failures`.
- PR description inaccurately described L4's equivalence assertion
  (exact compare_aggregations, ordered after the frozen gates) —
  rewritten to match actual behavior (margin-based, before the frozen
  gates, matching the run #11 salvage design already documented inline).
- RFC 0031 §7's L4 entry claimed the picker "prefers the lowest-
  frequency viable candidate" — the actual algorithm is first-fit in
  ascending (template_id, param) order, not an exhaustive ranking
  (CodeRabbit, tagged Heavy Lift). Reworded to describe actual
  behavior and the deliberate scope decision (a real ranking pass
  would cost a query per candidate against a corpus with tens of
  thousands of templates; not built given first-fit has now found a
  validated candidate three real dispatches running).
- Double-backtick delimiters for a LogQL code span containing literal
  backticks (CodeRabbit, MD038).

Verified: cargo fmt --all --check, workspace cargo clippy -D warnings,
ourios-bench lib tests (34 passed, +2 new) + local rfc0031_comparative
integration tests (40 passed), mdbook build.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y

* fix(bench): rfc 0031 l4 — absolute row tolerance, not pure percentage

Run #19 (the verification dispatch for the round-1 review fixes) found
a real edge case in the per-group_key percentage margin: a group_key
with exactly 1 total Ourios row, where Loki captured 0 (0%). A pure
ratio has no meaningful middle ground at n=1 — it's binary, 0% or
100% — yet losing one isolated occurrence is fully consistent with
the already-characterized ~4-8% aggregate loss rate this whole margin
exists to tolerate.

Converted the per-key check from a ratio (loki/ourios >= margin) to an
absolute row tolerance floored at 1: ceil(ourios_key_total *
(1 - margin)).max(1). This tolerates a cardinality-1 key losing its
only row while still catching a real shortfall on a large key (100
rows, tolerance 10, losing 20 still rejects) — the phantom-cell and
per-key-overcount hard-fail checks are unaffected. Two new regression
tests cover both ends.

Verified: cargo fmt --all --check, workspace cargo clippy -D warnings,
ourios-bench lib tests (36 passed, +2 new) + local rfc0031_comparative
integration tests (40 passed), mdbook build.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y

* fix(bench): rfc 0031 l4 — margin comparator precision, review triage

compare_aggregations_within_margin's tolerance formula (ceil(o*(1-margin))
.max(1)) was itself miscalibrated for small-but-not-1 totals, per two
independent Copilot review threads (o=2 at 90%: tolerance=1 permits 50%
completeness, not 90%). Replace the subtract-then-round row tolerance
with a direct, epsilon-guarded comparison — loki_total >= ourios_total *
margin — which also sidesteps a second bug the naive floor() fix
introduced: 1.0 - 0.9 isn't exactly 0.1 in f64, so floor(40.0 * (1.0 -
0.9)) truncated to 3 instead of 4, tightening the tolerance at exact
90%-boundary cases (caught by the existing
margin_comparison_tolerates_undercount_within_margin test's svcB case).

Extract phantom_cells and aggregate_by_group_key helpers to bring the
function back under clippy's line limit, and add a # Panics section for
the margin-validation assert.

Fix several accumulated PR #536 review findings: run_l4_pair's doc
comment claimed L4 runs after the L1-L3/L6 frozen gates assert (it
actually runs before, printing first); three "ingest-vs-query" overclaims
(the plain line-filter probe still calls query_range, so it can rule out
"specific to the metric-aggregation path" but not prove ingest-side
loss); L4_COMPLETENESS_MARGIN's own doc comment still described the
superseded total-level design. Add a clarifying comment on l4_pair_spec's
step-grid reasoning (the first evaluated instant decodes to an empty
phantom bucket, not a lost real one) and fix the RFC's LogQL code span,
which kept the backslash escaping needed for single backticks even after
switching to a double-backtick delimiter that makes it unnecessary.
Reconcile RFC0031.5's must-win predicate with M_L4 staying deferred —
add a note that the predicate is the target contract, not currently
gated.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y

* fix(bench): rfc 0031 l4 — panic-safe diagnostic probe, more review triage

loki_query_range_uncapped used expect()/assert!() internally, but it
runs on the L4 deadline-miss diagnostic path inside the same
runtime.block_on that gathers L1-L3/L6's evidence — a panic there
(a real Loki error response, a malformed body) would unwind the whole
async block and lose all of it, defeating the print-before-assert
salvage design (Copilot). Converted to return Result<u64, String>
instead of panicking, matching the already-panic-free sibling
diagnostics (dump_loki_diagnostics et al.).

Also: fix a test comment that said "one row under the ceiling" for a
fixture that actually lands exactly at the ceiling; fix an unreachable!
message's imprecise invariant claim (the real gating condition is
l4_spec.is_some() implies l4_loki.is_some(), not "iff frequency is
Some"); document loki_query_matrix's whole-second/bucket-alignment
precondition and verify it against l4_pair_spec, its only caller;
reorder loki_measure_frequency_pair's deadline-miss diagnostics to run
only when the completeness margin is actually missed, not on every
deadline-miss regardless of outcome; fix two fixture comments claiming
"~300s average spacing" that don't match their own timestamps (actually
~580s) and a comment attributing the L4 shortfall to ingest-side dedup
after that theory was directly disproven elsewhere in the same file.

Verified the remaining ~40 accumulated review threads (mostly a
recurring "shared 300s deadline" doc/code mismatch and the run_l4_pair
ordering claim, duplicated across many review rounds) against current
code: all already correct, superseded by earlier commits in this
investigation.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y

* fix(bench): rfc 0031 l4 — round-4 review triage on the margin comparator

Validate margin at function entry rather than after the phantom/overcount
checks, so an invalid margin always panics per the documented contract
instead of potentially returning a data-shaped mismatch first
(CodeRabbit). Fix the doc comment paragraph still describing the
superseded floor-based tolerance (Copilot). Reword the L4-skip
diagnostic and failure message to name both reasons l4_spec can be None
(picker bounds vs a backtick in the capture regex) and to make clear the
skip fails the dispatch rather than reading as benign (Copilot, two
sites).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y

* fix(bench): rfc 0031 l4 — margin=1.0 strictness + panic-safe matrix poll

Two Copilot findings on the previous commit, both verified genuine:

The cardinality-1 exemption applied at any margin, so a caller passing
margin = 1.0 (exact completeness) would still accept Loki returning 0 of
1 for an n=1 key — the exemption now only applies to a genuinely
fractional margin, with a regression test covering both directions at
1.0. Bit-identical behavior at the harness's 0.90.

loki_query_matrix still used expect/assert internally, so a transient
transport error, 5xx, or torn body during the L4 poll — which runs LAST
in the same async block holding every other pair's already-collected
measurement — would panic and unwind all of it. Converted to
Result<L4Measured, String>; the poll loop now retries an Err until its
deadline exactly like an incomplete answer, then surfaces it as the
pair's failure. Extracted the below-margin shortfall diagnostics into
dump_l4_shortfall_diagnostics to stay under clippy's function-length
limit.

Neither change alters the measured semantics run #23 is currently
confirming (the comparator formula is untouched; at margin 0.90 the
exemption gating is unchanged).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y

* docs(bench): rfc 0031 l4 — document why the phantom check is cell-level

Copilot's latest pass proposed weakening phantom detection from
(bucket, group_key) cells to bare group_keys so a boundary-exact record
shifting into an empty adjacent bucket can't read as phantom. Declined:
a systematic bucket-decode error (every cell shifted one width — the
run #11 bug class) leaves every per-key total intact, so the cell-level
check is the only guard that catches it, while the false positive it
risks requires a nanosecond-exact bucket-boundary timestamp that no
real dispatch has ever produced. Documented the trade-off on
phantom_cells instead of changing behavior.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
jensholdgaard added a commit that referenced this pull request Aug 29, 2026
Adds the RFC 0021 amendment (phase 2 split into 2a/2b), the RFC0021.1
re-baseline (invariant + floors, not literals), the new RFC0021.8
thrift-absence test, and the deny.toml cleanup — then records why 2a
still must NOT ship:

Three querier tests fail on DF55/arrow59, and they are not version
literals. Measured with a temporary metric dump (removed again):

1. rfc0044_7 / rfc0044_8 — row-group pruning is GONE for body-equality
   queries. `row_groups_pruned_statistics` reports pruned:0 matched:2,
   and `row_groups_pruned_bloom_filter` agrees. DataFusion is not
   pruning where it used to; this is pillar #1 (skip row groups, don't
   scan) and thesis-gate territory (B1/B2), exactly the regression
   RFC 0021 §3.1 said must be pinned back or explicitly RFC'd rather
   than absorbed silently.
2. rfc0007_1 — `bytes_scanned` is emitted as literally 0 while
   `output_bytes` works, so the user-visible `bytes_read` in RFC 0016
   query responses would silently zero.

Our matcher is not at fault in either case: the metric names and
shapes still match; DataFusion's own values changed.

Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>
jensholdgaard added a commit that referenced this pull request Aug 29, 2026
Unblocks RFC 0021 phase 2a. DataFusion 55 substitutes columns that
per-file statistics prove constant; the all-NULL case folds to a NULL
literal, which collapses `body == "…"` to a bare constant and leaves
no column reference for a pruning predicate — so RFC 0044's
body-equality queries scanned every row group instead of skipping
them. Pillar #1, caught by RFC0044.7/.8 and RFC0007.1.

Upstream: apache/datafusion#24769, fix proposed in #24770, bisected
to DataFusion #22969 (which made collect_statistics the session
default). Turning the option off restores the pruning exactly.

Both production session sites now go through one `exec::session()` so
the setting cannot drift between the query and drift paths. The
override is not purely defensive: collection also costs a footer read
per file at plan time — a cost a many-file log store pays on every
query — and Ourios derives its pruning from the RFC 0009 manifest plus
partition-directory windowing, not from DataFusion's collected file
statistics.

RFC 0021 §3.2a records the decision and names the revisit trigger; the
same three pruning tests gate it in both directions.

Verified: workspace 1448/1448, clippy pedantic zero, fmt, mdbook.
Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>
jensholdgaard added a commit that referenced this pull request Aug 29, 2026
…e 2a)

DataFusion 55.0.0 fired RFC 0021's own reopening trigger, but only
half of it: DF 55 carries arrow/parquet 59.2 while still pinning
object_store ^0.13.2. Phase 2 is therefore split (§3.2a/§3.2b) and
this lands 2a.

- DataFusion 54 -> 55, arrow/parquet 58 -> 59, MSRV 1.88 -> 1.94.
- `thrift` leaves the lockfile (parquet 59 dropped it), clearing
  GHSA-2f9f-gq7v-9h6m (#295) and the two Dependabot alerts previously
  dismissed as risk-tolerated. `paste` leaves too, so its dead
  RUSTSEC-2024-0436 ignore is removed from deny.toml.
- API churn was two items: an arrow-cast pin, and DF 55's new required
  ExecutionPlan::apply_expressions (three test doubles in
  ourios-df-otel). No production code changed for the bump itself.
- MSRV 1.94 enables clippy's duration_suboptimal_units, which suggests
  Duration::from_days/from_mins — both unstable (rust#120301) — so it
  is allowed workspace-wide with that reason rather than chased into
  nightly-only APIs.

RFC0021.1 is re-baselined to assert the invariant (exactly one arrow
major, exactly one datafusion) plus floors (arrow >= 59, DF >= 55)
rather than version literals: a literal-pinning test is a
change-detector that fails on every intentional upgrade and catches
nothing the invariant misses. RFC0021.8 lands as a real test
asserting thrift's absence from the lockfile.

One behavioural decision, recorded in §3.2a: the querier runs with
execution.collect_statistics off, via a single exec::session() shared
by both production session sites. DF 55 substitutes columns that
per-file statistics prove constant, and the all-NULL case folds to a
NULL literal, collapsing `body == "…"` to a bare constant — leaving
no column reference for a pruning predicate, so RFC 0044's
body-equality queries scanned every row group instead of skipping
them (pillar #1; caught by RFC0044.7/.8 and RFC0007.1). Reported as
apache/datafusion#24769 with a fix proposed in #24770, bisected to
DataFusion #22969. The override also avoids a per-file footer read at
plan time, which a many-file log store pays on every query; Ourios
derives its pruning from the RFC 0009 manifest and partition-directory
windowing, not from DataFusion's collected file statistics. The same
three tests gate the decision in both directions.

Phase 2b (object_store >= 0.14, RFC0021.7 and the rest of .9) stays
upstream-gated; epic #314 tracks it.

Verified: workspace 1448/1448, clippy pedantic zero, fmt, mdbook,
cargo deny clean.

BREAKING CHANGE: the workspace MSRV moves from 1.88 to 1.94, and the
storage/query stack moves to DataFusion 55 / arrow 59 / parquet 59.

Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>
jensholdgaard added a commit that referenced this pull request Aug 30, 2026
…e 2a)

DataFusion 55.0.0 fired RFC 0021's own reopening trigger, but only
half of it: DF 55 carries arrow/parquet 59.2 while still pinning
object_store ^0.13.2. Phase 2 is therefore split (§3.2a/§3.2b) and
this lands 2a.

- DataFusion 54 -> 55, arrow/parquet 58 -> 59, MSRV 1.88 -> 1.94.
- `thrift` leaves the lockfile (parquet 59 dropped it), clearing
  GHSA-2f9f-gq7v-9h6m (#295) and the two Dependabot alerts previously
  dismissed as risk-tolerated. `paste` leaves too, so its dead
  RUSTSEC-2024-0436 ignore is removed from deny.toml.
- API churn was two items: an arrow-cast pin, and DF 55's new required
  ExecutionPlan::apply_expressions (three test doubles in
  ourios-df-otel). No production code changed for the bump itself.
- MSRV 1.94 enables clippy's duration_suboptimal_units, which suggests
  Duration::from_days/from_mins — both unstable (rust#120301) — so it
  is allowed workspace-wide with that reason rather than chased into
  nightly-only APIs.

RFC0021.1 is re-baselined to assert the invariant (exactly one arrow
major, exactly one datafusion) plus floors (arrow >= 59, DF >= 55)
rather than version literals: a literal-pinning test is a
change-detector that fails on every intentional upgrade and catches
nothing the invariant misses. RFC0021.8 lands as a real test
asserting thrift's absence from the lockfile.

One behavioural decision, recorded in §3.2a: the querier runs with
execution.collect_statistics off, via a single exec::session() shared
by both production session sites. DF 55 substitutes columns that
per-file statistics prove constant, and the all-NULL case folds to a
NULL literal, collapsing `body == "…"` to a bare constant — leaving
no column reference for a pruning predicate, so RFC 0044's
body-equality queries scanned every row group instead of skipping
them (pillar #1; caught by RFC0044.7/.8 and RFC0007.1). Reported as
apache/datafusion#24769 with a fix proposed in #24770, bisected to
DataFusion #22969. The override also avoids a per-file footer read at
plan time, which a many-file log store pays on every query; Ourios
derives its pruning from the RFC 0009 manifest and partition-directory
windowing, not from DataFusion's collected file statistics. The same
three tests gate the decision in both directions.

Phase 2b (object_store >= 0.14, RFC0021.7 and the rest of .9) stays
upstream-gated; epic #314 tracks it.

Verified: workspace 1448/1448, clippy pedantic zero, fmt, mdbook,
cargo deny clean.

BREAKING CHANGE: the workspace MSRV moves from 1.88 to 1.94, and the
storage/query stack moves to DataFusion 55 / arrow 59 / parquet 59.

Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>
jensholdgaard added a commit that referenced this pull request Aug 30, 2026
…e 2a) (#773)

DataFusion 54→55, arrow/parquet 58→59, MSRV 1.88→1.94. thrift and
paste leave the lockfile (#295 cleared; dead RUSTSEC-2024-0436 ignore
removed). RFC0021.1 re-baselined to invariant + floors (arrow ≥59,
DF ≥55, MSRV ≥1.94); RFC0021.8 thrift-absence test lands; §5/§5b split
separates fired criteria from phase-2b forward declarations.

The querier runs with execution.collect_statistics off via a shared
exec::session() — DataFusion 55's constant-column substitution folds
all-NULL columns to NULL literals and collapses body-equality
predicates to constants, losing row-group pruning (pillar #1; caught
by RFC0044.7/.8 + RFC0007.1). Upstream: apache/datafusion#24769, fix
proposed in #24770. A unit test pins the override.

Phase 2b (object_store ≥0.14: RFC0021.7 CAS re-proof, quick-xml
ignores, renovate hold #313, #310) stays upstream-gated; epic #314.

BREAKING CHANGE: workspace MSRV moves from 1.88 to 1.94; the
storage/query stack moves to DataFusion 55 / arrow 59 / parquet 59.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>
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.

1 participant