Repository navigation
feat(regression): baselines and per-row regression gating (P2) - #50
Conversation
An aggregate says something changed. A baseline says which rows. - Rows join across runs by a content hash of (input, expected_output), not by id: renaming a sample or sorting the YAML keeps its history, while rewriting the expected answer correctly reads as a different test rather than as a regression. - The tolerance is the run's own detectable difference, derived from its repetitions and pass rate, instead of the hard-coded epsilon every competitor ships — a constant that is simultaneously too tight for three executions and too loose for three hundred. --compare-epsilon still accepts a fixed number for contracts that need one. - Status and confidence are reported separately. A single-execution run can see that a row went green to red and cannot prove it; both facts travel. The gate counts every regression by default (a PR should stop for a real break) and only provable ones under --confident-only, and says which is which when it fails. - The baseline is a pointer file naming an existing report artifact, not a copy and not a database row. It travels in a CI artifact and diffs in a pull request — which is the reason this package does not need a hosted service to give CI a memory. - Promoting a report from another dataset is refused (no row would ever join, silently disabling the gate), and --promote-baseline refuses a run that did not finish clean (a shipped regression must not become the new bar). New: eval-harness:baseline, --compare/--max-regressions/--confident-only/ --compare-epsilon/--comparison-out/--promote-baseline on eval-harness:run, samples[].row_hash + sample_aggregates[].row_hash, and the versioned eval-harness.comparison.v1 payload. Tests: 967 (was 905), 2655 assertions. PHPStan level 6 clean, Pint clean. Co-Authored-By: Claude <[email protected]>
Deploying eval-harness with
|
| Latest commit: |
3c611be
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://adc2fbf7.eval-harness.pages.dev |
| Branch Preview URL: | https://feat-p2-baselines.eval-harness.pages.dev |
Deploying laravel-patent-box-tracker with
|
| Latest commit: |
3c611be
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ac6ab98e.laravel-patent-box-tracker.pages.dev |
| Branch Preview URL: | https://feat-p2-baselines.laravel-patent-box-tracker.pages.dev |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 208d624e17
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| $this->line(''); | ||
| $this->line(sprintf('<options=bold>Compared against %s</>', $comparison->referenceLabel ?? 'the reference run')); | ||
| $this->line(sprintf( |
There was a problem hiding this comment.
Keep comparison diagnostics off JSON stdout
When eval-harness:run --json --compare=... is used without --out, the JSON report is written to stdout first and these line() calls append human-readable comparison text and tables to the same stream. The resulting output is not valid JSON, so CI consumers cannot parse the advertised machine-readable report; send diagnostics to stderr or suppress them when JSON is streamed.
AGENTS.md reference: AGENTS.md:L18-L20
Useful? React with 👍 / 👎.
| $repetitions = $this->int($report, 'repetitions') ?? 1; | ||
| $passRate = $this->float($report, 'pass_rate') ?? 0.0; | ||
|
|
||
| return SamplingPrecision::differenceResolution($passRate, $repetitions); |
There was a problem hiding this comment.
Reuse the report's per-row precision for confidence
For --confident-only, this recomputes resolution from the run-wide pass rate, even though EvalReport::precision() deliberately uses mean within-row variance for a per-row gate. On a deterministic mixed suite where half the rows always pass and half always fail, the report correctly records zero within-row variance, but this code treats the pooled 0.5 pass rate as noise and can classify a real row regression as unconfident, allowing it through the gate. Read the current report's precision.resolution or recompute from its within-row variance instead.
Useful? React with 👍 / 👎.
| if ($passRateDelta !== null && abs($passRateDelta) > $resolution) { | ||
| return true; | ||
| } | ||
|
|
||
| return $scoreDelta !== null && abs($scoreDelta) > $resolution; |
There was a problem hiding this comment.
Require confidence from the regressing axis
When the two aggregate axes move in opposite directions, status() may mark the row regressed because pass rate decreased, while this method marks it confident solely because the score mean increased beyond the resolution. For example, a tiny pass-rate drop within noise plus a large score improvement becomes a confident regression and incorrectly fails a --confident-only gate. Confidence must be based on a sufficiently large delta in the direction that produced the regression.
Useful? React with 👍 / 👎.
| $report = $this->readReport($path); | ||
|
|
||
| if ($report === null || ($report['dataset'] ?? null) !== $dataset) { | ||
| continue; |
There was a problem hiding this comment.
Exclude comparison payloads from latest-report discovery
After a run writes --comparison-out=diff.json, that versioned comparison artifact is stored in the same prefix, has a matching dataset, and is normally newer than the report it compared. This filter therefore accepts it as a candidate for the next --compare=latest; because comparison payloads have no sample_aggregates, the next gate sees every current row as added and reports zero regressions. Require the report schema discriminator rather than accepting every dataset-bearing JSON file.
AGENTS.md reference: AGENTS.md:L18-L20
Useful? React with 👍 / 👎.
| $hash = $aggregate['row_hash'] ?? null; | ||
| if (! is_string($hash) || $hash === '') { | ||
| continue; |
There was a problem hiding this comment.
Reject references that predate row hashes
Reports created before this commit remain valid eval-harness.report.v1 artifacts but their aggregates do not contain the newly additive row_hash field. Skipping all such aggregates makes a newly generated run appear entirely added, so promoting an existing report after upgrading silently disables the regression gate instead of detecting changes or rejecting an unsupported reference. Preserve compatibility with valid older reports or fail the comparison explicitly rather than returning a green zero-regression result.
AGENTS.md reference: AGENTS.md:L18-L20
Useful? React with 👍 / 👎.
| try { | ||
| $this->disk()->put($this->pointerPath($dataset), $encoded); | ||
| } catch (Throwable $e) { |
There was a problem hiding this comment.
Treat a false baseline-pointer write as failure
Laravel filesystem adapters can return false from put() without throwing. In that case this method still returns the pointer, causing both baseline commands to announce successful promotion even though no pointer was persisted; check the boolean result and throw EvalRunException just as the report-writing path does.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds baseline management and per-row regression comparison/gating so runs can be compared against a reference (baseline/latest/path) and fail CI with row-level diagnostics.
Changes:
- Introduces content-based row hashing (
row_hash) and threads it through JSON reports and sample aggregates. - Adds regression comparison primitives (
RunComparator,RunComparison,RowComparison) plus gating policy (RegressionGate). - Adds baseline pointer storage + CLI workflows (
BaselineStore,eval-harness:baseline) and extendseval-harness:runwith compare/gate/promote options.
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Unit/Regression/RunComparatorTest.php | Adds unit coverage for row-join semantics, resolution/epsilon behavior, and payload counts. |
| tests/Unit/Regression/RegressionGateTest.php | Adds unit coverage for gate counting rules, confidence-only mode, and failure messaging. |
| tests/Unit/Regression/BaselineStoreTest.php | Adds unit coverage for baseline pointer promotion, resolution, clearing, and path safety. |
| tests/Unit/Datasets/RowHashTest.php | Verifies hash stability across renames/metadata/key order and sensitivity to semantic changes. |
| tests/Unit/Console/EvalCommandCompareTest.php | End-to-end tests for eval-harness:run compare/gate/promote behavior and artifacts. |
| tests/Unit/Console/BaselineCommandTest.php | Tests baseline CLI promote/show/clear, including invalid inputs. |
| src/Reports/SampleAggregate.php | Extends aggregate contract with row_hash and updates factory/serialization. |
| src/Reports/JsonReportRenderer.php | Emits samples[].row_hash in the JSON report output. |
| src/Reports/EvalReport.php | Computes/stores row_hash in sample_aggregates using RowHash. |
| src/Regression/RunComparison.php | New value object for comparison results + summary/payload serialization. |
| src/Regression/RunComparator.php | New comparator joining current/reference runs by row_hash and computing deltas/status/confidence. |
| src/Regression/RowComparison.php | New row-level comparison model and JSON shape, including “newly failing”. |
| src/Regression/RegressionSchema.php | Defines version identifiers for comparison and baseline pointer payloads. |
| src/Regression/RegressionGate.php | New gate policy for pass/fail decisions based on regressions and thresholds. |
| src/Regression/BaselineStore.php | New baseline pointer store, report resolution, latest-report lookup, and safe dataset slugging. |
| src/EvalHarnessServiceProvider.php | Registers new command and binds regression services. |
| src/Datasets/RowHash.php | New canonical hashing implementation for dataset row identity. |
| src/Console/EvalCommand.php | Adds compare/gate/promote flow after a run, plus option validation. |
| src/Console/Concerns/WritesEvalReports.php | Adds writeArtifact() and tracks last written report artifact path. |
| src/Console/Concerns/ComparesRuns.php | New shared concern for resolving references, comparing, rendering, gating, and writing comparison artifacts. |
| src/Console/BaselineCommand.php | New artisan command for promoting/showing/clearing baselines. |
| docs/PROGRESS.md | Documents the feature drop and key design decisions. |
| docs/LESSON.md | Captures learnings about option validation, output assertions, slug behavior, and artifact writing. |
| docs-site/docs/guides/baselines-and-regressions.md | New user guide for baselines, row hashing, tolerances, and comparison payloads. |
| docs-site/docmd.config.json | Adds navigation entry for the new guide. |
| README.md | Adds README feature bullet for baselines and per-row regressions. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| public function isNewlyFailing(): bool | ||
| { | ||
| return $this->isRegression() | ||
| && ($this->before['pass_rate'] ?? null) !== null | ||
| && $this->before['pass_rate'] >= 1.0 | ||
| && ($this->after['pass_rate'] ?? 1.0) < 1.0; | ||
| } |
| use Padosoft\EvalHarness\Datasets\RowHash; | ||
| use Padosoft\EvalHarness\Statistics\SamplingPrecision; |
| $this->info(sprintf( | ||
| 'Wrote %d bytes to disk [%s] at path [%s].', | ||
| strlen($payload), |
| $reportDataset = $payload['dataset'] ?? null; | ||
|
|
||
| // Promoting a report from a different dataset would silently make every | ||
| // later comparison meaningless — the rows would never join, so every row | ||
| // would read as "added" and no regression could ever be detected. | ||
| if (is_string($reportDataset) && $reportDataset !== $dataset) { | ||
| $this->error(sprintf( | ||
| "Report [%s] belongs to dataset '%s', not '%s'. Refusing to promote it.", | ||
| $path, | ||
| $reportDataset, | ||
| $dataset, | ||
| )); | ||
|
|
||
| return self::FAILURE; | ||
| } |
Applies all six findings from the PR #50 review round. Two of them ended in a gate that passes when it should not, which is the only kind of bug a regression gate cannot afford. - A reference report with no per-row data joined nothing, so every current row read as "added" and the run reported zero regressions and passed. The join key is now negotiated — content hash when both sides carry it, sample id as a degraded fallback, and none at all when the reference has nothing to join on. That last case is reported as comparable: false with a reason, and the command warns and skips gating exactly as it does for a missing baseline. - A comparison payload written by --comparison-out sits in the reports prefix, names the same dataset and is newer than the report it describes, so it won --compare=latest and, having no aggregates, made the next gate see zero regressions. Only artifacts declaring the report schema are candidates now. Plus: - Comparison diagnostics no longer land in a JSON report streamed to stdout. They go to stderr where one exists and are dropped when it does not, because an unparseable payload fails a CI job somewhere else entirely, which is worse than an unprinted note. - The tolerance comes from the report's own precision block rather than being recomputed from the pooled pass rate. On a deterministic suite where half the rows always pass, the pooled 0.5 implies noise the run does not have, and a real regression could be waved through --confident-only as unprovable. - Confidence is judged on the axis that produced the verdict. Judged independently, a score improvement could certify a pass-rate regression and fail a --confident-only gate on the strength of good news. - A filesystem put() returning false now raises instead of announcing a promotion that never landed. Tests: 994 (was 981), 2721 assertions, including one that parses the JSON report off stdout while a comparison runs. Co-Authored-By: Claude <[email protected]>
|
All six applied in
Tests: 994 (was 981), 2721 assertions; PHPStan level 6 clean; Pint clean. Generated by Claude Code |
| $pointer = $baselines->pointer($dataset); | ||
| $path = is_string($pointer['report_path'] ?? null) ? $pointer['report_path'] : 'baseline'; | ||
|
|
||
| return ['payload' => $payload, 'label' => sprintf('the baseline [%s]', $path)]; |
| $reportDataset = $payload['dataset'] ?? null; | ||
|
|
||
| // Promoting a report from a different dataset would silently make every | ||
| // later comparison meaningless — the rows would never join, so every row | ||
| // would read as "added" and no regression could ever be detected. | ||
| if (is_string($reportDataset) && $reportDataset !== $dataset) { | ||
| $this->error(sprintf( | ||
| "Report [%s] belongs to dataset '%s', not '%s'. Refusing to promote it.", | ||
| $path, | ||
| $reportDataset, | ||
| $dataset, | ||
| )); | ||
|
|
||
| return self::FAILURE; | ||
| } |
Second review round on PR #50. Three of the five findings were false positives (`??` already suppresses the null-offset warning, short-circuit ordering already prevented the second access, and a `{@see}` docblock link is a real use of an import) — but two of them flagged code that made a reader ask the question, so the reads are now explicit locals. The real one: promotion refused a report from a *different* dataset but accepted a payload with no `dataset` field at all, and accepted any JSON regardless of whether it was a report. Both silently disable the gate in the same way — nothing joins, every row reads as added, zero regressions, green. Promotion now requires the report schema version and an exact dataset match; "cannot prove it matches" is not a reason to accept. Also: writeArtifact's success line now names what it wrote, since a run can emit a report and a comparison through the same helper. Tests 994 -> 997. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01EZ4u82zdoa8naYm75kppMC
Second item from the 2026-08-24 competitor analysis, on top of #49. An aggregate says something changed; a baseline says which rows.
The capability existed here only inside the adversarial lane (manifests). This generalises it to the main lane and improves on the reference implementation in two places.
What lands
eval-harness:baseline {dataset} [--report=] [--show] [--clear]— promote, inspect, forget.eval-harness:run --compare=baseline|latest|<path> --max-regressions=N— plus--confident-only,--compare-epsilon,--comparison-out,--promote-baseline.Regression\RunComparatorjoining two reports row by row,RegressionGateapplying the policy, and a versionedeval-harness.comparison.v1payload.Three decisions worth reviewing
1. Rows join on content, not on id. The key is a SHA-256 of (input, expected_output) — not the id, not the metadata, not the key order. Renaming a sample or sorting the YAML keeps its history; rewriting the expected answer correctly reads as removed + added, because the measurements taken against the old row no longer describe the new test. The id is still what every table and message shows — a hash means nothing to a human.
2. The tolerance is measured, not chosen. Instead of the fixed epsilon every tool in this space ships — a constant that is simultaneously too tight for three executions and too loose for three hundred — the tolerance is the run's own detectable difference from #49.
--compare-epsilonstill accepts a fixed number for contracts that need one, and the payload says which of the two was used (resolution_is_statistical).3. Status and confidence are reported separately. A single-execution run can see that a row went green to red and cannot prove it; both facts travel (
status,confident). The gate counts every regression by default — a PR should stop for a real break — and only provable ones under--confident-only, for a scheduled lane where a false alarm costs more than a day. When it fails on rows it could not prove, the message says so: "1 of 3 exceed this run's detectable difference of 53.3 points, the rest are within sampling noise". A gate that hides that loses the audience it needs the next time it is right.Why the baseline is a file
When runs and baselines live only in a database, the baseline lives in one database — whoever promoted it — and CI, starting from an empty schema every time, has no history at all. That gap is what a hosted service then gets sold to fill.
Here the baseline is a pointer to a report artifact that already exists on the reports disk: it travels in a CI artifact, commits next to the dataset, and a comparison in CI reads the same bytes the developer read locally. Two refusals keep it honest — promoting a report from a different dataset (no row would join, silently disabling the gate) and
--promote-baselineon a run that did not finish clean (a shipped regression must not become the new bar).A missing or deleted baseline warns and keeps the run's own exit code. A build going red because somebody cleaned a bucket teaches everyone to ignore the gate.
Gates
composer validate --strictvendor/bin/phpunitvendor/bin/phpstan analyse(level 6)vendor/bin/pint --testNew tests:
RowHashTest,RunComparatorTest,RegressionGateTest,BaselineStoreTest,BaselineCommandTest,EvalCommandCompareTest(end-to-end: run → promote → break a row → gate fails naming it).Report contract additions:
samples[].row_hash,sample_aggregates[].row_hash. Docs:docs-site/docs/guides/baselines-and-regressions.md+ nav, README bullet, PROGRESS/LESSON.Next in the series: an optional database index of runs/rows with
eval-harness:reindex(the artifact stays the source of truth), then trajectory metrics.Generated by Claude Code