Skip to content

Implement gate() and parseReport() - #5

Open
ruanmartinelli wants to merge 2 commits into
mainfrom
claude/code-review-improvements-5kqcfs
Open

Implement gate() and parseReport()#5
ruanmartinelli wants to merge 2 commits into
mainfrom
claude/code-review-improvements-5kqcfs

Conversation

@ruanmartinelli

@ruanmartinelli ruanmartinelli commented Jul 5, 2026

Copy link
Copy Markdown
Owner

gate and loadBaseline are exported from the public API but were TODO: implement stubs that throw at runtime. This fills them in — with one design change per review feedback: the library never touches node:fs.

What

  • parseReport(source) (replaces loadBaseline(path)) — a pure function that takes a JSON string or an already-parsed value, validates the basic { name, byModel } shape, and returns the typed report. The caller owns I/O:
    const baseline = parseReport(readFileSync('baseline.json', 'utf8'))
  • gate(report, baseline, opts?) — already pure; compares per model:
    • overall score dropped below baseline → regression
    • baseline model missing from the fresh report → regression
    • opts.byTag holds each baseline model's per-tag scores to the same standard
    • opts.tolerance (default 0) forgives drops of at most that much — useful for absorbing eval noise in CI
    • models/tags that are new in the fresh report are ignored (not regressions)

Regression strings are human-readable and CI-log friendly, e.g. m1: overall 0.7000 < baseline 0.8000.

Also adds node-compile-cache (dropped in the repo root by test runs) to .gitignore.

Tests

The placeholder it.todos are now 16 real tests — all pure, no temp files: JSON round-trip, already-parsed input, malformed values, every regression path, tolerance boundaries, and the empty-baseline case. pnpm typecheck && pnpm test: 85 passing.

Notes

  • loadBaseline is gone rather than kept as an alias — it was an unusable stub on main, so nothing can depend on it.
  • opts.tolerance is a small addition beyond the original doc comment (which only mentioned byTag) — happy to drop it if you'd rather keep the surface minimal.
  • The refactor plan in Proposal: v0.2 refactor plan (persistence, hooks, timeouts, per-case report) #10 will be updated to reflect the no-node:fs principle (its phase 1 proposed fs-touching save/load helpers).

History

The first commit implemented loadBaseline with readFile; the second makes the library fs-free per review feedback.

🤖 Generated with Claude Code

https://claude.ai/code/session_015XMPpi4PX2bWwAoMLaZZ4k

Replace the TODO stubs with real implementations:

- loadBaseline reads and parses a saved JSON report, validating its
  basic { name, byModel } shape with clear error messages.
- gate compares a fresh report against a baseline per model: an overall
  score drop, or a baseline model missing from the report, is a
  regression. opts.byTag extends the same check to per-tag scores, and
  opts.tolerance (default 0) forgives small drops to absorb eval noise.
  New models and tags are ignored.

Converts the placeholder TODO tests into real coverage and documents
both functions in the README. Also gitignores node-compile-cache,
which the test run drops in the repo root.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015XMPpi4PX2bWwAoMLaZZ4k
@ruanmartinelli
ruanmartinelli force-pushed the claude/code-review-improvements-5kqcfs branch from 549fe56 to bf526fa Compare July 7, 2026 00:43
The library should never touch node:fs. parseReport takes a JSON
string (or an already-parsed value), validates the { name, byModel }
shape, and returns the report — the caller owns the readFile. gate was
already pure and is unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015XMPpi4PX2bWwAoMLaZZ4k
@ruanmartinelli ruanmartinelli changed the title Implement gate() and loadBaseline() Implement gate() and parseReport() Jul 7, 2026
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