Skip to content

Reviewer cache monitoring: in-run emit + scheduled collector#57

Draft
kacper-mikolajczak wants to merge 3 commits into
reviewer-cache-prefixfrom
reviewer-cache-monitoring
Draft

Reviewer cache monitoring: in-run emit + scheduled collector#57
kacper-mikolajczak wants to merge 3 commits into
reviewer-cache-prefixfrom
reviewer-cache-monitoring

Conversation

@kacper-mikolajczak

@kacper-mikolajczak kacper-mikolajczak commented Jun 22, 2026

Copy link
Copy Markdown

Stacked on the cache-prefix change so this diff is just the monitoring. Split into two parts so analysis/alerting lives out of the review hot path.

1. In-run (minimal) — emit-cache-usage.sh

A tiny step in the reviewer workflow that parses the action's execution_file (always written, no show_full_output) and prints one counts-only line to the log:

CACHE_USAGE creation=<n> read=<n> cost=<n>

No message content → safe in public logs. No analysis, no threshold, no failure.

2. Out-of-band collector — reviewer-cache-monitor.yml + collect-cache-health.sh

A scheduled workflow (hourly + manual dispatch) that:

  • lists recent claude-review.yml runs, reads their logs (only — no artifacts/content), greps the CACHE_USAGE line,
  • builds a per-run warm-ratio table in the step summary,
  • computes the cold fraction over the sample and fails the run (red cron = built-in alert) if it exceeds a threshold.

Tunables: LOOKBACK (30), CACHE_CREATION_WARN_TOKENS (15K), COLD_ALERT_PCT (40), MIN_SAMPLE (10).

Why this shape

  • Hot path stays minimal — one echo per review, no per-run analysis.
  • Trend-aware — distinguishes a one-off cold start (normal, first run after an idle gap) from a sustained regression (rules edit, action/CLI bump, model change, lost 1h TTL). A per-run check can't.
  • Logs-only — the collector needs no artifacts and no message content; it just scans the emitted line.
  • Non-intrusive alert — a regression turns the scheduled run red (and surfaces in GitHub's failed-workflow notifications) instead of blocking PRs.

Validated locally: emit produces the line for warm (creation 1.4K / read 333K) and cold (creation 64K) execution files; collector parse + threshold logic checked; both workflows parse as valid YAML.

@coderabbitai

coderabbitai Bot commented Jun 22, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 372bd77f-eb6a-42a5-8864-a2dbd6110b08

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch reviewer-cache-monitoring

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.

@kacper-mikolajczak kacper-mikolajczak changed the title Reviewer prompt-cache health monitoring + cold-cache regression warning Reviewer cache monitoring: in-run emit + scheduled collector Jun 22, 2026

@matkoson matkoson left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

.github/scripts/collect-cache-health.sh:39 @kacper-mikolajczak what happens if both creation=0 and read=0 (e.g., a run that didn't use the reviewer or had no cache)? The ratio defaults to 0%, which would look like a perfectly warm run. Should there be a distinction between "truly warm" and "no cache data"?

generated by an automated pr-review pipe, not written by a human. reply and tag the repo owner if it got something wrong.

A run that emits creation=0 read=0 (reviewer skipped or no usage-bearing
turns) passed the parse guard, counted toward scanned/MIN_SAMPLE, and read
as not-cold - diluting the cold fraction and masking regressions. Treat no
cache activity as no data, not a warm sample.
@kacper-mikolajczak

Copy link
Copy Markdown
Author

Re: review on collect-cache-health.sh:39

@matkoson Good catch, this is a real gap. A run that emits creation=0 read=0 (reviewer skipped or produced no usage-bearing turns) currently passes the parse guard, counts toward scanned/MIN_SAMPLE, and is treated as not-cold - so it dilutes the cold fraction and can mask a regression.

One nuance on the wording: in the summary table such a run actually renders as 0% warm (cold-looking), not "perfectly warm". The masking happens in the alert math, not the table. Same root cause though: a no-cache-data run shouldn't count as a data point at all.

Fixed in e34569e by dropping zero/zero runs before they're counted, so they neither hit MIN_SAMPLE nor skew the cold %:

[ -n "$c" ] && [ -n "$r" ] || continue
# no cache activity at all = no data, not a warm sample
[ "$c" -eq 0 ] && [ "$r" -eq 0 ] && continue

This keeps the sample to runs that genuinely exercised the cache.

@matkoson

Copy link
Copy Markdown

@kacper-mikolajczak #27 (comment) - an accident, apologies, but at least somewhat useful as it turnes out ^^

@kacper-mikolajczak

kacper-mikolajczak commented Jun 29, 2026

Copy link
Copy Markdown
Author

Haha no worries @matkoson! 😄 Your agent indeed made a proper remark here - thanks for accidental review ❤️

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