docs: apply RFC maturity-model amendments - #2
Closed
jensholdgaard wants to merge 1 commit into
Closed
Conversation
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>
This was referenced Apr 28, 2026
jensholdgaard
added a commit
that referenced
this pull request
May 19, 2026
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>
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
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>
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>
4 tasks
jensholdgaard
added a commit
that referenced
this pull request
Jun 7, 2026
Review pass 4 (internal consistency): drop drift from the §3.3 shared list and principle #4 (it is deferred per §6.3; drift-alias is resolves_to); fix the §6.2 hex wording (hex strings parsed case-insensitively, canonical lowercase — was self-contradictory); scope severity_name to a severity RHS in the §7 EBNF (split comparison into severity_cmp | scalar_cmp so `service == error` is not grammatical); cite CLAUDE.md §4.6 in principle #2. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jensholdgaard
added a commit
that referenced
this pull request
Jun 8, 2026
`service_of(&record.resource_attributes)` scans the resource attributes and allocates a fresh `String` every call. It was invoked from each hot-path helper (overflow retention, parse-failure, tokenizer-failure, emit), so a single ingested line re-scanned and re-allocated the service identity up to four times. Resolve it once at the top of `ingest` and thread it as `&str` through `ingest_string` / `ingest_structured` / `attach_and_maybe_widen` and the four helpers. Behaviour is identical — the same per-(tenant, service) instruments are driven with the same value — but the ingest hot path (pillar #2) now does one scan + one alloc per line. Also aligns the cluster.rs telemetry doc-comments touched by these helpers to the dotted `ourios.miner.*` names. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Jun 13, 2026
jensholdgaard
added a commit
that referenced
this pull request
Jun 14, 2026
* docs(rfc-0012): draft meta-RFC for CLAUDE.md §2 pillar-#2 wording A meta: RFC (CLAUDE.md is load-bearing; its footer requires a meta: RFC + majority maintainer approval). Pillar #2 currently frames the 50–200× compression as on-disk bytes "before any byte-level codec runs", which RFC 0011 (accepted) refuted — it's a logical reduction whose value is query pruning (B1/B2), not on-disk bytes vs zstd (A1 is a diagnostic). The RFC specifies the exact before/after pillar-#2 wording, the coupled benchmarks.md §2 "Why this bar" reconciliation, §5 doc-state acceptance criteria (RFC0012.1–.3), and the open questions (majority approval; whether §1's thesis sentence wants a parallel touch; the footer changelog line). Per CLAUDE.md §8.5 the edit is NOT made in-session — drafted only; a maintainer enacts after approval. Status: drafted. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(rfc-0012): reference Scenario RFC0012.2, not "§5.2" (Copilot) §6 said "the §5.2 grep"; §5.2 is not a section. The grep-based check lives in Scenario RFC0012.2 — reference the scenario id directly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(rfc-0012): widen scope to all coupled byte-level echoes (Copilot) Copilot noted §1 claimed "the one coupled paraphrase" but a third spot carries the same on-disk framing: RFC 0001 §1's summary ("compression target is 50–200× over raw bytes before any byte-level codec runs"). Reword §1, expand §3.2 to cover both benchmarks.md §2 and RFC 0001 §1 (the latter flagged as an accepted-RFC factual prose fix, maintainer's call at enactment), and broaden the RFC0012.2 grep to repo-wide. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(rfc-0012): complete the echo set + precise framing grep (Copilot) Three valid review points: - §3.1 referenced brittle CLAUDE.md line numbers → use the section/item reference instead. - §3.2 missed a fourth echo: README.md's template-miner bullet ("before any byte-level codec runs") — added; the authoritative list is now whatever the RFC0012.2 framing-grep surfaces. - RFC0012.2's grep was too broad (`50.200` matches legit logical-reduction mentions in roadmap/0005). Narrowed to the framing phrases ("before any byte-level codec" / "over a competent byte codec"), which match exactly the four spots; the bare figure is explicitly left alone. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(rfc-0012): refer to the 50–200× figure, not the regex form (Copilot) RFC0012.2 said "the bare `50.200` figure" using the grep regex form, which reads oddly beside "the 50–200× figure". Reworded to "the framing, not the 50–200× figure itself" — no `50.200` in the prose. 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
Jun 14, 2026
…vel (#203) * docs: enact RFC 0012 — §2 pillar-#2 is a logical 50–200×, not byte-level Maintainer-approved enactment of RFC 0012 (the meta: RFC; CLAUDE.md's footer requires meta: RFC + majority approval — satisfied). - CLAUDE.md §2 pillar #2: the 50–200× is reworded as a **logical** reduction whose payoff is query pruning (gates B1/B2), not on-disk bytes vs a byte codec; A1 named a recorded diagnostic (RFC 0011). The "single biggest engineering risk" sentence is preserved verbatim. - CLAUDE.md footer: 2026-06-14 changelog line + Last-updated bump. - Coupled reconciliations (RFC 0012 §3.2): benchmarks.md §2 A1 "Why this bar" → "Why recorded (diagnostic)"; README.md template-miner bullet; RFC 0001 §1 summary (accepted-RFC factual reword + RFC 0011 pointer). - RFC 0012 → `accepted`. After this, a repo grep for the framing phrases ("before any byte-level codec" / "over a competent byte codec") returns only RFC 0011 and RFC 0012 (which quote the old wording to describe the change) — RFC0012.2 satisfied. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs: link RFC 0011 in README + rewrap CLAUDE.md footer (Copilot) Two format nits on the RFC 0012 enactment: link the README's RFC 0011 reference like its other RFC bullets, and rewrap the CLAUDE.md footer so "This document is load-bearing…" is its own wrapped line rather than jammed after the new changelog sentence. 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
Jun 14, 2026
) Resolves RFC 0012 §7's open question (maintainer-directed). The §1 thesis sentence (CLAUDE.md) and README's parallel both list "collapses … the compression layer …"; add a one-clause clarification to each that the compression collapsed is Parquet's byte codec plus the miner's *logical* reduction (query pruning), not on-disk bytes that beat a codec. The load-bearing thesis sentence itself is unchanged; only the clarifying clause is added. CLAUDE.md footer changelog extended to note §1; RFC 0012 §7 open-question marked resolved. Within RFC 0012's approved meta scope. Co-authored-by: Claude Opus 4.8 (1M context) <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>
This was referenced Jul 3, 2026
Merged
This was referenced Jul 9, 2026
jensholdgaard
added a commit
that referenced
this pull request
Jul 10, 2026
* docs(bench): reconcile §9.12 with the resolved #444 decision The C2 gate landed per-service in #451 (option 3), but §9.12 still framed #444 as an open three-way fork ("tokenizer masking vs. length-aware thresholding vs. accept-and-scope — an RFC-level pillar #2 decision") and called it "a decision, not a performance one". Update both spots to the resolution: option 3 shipped (per-service gate, RFC 0006 §3.4.3), masking parked as a future strategic RFC (a Collector transform/redaction processor can polish infra tokens upstream), length-thresholding rejected; #444 accepted the query-capability tradeoff on hostile infra logs. Doc-only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X467Druw4cQeEPVp52DTG3 * docs(bench): clarify §9.12 re-score date and the template-probe recovery CodeRabbit review on #452: - The §9.12 heading is the run date (2026-07-09); the #444 resolution is 2026-07-10. Flag the C2 re-score date in the heading so the two aren't read as contradictory. - "no single template query recovers it" understated the measurement — a single template_id probe recovers that id's slice (11,523 rows), just not the full dominant event. Reworded. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X467Druw4cQeEPVp52DTG3 * docs(bench): short §9.12 heading + note; unslash transform/redaction Copilot review on #452: - The re-score annotation made the §9.12 heading long/noisy (ToC + anchor slug). Reverted to the short heading and moved the note to an italic sentence right below (CodeRabbit's "explicitly state" option). - "transform`/`redaction` processor" read as one processor and broke oddly across the slash line-break; now "transform or redaction". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X467Druw4cQeEPVp52DTG3 * docs(bench): clarify template_id 1 is a run-specific id, not canonical Copilot review on #452: "template_id == 1" read as if 1 were a stable canonical template. Reworded to "cart's dominant template (id 1 in this run — a run-specific identifier, not a canonical one)". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X467Druw4cQeEPVp52DTG3 * docs(bench): drop positional "below" in the §9.12 gate reference Copilot review on #452: "the per-service gate below" is a positional reference that misleads if the paragraph moves (the gate is introduced earlier in the section). Reference it directly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X467Druw4cQeEPVp52DTG3 * docs(bench): make the #444 antecedent explicit in §9.12 Copilot review on #452: "#444 resolved it" could read as resolving the red-herring failure-flag confound rather than the fragmentation decision. Reword to "#444 settled how to handle the fragmentation". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X467Druw4cQeEPVp52DTG3 * docs(bench): use "length-aware thresholding" consistently in §9.12 Copilot review on #452: the same #444 option was named "length-aware thresholding" then "length-thresholding". Use the full term both times. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X467Druw4cQeEPVp52DTG3 --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jul 11, 2026
jensholdgaard
added a commit
that referenced
this pull request
Jul 11, 2026
#476) * fix(bench): byte-capped Loki push batching (run #2 finding) Run #2 (29162850605) reached 200k/690k batches before a count-capped push (500 LogsData) encoded to 5.28 MB and hit Loki's stock 4 MiB internal gRPC cap (503 ResourceExhausted, permanently — the retry loop then correctly gave up at its deadline). Count-capped batching is blind to heterogeneous batch sizes. Batch by ENCODED BYTES (~3 MiB flush, 1 MiB headroom under the cap; the per-ResourceLogs estimate slightly undercounts envelope framing) with a 500-line secondary cap. Flush BEFORE appending an overflowing line so no push can exceed the cap. Adapting the pusher to Loki's stock limit is the anti-strawman direction — a real OTLP exporter batches under size limits too. Also in run #2, positive findings: the generalized picker chose kafka/WARN(>=13)/1 row on the real corpus, and the OURIOS HALF COMPLETED AT 4.9M-RECORD SCALE (store build + in-process query returned exactly the expected row). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(bench): backtick ResourceExhausted (clippy doc_markdown) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(bench): fail fast on an oversized OTLP payload (Copilot review) An at/over-4-MiB payload 503s permanently; burning the 120 s retry deadline on it masks the cause. push_otlp now asserts the actual encoded size up front, converting the batcher's byte estimate into a checked guarantee. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(bench): accurate estimate comment + progress-log wording Copilot review: the estimate comment claimed undercounting (the +8 padding actually tracks framing closely — and push_otlp's assert is the real guarantee); the progress log printed lines-read as 'batches pushed'. Both now say what they mean, and the final request count increments in the tail flush instead of a blind +1. 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 11, 2026
…y) (#477) * fix(bench): 1.5 MB flush cap — Loki inflates OTLP internally (run #3) Run #3 (29164195463) failed at the same corpus region as run #2 with the IDENTICAL internal size (5,276,869 bytes) — but this time our fail-fast did NOT fire, proving our HTTP payload was under 4 MiB while Loki's internal gRPC message exceeded it. Loki's OTLP→logproto translation INFLATES content ≥1.76x here: OTLP shares resource/scope attributes per batch; the internal push repeats labels and structured metadata per entry. Drop FLUSH_BYTES to 1.5 MB (≥2.6x inflation headroom under the stock 4 MiB cap). push_otlp keeps asserting our own encoded size as a floor guarantee — if a single mega-record alone ever exceeds the cap, the fail-fast will name it precisely. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(bench): sync pusher docs to the 1.5 MB cap (Copilot review) The fn doc referenced the old ≈3 MiB figure and the constant's doc didn't flag 3 MiB as the PREVIOUS value; both now point at FLUSH_BYTES and date the run #3 measurement to the prior cap. 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 11, 2026
…flation) (#478) * fix(bench): raise Loki's internal gRPC cap — single-line inflation (run #4) Run #4 (29165198664) failed on the SAME ~5.27 MB internal message as runs #2/#3 despite the outer cap halving (3 MiB → 1.5 MB), and the fail-fast stayed silent — decisive: a single kafka LogsData line's content alone inflates past Loki's stock 4 MiB internal gRPC cap. No outer batching can split an indivisible unit. Add -server.grpc-server-max-recv/send-msg-size=16 MiB to the indicative run's documented ingest-side flags (standard operator tuning, in Loki's favour — it lets Loki accept the data at all). This preserves the identical-ingest precondition the equivalence check requires; skipping the line would silently unequalize the two corpora. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(bench): correct dskit flag names for the gRPC msg-size cap Copilot caught that the flags are -server.grpc-max-recv/send-msg-size- bytes (dskit's server registry, defaults exactly the 4 MiB we hit), not -server.grpc-server-max-*. The wrong names would have failed Loki's startup and burned run #5. Verified against dskit source. Also backtick the kafka service name in the comment. 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
…riants 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>
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>
This was referenced Jul 22, 2026
jensholdgaard
added a commit
that referenced
this pull request
Jul 22, 2026
Maintainer direction: structured bodies are never truncated; the hazard-#2 guard is a structured_body_bytes metric + per-service alert, not a size cap. Drop the opt-in structured_body_byte_limit (former Option B) to a rejected alternative. Resolve the dependent open questions with stated defaults (event_name keys the structured path only; gap 3 in-scope; gen_ai.* promotion stays config, never implicit). §5/§6 updated to the fidelity+observability scenario. Refs #546 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>
jensholdgaard
added a commit
that referenced
this pull request
Jul 22, 2026
Review fixes on the RFC 0037 spec (Copilot + CodeRabbit): - §5 reformatted to the repo's canonical scenario format (blockquoted `Scenario <id> — <title>` + Given/When/Then/And bullets, greppable ids). - §3.2 / hazards H2: corrected the no-cap rationale — the real reason hazard #2 doesn't apply to structured bodies is that the writer disables dictionary encoding on the `body` column by design (writer.rs §3.6), not that bodies are "unique per record". hazards H2 reworded to read as RFC 0037's proposed (not-yet-implemented) mitigation, and structured_body_bytes as a meter. - Dropped the inconsistent "(gap N)" heading labels on §3.1–§3.3. - Removed a leaked `[[...]]` memory-link placeholder from §3.2. Refs #546 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>
jensholdgaard
added a commit
that referenced
this pull request
Jul 22, 2026
…598) * docs(rfc): specify RFC 0037 — GenAI / structured-event log handling OpenTelemetry models LLM/agent activity as log events (event_name + structured AnyValue body such as gen_ai.input.messages). Ourios already ingests, stores, and reconstructs these correctly (body_kind=Structured, event_name column, RFC 0025 absent-body); this RFC scopes three deltas on top: fold event_name into the structured-template key (§3.1); guard structured-body size by observation not truncation — Option A, full fidelity + structured_body_bytes metric (§3.2); and extend `count … by` to group on a promoted attribute column (§3.3). §3.5 records why a true GenAI vertical slice is rejected (promotion already yields the typed columns) and the SIG-grounded recommended promotion set. §5 acceptance criteria frozen; docs/hazards.md H2 amended for structured bodies. Corpus: synthetic fixture now, Claude Code's own OTLP export as a real available-now AI-agent source, otel-demo v9 for gen_ai.* validation. Status specified — implementation follows in slices. Refs #546. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org> * docs(rfc): address review — scenario format, dict rationale, gap labels Review fixes on the RFC 0037 spec (Copilot + CodeRabbit): - §5 reformatted to the repo's canonical scenario format (blockquoted `Scenario <id> — <title>` + Given/When/Then/And bullets, greppable ids). - §3.2 / hazards H2: corrected the no-cap rationale — the real reason hazard #2 doesn't apply to structured bodies is that the writer disables dictionary encoding on the `body` column by design (writer.rs §3.6), not that bodies are "unique per record". hazards H2 reworded to read as RFC 0037's proposed (not-yet-implemented) mitigation, and structured_body_bytes as a meter. - Dropped the inconsistent "(gap N)" heading labels on §3.1–§3.3. - Removed a leaked `[[...]]` memory-link placeholder from §3.2. Refs #546 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org> --------- Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
jensholdgaard
added a commit
that referenced
this pull request
Jul 23, 2026
…3.2) (#600) * feat(miner): observe structured-body size via a histogram (RFC 0037 §3.2) Structured (non-string) log bodies — GenAI/agent event bodies — are retained whole and never capped (truncating would violate §3.3, and the body column has dictionary encoding disabled by design so hazard #2's failure mode does not apply). The guard against oversized payloads is therefore observation, not truncation: a new ourios.miner.structured_body.size histogram (unit By, dimensioned by ourios.service) records the canonical-JSON byte length of every structured body, so an operator can spot a service shipping large payloads and fix it at the emitter. The name is authored in the weaver registry (semconv/registry/metrics.yaml, copying the ourios.miner.confidence group shape) and regenerated into ourios-semconv — never hand-written, so the CI no-diff and live-check gates hold. The instrument reuses the shared service_attrs helper and mirrors record_overflow rather than duplicating the meter plumbing. Acceptance: RFC0037.3 (rfc0037_structured_body.rs) asserts both unbounded fidelity (a large structured body is retained byte-for-byte, lossy_flag=false) and the metric emission (histogram sum == canonical-JSON length, carrying ourios.service). Refs #546 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org> * test(miner): colocated structured-body fidelity unit test + byte-identical assert Review fixes on slice B (CodeRabbit): - Add rfc0037_3_structured_body_retained_byte_for_byte, a unit test colocated with ingest_structured (§6.2 mandate): the structured branch retains the body's canonical JSON byte-for-byte, body_kind = Structured, non-lossy. The integration test keeps the metric-emission coverage, which needs the SDK meter provider. - Strengthen the integration test to assert the full canonical-JSON string (byte-for-byte), not just its length, so a same-length corruption fails. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org> * test(miner): assert the full required attribute set on the body-size metric Review fix (Copilot): the histogram data-point matcher only checked ourios.service; the registry marks ourios.tenant `required`, so the test now asserts both ourios.tenant and ourios.service ride the data point. Dropping tenant would previously have passed silently. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org> --------- Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Applies the amendments proposed at the bottom of
docs/verification.md(landed in #1). Stacked ondocs/verification-spec; base auto-flips tomainonce #1 merges.docs/rfcs/README.mdstatus:value list expanded todrafted | specified | red | green | validated | accepted | rejected | superseded.supersededandrejectedas terminals reachable from any stage.CLAUDE.mddocs/verification.mdand the rule "if a criterion cannot be turned into a test, the RFC has a gap."docs/rfcs/0001-template-miner.md,docs/rfcs/0002-query-dsl.mdstatus: draft→status: drafted, applying the renamed maturity stage. No body changes.Invariants and hazards touched
The amendments modify the process contract for handling §3 invariants and H-x hazards (RFCs now require §5 Acceptance criteria mapping each touched invariant/hazard to a numbered scenario). The §3 invariants themselves are unchanged. No hazard mitigation is weakened.
RFC 0001's body does not yet include §5 Acceptance criteria — its scenarios for
CLAUDE.md§3.1 +hazards.mdH1 land in a follow-up PR; the worked example indocs/verification.md§6 is the target shape.Test plan
mdbook build— clean exitdocs/verification.md,docs/rfcs/README.md, andCLAUDE.md§5.6 round-trip correctlydraft→draftedrename does not break any external references (none found in repo)Notes
CLAUDE.md§8.5 warns that editingCLAUDE.mdinvalidates the prefix cache for the rest of the editing session. This was unavoidable for §5.6.🤖 Generated with Claude Code