feat(miner): rfc 0023 green pt2 — parse-failure reason telemetry (RFC0023.6)

[jensholdgaard]

Discharges RFC0023.6: a ceiling-saturated tenant is diagnosable from telemetry alone.

• ourios.miner.parse_failure.reason — a weaver-registry enum attribute (below_floor | empty_line | tokenizer_failure | degenerate_widening | line_too_long | template_ceiling) on the existing ourios.miner.parse_failures counter, per the OTel error.type convention (one instrument + a cause dimension, never per-cause counters). Registry checked with weaver registry check; constants regenerated via weaver registry generate (semconv CI's no-diff gate covers drift).
• Every parse-failure exit threads its reason through the shared emit_string_parse_failure helper — the six causes now cover the entire §6.3 exit surface, not just the RFC 0023 diverts, so the counter's cause split is complete rather than partial.
• attach_and_maybe_widen went over the clippy line budget from the reason arg; the §6.4 rejection's audit emission moved to emit_rejected_degenerate_audit (a 1:1 payload mirror, too_many_arguments allowed with the reason inline).
• RFC0023.6 test on the in-memory meter harness (the rfc_internal.rs pattern): asserts reason = template_ceiling increments equal the diverted-line count and template.count reads the ceiling.

With this, RFC 0023 is six-of-seven green; only RFC0023.7 (the 16 GiB HDFS_v2 rerun under 8 GiB peak RSS on the hardware baseline) remains, which is a bench run rather than a code change.

Verification: cargo fmt --all --check, weaver registry check, cargo clippy --all-targets --all-features -- -D warnings, miner/semconv/ingester suites — all green locally.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Parse-failure metrics now include a specific reason, making miner telemetry easier to understand and filter.

• Bug Fixes

  • Improved reporting for common failure cases such as empty input, overly long lines, tokenizer issues, template saturation, and degenerate widening.
  • Added coverage to verify metric counts and ceiling behavior under limited template capacity.

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 54 minutes

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

How can I continue?

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

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

How do review limits work?

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

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

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: f2abdf60-4430-480f-86da-d18aef4a2648

📥 Commits

Reviewing files that changed from the base of the PR and between b37e00a and b5a6c88.

📒 Files selected for processing (1)

• crates/ourios-semconv/src/lib.rs

📝 Walkthrough

Walkthrough

This PR adds a reason: &'static str parameter to parse-failure recording in the miner cluster and metrics code, tagging failures by cause (e.g. empty_line, line_too_long, template_ceiling, below_floor, tokenizer_failure, degenerate_widening). A new semantic-convention attribute and metric registration are added, and a previously-ignored test is implemented to verify reason-tagged metrics.

Changes

Parse-failure reason tagging

Layer / File(s)	Summary
Semantic convention attribute and metric definition semconv/registry/attributes.yaml, semconv/registry/metrics.yaml, crates/ourios-semconv/src/lib.rs	Adds ourios.miner.parse_failure.reason enum attribute, registers it as required on metric.ourios.miner.parse_failures, and exports the generated OURIOS_MINER_PARSE_FAILURE_REASON constant (plus unrelated formatting/whitespace tweaks).
Metrics recording with reason attribute crates/ourios-miner/src/metrics.rs	record_parse_failure gains a reason parameter and attaches it, along with tenant and optional service, as metric attributes.
Cluster call sites pass explicit failure reasons crates/ourios-miner/src/cluster.rs	record_parse_failure, record_tokenizer_failure, and emit_string_parse_failure propagate explicit reasons (empty_line, line_too_long, template_ceiling, below_floor, tokenizer_failure, degenerate_widening); degenerate-widening audit emission is extracted into a new emit_rejected_degenerate_audit helper.
Bounded-memory test validates reason-tagged metrics crates/ourios-miner/tests/rfc0023_bounded_memory.rs	Implements the previously ignored rfc0023_6_ceiling_saturation_is_observable test using an in-memory telemetry exporter to assert diverted parse-failure counts by reason and template-count gauge values.

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

Possibly related PRs

• jensholdgaard/ourios#160: Established the original record_parse_failure instrumentation in cluster.rs/metrics.rs that this PR extends with the new reason dimension.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description covers the change, but it omits the required Summary/Related/Checklist template sections and checklist items.	Add the template headings, a Related section with the RFC/issue link, and a Checklist with fmt, clippy, tests, docs, and RFC status.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise and accurately identifies the miner parse-failure reason telemetry work.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch rfc0023-green-pt2-telemetry

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[Copilot]

Pull request overview

Adds a first-class “reason” dimension to miner parse-failure telemetry to make RFC0023.6 (ceiling saturation diagnosable via metrics) observable end-to-end, and updates miner control-flow to thread the reason through all parse-failure exits.

Changes:

• Introduces ourios.miner.parse_failure.reason as a semconv enum attribute and requires it on ourios.miner.parse_failures.
• Threads a reason through miner parse-failure recording paths (including ceiling divert, below-floor, long line, empty line, degenerate widening, tokenizer failure).
• Implements RFC0023.6 test asserting reason=template_ceiling increments and template.count reads the ceiling.

Reviewed changes

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

Show a summary per file

File	Description
semconv/registry/metrics.yaml	Requires the new reason attribute on the ourios.miner.parse_failures metric.
semconv/registry/attributes.yaml	Defines the ourios.miner.parse_failure.reason enum and its members.
crates/ourios-semconv/src/lib.rs	Adds the generated Rust constant for the new attribute key.
crates/ourios-miner/src/metrics.rs	Adds a reason attribute to parse-failure counter recording.
crates/ourios-miner/src/cluster.rs	Threads parse-failure reasons through all relevant miner exit paths and factors degenerate audit emission into a helper.
crates/ourios-miner/tests/rfc0023_bounded_memory.rs	Replaces the RFC0023.6 stub with an in-memory-metrics assertion test.

---

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

[Copilot]

Pull request overview

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

[coderabbitai]

🧹 Nitpick comments (2)

crates/ourios-miner/src/cluster.rs (1)

1288-1288: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Consider centralizing reason strings to guard against registry drift.

"template_ceiling" (and the other five reason literals across this file) are hand-typed &'static str values that must exactly match the weaver-generated ourios.miner.parse_failure.reason enum members. Nothing at compile time ties these literals back to the registry — a typo in one call site (or a future rename of the registry enum member) would silently emit mismatched telemetry rather than fail a build. The stack outline for the semconv layer mentions adding only "the generated constant" (singular, the attribute key) to ourios-semconv, suggesting the enum values themselves aren't generated as Rust constants.

Consider defining a small local enum (e.g. ParseFailureReason with an as_str() returning the six &'static str values) or a set of pub(crate) const strings in one place, so every call site (cluster.rs lines 1238, 1256, 1288, 1354, 1391, 1753, plus record_tokenizer_failure's "tokenizer_failure") references a single source of truth instead of retyping the literal.

Also applies to: 1354-1354

🤖 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-miner/src/cluster.rs` at line 1288, Centralize the
parse-failure reason literals used in cluster.rs so they cannot drift from the
weaver registry. Introduce a single source of truth, such as a local
ParseFailureReason enum with an as_str() method or a set of pub(crate) const
strings, and update every call site including the code around
record_tokenizer_failure and the parse-failure helpers to use those shared
symbols instead of hand-typed &'static str values like "template_ceiling".

crates/ourios-miner/tests/rfc0023_bounded_memory.rs (1)

267-340: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add metrics assertions for the other parse-failure reasons

template_ceiling is the only ourios.miner.parse_failures reason covered end-to-end here. Add at least one metrics-level assertion for empty_line, line_too_long, tokenizer_failure, below_floor, and degenerate_widening (a parameterized case would work well) so each emitted reason value is exercised in crate tests.

🤖 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-miner/tests/rfc0023_bounded_memory.rs` around lines 267 - 340,
The test rfc0023_6_ceiling_saturation_is_observable only asserts the
template_ceiling parse-failure reason, so expand it or add nearby test cases to
exercise the remaining ourios.miner.parse_failures reasons. Add metrics-level
assertions for empty_line, line_too_long, tokenizer_failure, below_floor, and
degenerate_widening, ideally using a parameterized helper that ingests records
triggering each path and verifies the exported reason attribute. Use the
existing helpers like init_in_memory, MinerCluster::new, record, and the
reason/tenant attribute checks to keep the assertions aligned with the current
metrics style.

Source: Coding guidelines

🤖 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-miner/src/cluster.rs`:
- Line 1288: Centralize the parse-failure reason literals used in cluster.rs so
they cannot drift from the weaver registry. Introduce a single source of truth,
such as a local ParseFailureReason enum with an as_str() method or a set of
pub(crate) const strings, and update every call site including the code around
record_tokenizer_failure and the parse-failure helpers to use those shared
symbols instead of hand-typed &'static str values like "template_ceiling".

In `@crates/ourios-miner/tests/rfc0023_bounded_memory.rs`:
- Around line 267-340: The test rfc0023_6_ceiling_saturation_is_observable only
asserts the template_ceiling parse-failure reason, so expand it or add nearby
test cases to exercise the remaining ourios.miner.parse_failures reasons. Add
metrics-level assertions for empty_line, line_too_long, tokenizer_failure,
below_floor, and degenerate_widening, ideally using a parameterized helper that
ingests records triggering each path and verifies the exported reason attribute.
Use the existing helpers like init_in_memory, MinerCluster::new, record, and the
reason/tenant attribute checks to keep the assertions aligned with the current
metrics style.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: de2489aa-adfc-4e0d-9a6a-27af1d15e826

📥 Commits

Reviewing files that changed from the base of the PR and between d171457 and b37e00a.

📒 Files selected for processing (6)

• crates/ourios-miner/src/cluster.rs
• crates/ourios-miner/src/metrics.rs
• crates/ourios-miner/tests/rfc0023_bounded_memory.rs
• crates/ourios-semconv/src/lib.rs
• semconv/registry/attributes.yaml
• semconv/registry/metrics.yaml

[jensholdgaard]

Merging with Copilot's re-review of the final head outstanding, with rationale: CI is fully green on b5a6c88; zero unresolved threads; Copilot reviewed all substantive content (through a0894f7, one comment) and the two subsequent commits are exactly its requested one-word brief fix (b37e00a) plus a rustfmt-only wrap (b5a6c88). Three re-review requests over ~2 h went unanswered — the review-coverage intent of the clean-gate rule is satisfied in substance, documented here rather than waived silently.