feat(cli): vouch metrics — first-class observability#193
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR introduces ChangesMetrics Observability
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/vouch/metrics.py (2)
556-611: 💤 Low valueMinor: variable
emitted_headersdefined after the closure that references it.Line 579 defines
emitted_headers: set[str] = set()after thegaugehelper function (lines 566-577) that uses it. While this works correctly in Python (variables are looked up when the closure is called, not defined), it's slightly unconventional to define a variable after a function that references it. For clearer code flow, consider moving line 579 to just before line 566.🤖 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 `@src/vouch/metrics.py` around lines 556 - 611, The closure gauge inside render_prometheus references emitted_headers which is declared later; move the emitted_headers: set[str] = set() declaration to just before the gauge function definition so the helper and state are grouped together (keep the same variable name and type annotation) and then leave the rest of render_prometheus unchanged.
300-306: 💤 Low valueConsider simplifying line 306 for readability.
The current logic
return not (until is not None and ts > until)is correct but requires careful parsing. A more direct expression would be:return until is None or ts <= untilBoth are equivalent, but the suggested form makes the intent clearer: "return True if there's no upper bound OR the timestamp is within the upper bound."
🤖 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 `@src/vouch/metrics.py` around lines 300 - 306, The _in_window function's final return is hard to read; replace the expression that checks the upper bound in _in_window (which currently uses `not (until is not None and ts > until)`) with a clearer direct check using the until variable and ts (i.e., return True when there is no upper bound or ts is <= until) so the logic reads as "no upper bound OR timestamp within upper bound" while keeping the earlier null checks for ts and since intact.
🤖 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.
Inline comments:
In `@docs/metrics.md`:
- Around line 137-148: The fenced code block containing the Prometheus
exposition snippet (the block with metrics like vouch_approval_rate,
vouch_citation_coverage, and vouch_claims_by_status) should include a language
identifier; update the opening triple-backtick to use a tag such as "text" or
"prometheus" so the block reads ```text (or ```prometheus) to satisfy the linter
and clarify formatting.
---
Nitpick comments:
In `@src/vouch/metrics.py`:
- Around line 556-611: The closure gauge inside render_prometheus references
emitted_headers which is declared later; move the emitted_headers: set[str] =
set() declaration to just before the gauge function definition so the helper and
state are grouped together (keep the same variable name and type annotation) and
then leave the rest of render_prometheus unchanged.
- Around line 300-306: The _in_window function's final return is hard to read;
replace the expression that checks the upper bound in _in_window (which
currently uses `not (until is not None and ts > until)`) with a clearer direct
check using the until variable and ts (i.e., return True when there is no upper
bound or ts is <= until) so the logic reads as "no upper bound OR timestamp
within upper bound" while keeping the earlier null checks for ts and since
intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 35104da4-f5b1-474e-a7af-3c7a5a4ad8b2
📒 Files selected for processing (4)
docs/metrics.mdsrc/vouch/cli.pysrc/vouch/metrics.pytests/test_metrics.py
| ``` | ||
| # HELP vouch_approval_rate approve / (approve + reject) over window. | ||
| # TYPE vouch_approval_rate gauge | ||
| vouch_approval_rate 0.75 | ||
| # HELP vouch_citation_coverage Fraction of claims fully cited. | ||
| # TYPE vouch_citation_coverage gauge | ||
| vouch_citation_coverage 0.8 | ||
| # HELP vouch_claims_by_status Claim count per status. | ||
| # TYPE vouch_claims_by_status gauge | ||
| vouch_claims_by_status{status="archived"} 1 | ||
| vouch_claims_by_status{status="working"} 4 | ||
| ``` |
There was a problem hiding this comment.
Specify language for fenced code block.
The fenced code block starting at line 137 should specify a language identifier. Since Prometheus exposition format doesn't have standard syntax highlighting, mark it as text or prometheus.
📝 Proposed fix
-```
+```text
# HELP vouch_approval_rate approve / (approve + reject) over window.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ``` | |
| # HELP vouch_approval_rate approve / (approve + reject) over window. | |
| # TYPE vouch_approval_rate gauge | |
| vouch_approval_rate 0.75 | |
| # HELP vouch_citation_coverage Fraction of claims fully cited. | |
| # TYPE vouch_citation_coverage gauge | |
| vouch_citation_coverage 0.8 | |
| # HELP vouch_claims_by_status Claim count per status. | |
| # TYPE vouch_claims_by_status gauge | |
| vouch_claims_by_status{status="archived"} 1 | |
| vouch_claims_by_status{status="working"} 4 | |
| ``` |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 137-137: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 `@docs/metrics.md` around lines 137 - 148, The fenced code block containing the
Prometheus exposition snippet (the block with metrics like vouch_approval_rate,
vouch_citation_coverage, and vouch_claims_by_status) should include a language
identifier; update the opening triple-backtick to use a tag such as "text" or
"prometheus" so the block reads ```text (or ```prometheus) to satisfy the linter
and clarify formatting.
Source: Linters/SAST tools
Add `vouch metrics`, a read-only observability surface for the review gate and corpus, derived purely from .vouch/audit.log.jsonl + the artifact files (no new on-disk state, no schema migration). Emits: - approval_rate (+ per-kind breakdown and decisions_by_kind) - citation_coverage / citation_broken (every evidence ref must resolve) - stale_ratio against an active-claim denominator (retired claims exempt), threshold matching `vouch lint --stale-days` - proposal_lag_seconds p50/p90/p99/mean/max (nearest-rank, matching Prometheus histogram_quantile); a create older than --since still pairs with an in-window approve so the window's left edge doesn't undercount - actors leaderboard (proposed/approved/rejected/confirmed), top-N - claims_by_status histogram, audit event volume Surfaces: - human table (default) - --json: stable, versioned schema (schema_version=1) documented in docs/metrics.md; ratios are null (not 0) when their denominator is empty - --prometheus: textfile-collector exposition with HELP/TYPE headers; null gauges omitted so a 0 never lies about the denominator - --since / --until accept durations (30d/12h/2w) or ISO dates; --stale-days and --top are configurable Closes vouchdev#192. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ce66b0d to
bcf4c98
Compare
|
LGTM! |
2 participants
Summary
vouch metrics, a read-only observability surface for the review gate and corpus..vouch/audit.log.jsonl+ the artifact files — no new on-disk state, no schema migration (per ROADMAP 0.3 observability hooks).What it emits
approval_rate—approve / (approve + reject)over the window, plus a per-ProposalKindbreakdown (approval_rate_by_kind) and rawdecisions_by_kind.citation_coverage/citation_broken— fraction of claims whose everyevidenceid resolves to a live Source/Evidence.stale_ratio— stale active claims over active claims; threshold matchesvouch lint --stale-days(default 180); retired claims (superseded/archived/redacted) are exempt.proposal_lag_seconds—p50/p90/p99/mean/maxfrom each proposal's*.create→*.approve(nearest-rank percentiles, matching Prometheushistogram_quantile). A create older than--sincestill pairs with an in-window approve, so the window's left edge doesn't undercount.actors— top-N leaderboard (proposed/approved/rejected/confirmed).claims_by_statushistogram and audit event volume.Surfaces
--json— stable, versioned schema (schema_version=1) documented indocs/metrics.md. Ratios arenull(not0) when the denominator is empty, so consumers can tell "no data" from a real zero.--prometheus— textfile-collector exposition with# HELP/# TYPEheaders; null gauges are omitted so a0never lies about the denominator.--since/--until— durations (30d/12h/2w) or ISO dates;--stale-daysand--topconfigurable.Design notes
decided/proposal files — the log is append-only and authoritative, and carries the timestamps the lag percentiles need.Test Plan
tests/test_metrics.py(45 tests) — fixture KB with statistics known by construction; asserts every metric, the stable JSON schema, the Prometheus exposition, windowing, and the CLI surface.pytest -q→ 201 passedruff check src/vouch tests→ cleanmypy src/vouch/metrics.py src/vouch/cli.py→ clean--json/--prometheusoutput, bad---sinceerror, mutually-exclusive-flag guard.Closes #192
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
vouch metricscommand that computes observability metrics from audit logs and artifacts. Supports human-readable table, JSON, and Prometheus output formats. Metrics include approval rates, citation coverage, staleness tracking, proposal lag analysis, and actor activity leaderboards. Configurable time windows with--sinceand--untilflags.Documentation