Adds the evaluation layer on top of the review traces: five deterministic
scores describing how the reviewer behaved, a bridge that turns human reactions
into ground truth, and a dataset seeded from the reviews already run.
The two are kept apart on purpose. feedback.db has recorded 113 reviews and
zero reactions, resolutions or replies — nobody has ever responded to a bot
comment — so an accuracy metric cannot be built yet. The scorers therefore
measure behaviour, which is computable from data in hand, and feedback_scores
turns verdicts into scores the moment any arrive.
eval_scores.py emits finding_rate, severity_info_ratio, severity_max,
dropped_findings and cost_per_finding into the same ingestion batch as the
trace. Undefined values are omitted rather than reported as zero: an info ratio
over a silent review is undefined, and charting it as 0 would read as perfect
calibration.
dropped_findings needed a parser change. Both parsers silently discard findings
with an unusable path/line, which made a model emitting garbage locations
indistinguishable from one that found nothing. last_parse_dropped() exposes the
delta, read at parse time — after apply_repo_config the drops are the config
working as intended, not the model misbehaving.
feedback_scores.py scores the session ("{repo}#{pr}"), because feedback arrives
days later against a PR and nothing records which re-run produced which
comment. review_acceptance is absent rather than 0 when nothing was engaged.
eval_bootstrap.py registers the score configs, seeds the pragent-reviews
dataset, and can backfill scores onto traces that predate the scorers.
expectedOutput is the reviewer's own prior output, flagged
labelled_by_human: false — a regression baseline, not verified truth.
Also fixes a silent telemetry failure: the ingestion endpoint answers 207 when
only some events succeed, so a batch with every event rejected still looked
like success. Score events were missing the required per-event timestamp and
ingested nothing while reporting 207. _warn_on_rejected_events now logs the
per-event errors under LANGFUSE_DEBUG.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
6.1 KiB
Evaluation — scorers, ground truth, and the dataset
Langfuse already receives one trace per review (README-langfuse.md). This is
the layer on top: numbers attached to those traces that say how the reviewer
behaved, and the beginnings of a ground-truth signal that says whether it was
right.
Those two things are deliberately kept apart, because only one of them exists yet.
What could and could not be built
feedback.db has recorded 113 reviews across 4 repos. It has recorded zero
reactions, zero thread resolutions and zero replies. The harvester, the schema
and the daily analyzer are all working; nobody has ever reacted to a bot
comment.
That rules out an accuracy metric today. Correctness needs labels, and a judge scored against no labels is theatre. So the scorers here measure behaviour, which is computable from data already in hand, and a separate bridge exists to turn human reactions into scores the moment any arrive.
The five behavioural scores
Emitted with every review by eval_scores.py, folded into the same ingestion
batch as the trace so they cost no extra request.
| score | type | what a change in it means |
|---|---|---|
finding_rate |
NUMERIC | Findings posted. 0 is the restraint case — good on clean code, a failure when the run degraded. Only the rate over time separates those. |
severity_info_ratio |
NUMERIC 0–1 | Share of findings the model rated info/trivial. Rising = the model is hedging rather than committing. None when the review was silent: a ratio over an empty set is undefined, and charting it as 0 would read as perfect calibration. |
severity_max |
CATEGORICAL | Highest severity surfaced, none when silent. Categorical because "did this ever surface something serious" is the real question, and a mean of severity ranks answers nothing. |
dropped_findings |
NUMERIC | Findings the model emitted that the parser rejected for an unusable path/line. This is the only score here that measures the model's raw output. |
cost_per_finding |
NUMERIC | Equivalent USD per finding. A cheaper model that finds nothing is not cheaper. |
Why dropped_findings needed a change to the parser
parse_findings and parse_review_output discard any finding with a missing or
unusable location. That happens silently, so a model emitting ten findings at
invalid locations was indistinguishable from a model that found nothing — both
produce an empty list. ai_review.last_parse_dropped() exposes the delta,
recorded at parse time.
It must be read at parse time specifically: by the time findings reach
_emit_langfuse, apply_repo_config has already filtered them by
severity_threshold and max_findings, and those drops are the config working
as intended, not the model misbehaving.
Ground truth: feedback_scores.py
Turns feedback.db into two session-level scores, keyed on "{repo}#{pr}"
(which is what langfuse_trace already sets as sessionId).
| score | meaning |
|---|---|
review_engagement |
Share of a PR's findings that drew any human reaction, resolution or reply. Watch this first — every quality number is vapour until it moves off 0. |
review_acceptance |
Net verdict over engaged findings, −1 to +1. Absent, not 0, when nothing was engaged: zero would claim humans judged the review neutral, when the truth is nobody looked. |
Session-level rather than trace-level because feedback arrives days later
against a PR, and nothing in feedback.db records which re-run of the reviewer
produced which comment. The session is both the available join and the honest
granularity.
Score ids are uuid5(namespace, repo#pr#name), so the daily backfill updates
rather than duplicates.
The dataset
pragent-reviews, one item per PR the reviewer has run on, seeded by
eval_bootstrap.py from feedback.db.
expectedOutput is the reviewer's own prior output, not human-verified
truth — every item carries metadata.labelled_by_human: false. Read it as a
regression baseline: re-run a candidate model over these PRs and the diff
against this column is the behaviour change. Promoting an item to real ground
truth means a human editing it in the dataset view after re-reading the PR.
Running it
# once per project: score configs + dataset (+ score historical traces)
python3 pilot/eval_bootstrap.py --db /data/feedback.db --backfill-traces
# ship feedback verdicts (runs daily from the feedback CronJob)
python3 pilot/feedback_scores.py --db /data/feedback.db
Both need LANGFUSE_HOST, LANGFUSE_PUBLIC_KEY, LANGFUSE_SECRET_KEY. In
cluster they come from the pragent-langfuse Secret and point at the ClusterIP
— never the NodePort, whose oauth2-proxy 302s ingestion to Logto and drops it.
Gotcha: HTTP 207 is not success
The ingestion endpoint answers 207 Multi-Status when some events failed, so
a batch where every event was rejected still returns 207. An early version
of these scorers omitted the required per-event timestamp and silently
ingested nothing while reporting success. langfuse_trace._warn_on_rejected_events
now logs the per-event errors under LANGFUSE_DEBUG=1. If scores are missing,
check that before anything else.
What the first run showed
Backfilled over 42 existing traces and 13 PRs:
cost_per_finding n=42 mean=0.3133 min=0.0880 max=0.9042
finding_rate n=42 mean=0.4762 min=0.0000 max=4.0000
severity_info_ratio n=14 mean=0.0000
review_engagement n=14 mean=0.0000
severity_max {none: 28, medium: 11, high: 1, critical: 2}
Two things worth keeping:
- The reviewer is not info-heavy.
feedback.dbshows 61 of 62 findings atINFO, which looked like a badly calibrated model. It is not:severity_maxreadsmedium/high/criticalon every trace that found anything, andseverity_info_ratiois flat 0. TheINFOin the DB comes fromfeedback_harvest._parse_severity, which defaults toINFOwhen its regex misses the severity badge in the rendered comment. The DB severity is a re-parse artifact; the score reads the model's structured output directly. - 28 of 42 reviews found nothing (67%), and engagement is flat zero. The first is not yet interpretable without the second.