Skip to content

feat(querier): RFC 0033 v2 — zstd artifact at the v2 key, publish-outcome labels - #522

Merged
jensholdgaard merged 5 commits into
mainfrom
rfc0033-v2-compressed-artifact
Jul 14, 2026
Merged

feat(querier): RFC 0033 v2 — zstd artifact at the v2 key, publish-outcome labels#522
jensholdgaard merged 5 commits into
mainfrom
rfc0033-v2-compressed-artifact

Conversation

@jensholdgaard

@jensholdgaard jensholdgaard commented Jul 14, 2026

Copy link
Copy Markdown
Owner

What

RFC 0033 v2 implementation — the zstd-compressed artifact at template_map.v2.json.zst, per the merged §3.2 amendment (#521). This is the run-#20 answer.

  • format_version: 2 in the decompressed body; version probe stays as defense-in-depth, running on the decompressed bytes: a v1 JSON body shipped inside v2 transport → UnknownVersion; anything that is not a zstd frame — including a plain uncompressed v1 JSON planted at the v2 key — → Torn (the §3.3 "not a frame" arm); both fold, never an error.
  • Publish targets the v2 key (tmp+rename / CAS unchanged), then best-effort deletes the stale v1 key (§3.4 amendment; unit-tested on both backends).
  • Abstention and the §3.7 artifact.size histogram operate on compressed bytes — the GET cost they always meant.
  • zstd = "0.13" on ourios-querier: zero new transitive crates (parquet already pulls it; ourios-bench binds it as the A1 reference codec).
  • Harness: TemplateMapProbe classifies each pair Warm / ColdPublished / ColdAbsent / ColdUnreadable at measurement time; on any absence, one post-measurement reproduce_publish_decision() (derive + to_artifact_bytes vs folded audit bytes) prints published — N B compressed, abstained — would-be N B ≥ folded M B, or publish error — would-be N B < folded M B yet nothing on the store. Run docs(architecture): OTLP log-format gap analysis (Half 1 investigation) #20's abstention-vs-failure ambiguity cannot recur. Reproduction runs outside the timed path.
  • Contract pins updated to the amendment (path pins follow the v2 key; .1/.3 round-trip/tear the compressed body; unknown-version arms cover both directions). No assertion weakened.

Measured (local .6 arm)

cold 518,016 B → warm ~1,188 B compressed (9,288 B JSON → ~7.8× frame compression) = ~436× past the 10× gate (v1 encoding was 55.8×).

✅ Measured — run #21 (29343438434)

The artifact published on otel-demo-v8 and every pair ran warm: warm (one artifact GET, 187,904 B compressed) vs the 513,862 B cold fold — warm/cold ≈ 1/2.73, a ~326 KB cut off every body-rendering query's honest total. The run #20 abstention-vs-failure ambiguity is resolved by the new outcome labels. Run #21's exit-1 was the Loki-side L3 low-volume-chunk race (0/9 rows before timeout), not an Ourios finding; all RFC 0033 numbers printed before the panic (record: §9.15). The original 1/10 corpus ratio gate amends to ≤ 1/2 per the maintainer-approved option 1 (the artifact is O(state), the fold O(history) — the ratio ages upward); the amendment PR carries the argument.

Post-dispatch commits on this branch (8c993aa expect-message wording, 1dbdfb8 zstd-bomb bound) don't touch measured byte counts — the run remains representative of the merge candidate.

⚠️ HOLD — measure before merge (discharged)

Do not merge on the clean gate. Comparative dispatch run #21 from this branch is the §6-amendment validation: it must show the corpus publish outcome explicitly (published + size, or abstained + would-be size vs 513,862 B fold). Numbers land here before merge.

Checks run

cargo fmt --all --check; workspace cargo clippy --all-targets --all-features -- -D warnings; strict rustdoc on ourios-querier + ourios-bench; cargo nextest run -p ourios-querier -p ourios-bench333 passed (11 skipped, the usual ignored S3/corpus arms). Rebased on the merged amendment; includes the yanked-spin lock bump (also on #519).

🤖 Generated with Claude Code

https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y

jensholdgaard and others added 2 commits July 14, 2026 16:57
…come labels

Implements the RFC 0033 §3.2 compressed-artifact amendment
(2026-07-13, run #20 / §9.14): the template-map artifact ships as one
zstd frame (crate-default level 3, an implementation constant) at the
new key `template_map.v2.json.zst`, with `format_version: 2` in the
decompressed body as defense-in-depth behind the version-in-key rule.

- Readers GET only the v2 key (`TEMPLATE_MAP_FILENAME` now names it);
  not-a-frame / failed decompression / post-decompression parse
  failures classify Torn, a decompressed `format_version` != 2
  classifies UnknownVersion — both treated as absent per §3.3.
- Publish stays atomic (tmp+rename local / CAS remote) at the v2 key
  and then best-effort unconditionally deletes the stale v1 key
  (`TEMPLATE_MAP_V1_FILENAME`), §3.4 amendment — failure swallowed
  like every other best-effort publish IO; covered by a new unit test
  on both backends.
- Abstention now compares the **compressed** byte size against the
  folded audit bytes (§3.2 amendment restated rule), and the §3.7
  `artifact.size` histogram records the compressed (published-object)
  bytes — unchanged code path, the published bytes are now compressed.
- The RFC 0031 comparative harness prints each pair's template-map
  publish outcome explicitly — `published` (compressed size),
  `abstained` (would-be compressed size vs folded audit bytes), or
  error — resolving run #20's abstention-vs-failure ambiguity. The
  abstention decision is reproduced (one fold + serialize + compress
  via the new `TemplateMap::to_artifact_bytes`) once, after every
  pair's timed measurement, off the measured path; `lost_race` cannot
  occur in the single-writer harness.
- `zstd = "0.13"` added to ourios-querier: zero new transitive crates
  (parquet 58's `zstd` feature already pulls it; ourios-bench binds it
  directly as the A1 reference codec).

Contract-pin changes, all mandated by the maintainer-approved §3.2/
§3.3/§3.4/§6 amendment blocks: `TEMPLATE_MAP_FILENAME` pins the v2
key; `TEMPLATE_MAP_FORMAT_VERSION` is 2; the public read/write surface
is `from_artifact_bytes`/`to_artifact_bytes` (the bytes are no longer
JSON, `to_json` is now the private body encoder); the RFC0033 .1/.3
round-trip and torn fixtures exercise the compressed body; the .6
local arm and .7 artifact-size sums now measure compressed bytes. No
test was weakened — every scenario asserts the same behaviour through
the amended encoding.

RFC0033.6 local arm after the amendment: cold=518,016 B (64-file
audit fold), warm=1,188 B compressed (9,288 B decompressed JSON,
~7.8x frame compression) — a ~436x collapse against the 10x gate
(v1 encoding: 55.8x).

Invariants: no Parquet schema change (the artifact is additive, both
keys invisible to every `*.parquet` walk); audit stream remains the
source of truth; tenant scoping and the row-vs-path stance unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y
@jensholdgaard
jensholdgaard requested a review from Copilot July 14, 2026 15:01
@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown

Warning

Review limit reached

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

Next review available in: 28 seconds

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

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

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

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 27243442-19ca-47af-9c70-c96abdcd7eaf

📥 Commits

Reviewing files that changed from the base of the PR and between 544c1f0 and 245ff4a.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (6)
  • crates/ourios-bench/tests/rfc0031_comparative.rs
  • crates/ourios-querier/Cargo.toml
  • crates/ourios-querier/src/lib.rs
  • crates/ourios-querier/src/template_map.rs
  • crates/ourios-querier/src/template_registry.rs
  • crates/ourios-querier/tests/it/rfc0033_cached_template_map.rs
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch rfc0033-v2-compressed-artifact

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the RFC 0033 cached template-map artifact implementation in ourios-querier to use a new v2 key (template_map.v2.json.zst) whose payload is a zstd-compressed JSON body (format_version: 2), and updates tests/bench harnesses to validate and report the amended publish/abstention behavior.

Changes:

  • Switch template-map artifact I/O from plain JSON (template_map.json) to zstd frame bytes at template_map.v2.json.zst, including decompression on read and compression on publish.
  • Add best-effort deletion of the stale v1 key after successful v2 publish (local + remote), with unit coverage.
  • Update comparative bench harness reporting to print publish outcome labels (published/abstained/error) for “no artifact” cases.

Reviewed changes

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

Show a summary per file
File Description
crates/ourios-querier/tests/it/rfc0033_cached_template_map.rs Updates RFC0033 integration tests to round-trip via compressed artifact bytes and adds decompressed-size logging.
crates/ourios-querier/src/template_registry.rs Updates module docs to reference the v2 artifact key.
crates/ourios-querier/src/template_map.rs Implements zstd transport, v2 key constants, v1-key deletion hygiene, and updates read/write logic + unit tests.
crates/ourios-querier/src/lib.rs Updates public exports and docs to reflect the v2 artifact key and v1 filename constant.
crates/ourios-querier/Cargo.toml Adds zstd = "0.13" dependency for artifact compression/decompression.
crates/ourios-bench/tests/rfc0031_comparative.rs Enhances measurement labeling via TemplateMapProbe and post-measurement publish-decision reproduction.
Cargo.lock Adds zstd and updates spin lock entry.
Comments suppressed due to low confidence (1)

crates/ourios-querier/src/template_map.rs:732

  • publish() wraps to_artifact_bytes() errors into a new QueryError::Storage with a serialize ... prefix, which can mislabel compression failures and makes error chains harder to interpret (the original error already includes context like compress template_map.v2.json.zst: ...). Prefer recording the metric and returning the original error unchanged (or use a more accurate prefix like encode).
        let bytes = self.to_artifact_bytes().map_err(|e| {
            METRICS.record_publish(PUBLISH_OUTCOME_ERROR);
            QueryError::Storage {
                detail: format!("serialize {TEMPLATE_MAP_FILENAME}: {e}"),
            }

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

Comment thread crates/ourios-querier/tests/it/rfc0033_cached_template_map.rs Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

Comments suppressed due to low confidence (1)

crates/ourios-querier/src/template_map.rs:732

  • publish() wraps to_artifact_bytes() errors into a new QueryError::Storage { detail: "serialize …" }, which (a) can be misleading now that this step includes compression, and (b) loses the original, already-specific error detail by nesting it inside another string.

Prefer recording the metric and returning the original error unchanged.

        let bytes = self.to_artifact_bytes().map_err(|e| {
            METRICS.record_publish(PUBLISH_OUTCOME_ERROR);
            QueryError::Storage {
                detail: format!("serialize {TEMPLATE_MAP_FILENAME}: {e}"),
            }

Comment thread crates/ourios-querier/src/template_map.rs
… bombs

The artifact is untrusted input: decode via a streaming reader capped
at 64 MiB (oversize classifies Torn), with the same bound enforced on
the publish side so a legitimate artifact can never trip the reader
and churn. Guarded by a bomb test (tiny compressed frame expanding
past the bound).

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.

Comment thread crates/ourios-querier/src/template_map.rs
Comment thread crates/ourios-querier/src/template_map.rs
@jensholdgaard
jensholdgaard merged commit 02d622e into main Jul 14, 2026
27 checks passed
jensholdgaard added a commit that referenced this pull request Jul 14, 2026
…run #21) (#528)

The §7 deferral's named condition is met: RFC 0033's v2 compressed
template map merged (#522) and comparative run #21 measured every pair
warm at 187,904 B acquisition vs the 513,862 B fold (§9.15). M_L2
freezes per channel, derived from the run record:

- processed (primary): M_L2 = 10 — measured 32.5–39.3× across §9.13
  runs #10#17, 37.3–45.1× on the post-artifact total.
- storage-side: a 1.1× floor, integer-exact as ourios × 11 ≤
  loki_storage × 10 (m_l2_storage_floor_tenths = 11). The
  post-artifact honest total (0 + 2,035,267 + 187,904 = 2,223,171 B)
  computes to 1.20–1.51× against the recorded Loki storage band; 1.1
  sits below the weakest point with margin for Loki's documented
  chunk-boundary wobble. A parity-plus floor, not a 10× claim — the
  write-side lever stays recorded, not chased.

Harness: frozen_gate_failures() gates the L2 pair on both channels in
the existing salvage ordering; scenario RFC0031.3 un-stubs green in
the .2/.4 style (boundary math + record-derived evidence, including
the pre-artifact total correctly failing the floor). The RFC 0033
§5.6 corpus acquisition gate (warm ≤ fold/2 when a warm pair exists)
also asserts in the dispatch run — fold from a cold pair when one
exists, one off-timed-path refold in the all-warm steady state,
loudly non-evaluable on refold failure. bytes_must_win_tenths carries
the sub-integer floor with the lgates honesty guards (zero ⇒ Invalid,
overflow fails closed on the correct side), unit-tested at the
boundaries.

M_L4 and F_L7 deferrals are untouched. Merge is gated on a fresh
dispatch (run #23) from this branch proving the new assertions pass.


Claude-Session: https://claude.ai/code/session_01WQY9wfrfRggqSpMLH8Xj3Y

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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.

2 participants