feat(parquet): typed promotion classes — writer side (RFC 0042 slice 1) - #649
Merged
Conversation
PromotedClass {String, I64, F64} with a per-key declaration on
PromotedAttributes (new_typed; the bare-string constructor is unchanged
for existing callers, and the implicit service.name stays string-class
and cannot be re-typed). fields() emits OPTIONAL Utf8/Int64/Float64 by
class; the batch projection dispatches per class — f64 widens ints,
i64 never narrows doubles, strings never parse into numeric columns
(RFC 0042 §3.1). The writer blooms i64 and string promoted columns but
not f64 (§3.5: float equality is typed-arm-only, a bloom would be dead
weight; ordering prunes via statistics).
RFC0042.1: typed columns materialize through the encode path with the
captured Claude Code shape (double cost_usd, int tokens), the JSON
attributes column byte-identical to an unpromoted run, and the bloom
split asserted from the footer. RFC0042.2: projection totality by
proptest over AnyValue variants (incl. absent forms) — the cell is the
§3.1-table value or NULL, never a parse.
RFC 0042 flips drafted -> specified with implementation underway.
Verified: cargo fmt --check, clippy --all-targets --all-features clean,
nextest 1269/1269.
Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>
📝 WalkthroughWalkthroughRFC 0042 typed promotion adds class-aware promoted keys, nullable Arrow fields, typed record-batch projections, class-specific bloom-filter handling, and server serialization updates. New unit, integration, property, and NaN tests validate the behavior. ChangesTyped promotion model
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PromotedAttributes
participant record_batch
participant writer
participant QuerySchema
PromotedAttributes->>record_batch: provide typed promoted keys
record_batch->>writer: emit nullable typed Arrow columns
writer->>writer: configure clustering and bloom filters
PromotedAttributes->>QuerySchema: provide promoted key metadata
QuerySchema-->>QuerySchema: serialize key strings
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
Implements the writer-side half of RFC 0042 typed numeric promotion for promoted attributes in ourios-parquet, extending RFC 0022 promotion from string-only to class-typed (String, I64, F64) projected columns while keeping the JSON attributes column as the source of truth.
Changes:
- Introduces
PromotedClass/PromotedKeyand updatesPromotedAttributesto carry typed promoted keys, emittingOPTIONALUtf8/Int64/Float64columns accordingly. - Updates record-batch projection and Parquet writer properties to dispatch by class and to skip bloom filters for
f64promoted columns (per RFC 0042 §3.5). - Adds RFC 0042 writer-side integration + property tests and updates MCP/query-schema consumers to continue publishing plain key strings.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| docs/rfcs/0042-typed-numeric-promotion.md | Advances RFC 0042 status to specified. |
| crates/ourios-server/tests/it/rfc0032_query_schema.rs | Adjusts schema test to compare promoted key strings after PromotedKey introduction. |
| crates/ourios-server/src/mcp.rs | Keeps MCP query-schema output as arrays of plain key strings despite internal typed keys. |
| crates/ourios-parquet/tests/it/rfc0042_typed_promotion.rs | Adds RFC0042.1 integration test and RFC0042.2 proptest for typed projection behavior. |
| crates/ourios-parquet/tests/it/main.rs | Registers the new RFC 0042 integration test module. |
| crates/ourios-parquet/src/writer.rs | Updates cluster key check for service.name and skips bloom filters for Float64 promoted columns. |
| crates/ourios-parquet/src/record_batch.rs | Implements class-dispatched promoted column builders and typed projection in batch materialization. |
| crates/ourios-parquet/src/promoted.rs | Adds PromotedClass/PromotedKey, typed fields(), and i64_value/f64_value projections. |
| crates/ourios-parquet/src/lib.rs | Re-exports PromotedClass and PromotedKey. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
any::<f64>() draws NaN among its special values, and Some(NaN) != Some(NaN) under IEEE equality would falsely fail the totality property. Compare bits instead — the f64 projection is pass-through and the int widening deterministic, so exact bit equality is the contract, stricter than the ==-based assertion it replaces. Adds a deterministic NaN case alongside (per-fix test obligation): NaN passes through the f64 projection as a value, not an absence, and projects NULL under i64 — not left to the generator's draw. Signed-off-by: Jens Holdgaard Pedersen <jens@holdgaard.org>
jensholdgaard
force-pushed
the
feat/rfc0042-parquet-typed-promotion
branch
from
July 26, 2026 19:58
729d2f8 to
94fbb24
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/ourios-parquet/tests/it/rfc0042_typed_promotion.rs (1)
223-235: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the composite
AnyValuecases in the totality generator.
arb_any_value()says it covers everyAnyValueshape, but it only generates absent/primitive values. Since the OTLP value set includesArrayValueandKvlistValue, add shallow generator arms for them so RFC0042.2 actually exercises promoted projections on structured attribute values.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/ourios-parquet/tests/it/rfc0042_typed_promotion.rs` around lines 223 - 235, Extend arb_any_value() with shallow proptest arms for AnyValue::ArrayValue and AnyValue::KvlistValue, while retaining the existing absent and primitive variants. Generate valid structured values without recursive or unbounded nesting so the totality generator covers both composite shapes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@crates/ourios-parquet/tests/it/rfc0042_typed_promotion.rs`:
- Around line 223-235: Extend arb_any_value() with shallow proptest arms for
AnyValue::ArrayValue and AnyValue::KvlistValue, while retaining the existing
absent and primitive variants. Generate valid structured values without
recursive or unbounded nesting so the totality generator covers both composite
shapes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 95c46169-d7cf-41ab-8e34-5c9251071506
📒 Files selected for processing (9)
crates/ourios-parquet/src/lib.rscrates/ourios-parquet/src/promoted.rscrates/ourios-parquet/src/record_batch.rscrates/ourios-parquet/src/writer.rscrates/ourios-parquet/tests/it/main.rscrates/ourios-parquet/tests/it/rfc0042_typed_promotion.rscrates/ourios-server/src/mcp.rscrates/ourios-server/tests/it/rfc0032_query_schema.rsdocs/rfcs/0042-typed-numeric-promotion.md
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
crates/ourios-parquet/src/promoted.rs:33
PromotedClassis a new public enum and looks likely to grow additional promotion classes over time (e.g., bool/u64/etc). Without#[non_exhaustive], adding a new variant later is a semver-breaking change for downstream exhaustivematches. Mark it#[non_exhaustive]now while the API is new to keep future additive variants non-breaking.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)]
pub enum PromotedClass {
Comment on lines
+1093
to
1099
| for field in promoted.fields() { | ||
| if *field.data_type() == arrow_schema::DataType::Float64 { | ||
| continue; | ||
| } | ||
| let path = ColumnPath::new(vec![field.name().clone()]); | ||
| builder = builder.set_column_bloom_filter_enabled(path, true); | ||
| } |
jensholdgaard
added a commit
that referenced
this pull request
Jul 26, 2026
…lasses The FinOps loop's headline query is sum(attr.cost_usd) by attr.model, but #608's promotion set carries no cost or usage key, so it errors with the promotion hint. This PR's first draft promoted the keys bare — and review caught that RFC 0022 §3.1's string-only projection would have made always-NULL columns and a silently EMPTY sum (worse than the loud error). That finding became RFC 0042; with its typed classes now green in-repo (#649-#653), the promotion lands the way it must: classes matching the AnyValue variants Claude Code actually emits, verified against captured api_request events — - { key: cost_usd, type: f64 } # doubleValue - { key: input_tokens, type: i64 } # intValue - { key: output_tokens, type: i64 } # intValue cost_usd_micros is skipped as redundant under f64 aggregation; cache token counts are additive later if a query wants them. Cost stays the source's responsibility (Claude Code computes and stamps it); Ourios stores, attributes, and aggregates. Pricing tables and FOCUS-shaped output belong to consumers of the query surface. Verified: the server boots the config through the RFC0042.6 validation path (fails only on the port the running dogfood server holds). Promotion is a write-side projection — RFC0042.9's live sum lands with the first capture after the dogfood server restarts on this config. Signed-off-by: Jens Holdgaard Pedersen <jens@holdgaard.org>
jensholdgaard
added a commit
that referenced
this pull request
Jul 26, 2026
…ice 4) (#653) RFC0042.8: two input files — the key promoted under the string class in one epoch, unpromoted in the other — consolidate under the current i64 declaration into an Int64 column projected from JSON truth (both epochs' values present), and the rewrite is deterministic: byte-identical across two identical runs, the RFC0036.4 property under a fixed config. The RFC banner records where the ladder stands: .1-.8 green in-repo across #649-#652 + this test; RFC0042.9 (the dogfood corpus gate) outstanding until the first agent capture under the typed set, so the status stays specified with the implementation ahead of the flip. Verified: cargo fmt --check, clippy clean, nextest 165/165 (parquet), mdbook build. Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>
jensholdgaard
added a commit
that referenced
this pull request
Jul 26, 2026
) * feat(dogfood): promote cost_usd + token counts under RFC 0042 typed classes The FinOps loop's headline query is sum(attr.cost_usd) by attr.model, but #608's promotion set carries no cost or usage key, so it errors with the promotion hint. This PR's first draft promoted the keys bare — and review caught that RFC 0022 §3.1's string-only projection would have made always-NULL columns and a silently EMPTY sum (worse than the loud error). That finding became RFC 0042; with its typed classes now green in-repo (#649-#653), the promotion lands the way it must: classes matching the AnyValue variants Claude Code actually emits, verified against captured api_request events — - { key: cost_usd, type: f64 } # doubleValue - { key: input_tokens, type: i64 } # intValue - { key: output_tokens, type: i64 } # intValue cost_usd_micros is skipped as redundant under f64 aggregation; cache token counts are additive later if a query wants them. Cost stays the source's responsibility (Claude Code computes and stamps it); Ourios stores, attributes, and aggregates. Pricing tables and FOCUS-shaped output belong to consumers of the query surface. Verified: the server boots the config through the RFC0042.6 validation path (fails only on the port the running dogfood server holds). Promotion is a write-side projection — RFC0042.9's live sum lands with the first capture after the dogfood server restarts on this config. Signed-off-by: Jens Holdgaard Pedersen <jens@holdgaard.org> * docs(dogfood): a wrong-class promotion yields a NULL aggregate, not an empty result The comment claimed sum over an always-NULL column silently returns empty; the RFC0002.18 semantics is a NULL aggregate (every input excluded, value null). Same silence, precise shape. Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org> --------- Signed-off-by: Jens Holdgaard Pedersen <jens@holdgaard.org> Signed-off-by: Jens Holdgaard Pedersen <Jens@holdgaard.org>
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.
What (RFC 0042 slice 1 of ~4)
The writer half of typed numeric promotion, enacting RFC 0022 §7.1 via RFC 0042 (
drafted→specifiedin this PR):PromotedClass { String, I64, F64 }with per-key declarations onPromotedAttributes::new_typed. The bare-string constructor is byte-for-byte unchanged for existing callers; the implicitservice.namestays string-class and cannot be re-typed (pinned by test).fields()emitsOPTIONALUtf8/Int64/Float64; the batch projection dispatches per class through a three-variant builder.f64widens ints (cost_usd: 0as int still lands),i64never narrows doubles, strings never parse into numeric columns.i64+ string promoted columns keep the RFC 0022 bloom;f64columns get none — float equality is typed-arm-only, so a bloom is dead weight; ordering prunes via statistics. Asserted from the Parquet footer.Criteria
cost_usd, int tokens) plus the widening/narrowing/string rows; the JSONattributescolumn is byte-identical to an unpromoted run ("projection, not truth" re-asserted for typed classes).AnyValuevariants × classes (including value-absent forms): every cell is the §3.1-table value orNULL, never a panic or parse.Cross-crate ripples
resource_keys()/log_keys()now return&[PromotedKey]; the three consumers (writer cluster-keys check, MCP query-schema resource, its test) updated mechanically. The RFC 0032 resource keeps publishing plain key strings — whether to expose classes there is a slice-2/RFC 0032 question, not smuggled in here.Not in this slice
Config parsing (
{key, type}entries — slice 2), the querier schema adapter + typed predicates/aggregation (slice 3), compaction re-projection test + dogfood (slice 4). #646 stays drafted until the ladder supports it.Invariants
Touches hazard #5 territory on paper only: columns remain additive
OPTIONAL(RFC 0042 §3.6, same evolution class as RFC 0022). Reconstruction/OTLP fidelity untouched — promoted cells are query-only projections and RFC0042.1 proves the JSON truth column identical.Verification
cargo fmt --check·clippy --all-targets --all-featuresclean (pedantic) ·cargo nextest run1269/1269.Summary by CodeRabbit
New Features
Bug Fixes
Documentation