fix(eval): run the harness on Windows and keep the baseline POSIX - #193
Merged
Merged
Conversation
Two gates the v1.5 eval path needs to be trustworthy on every platform. **`npm run eval` could not start on Windows.** The three scripts resolve `node_modules/.bin/electron.cmd` and spawn it. Node 24 refuses to spawn a `.cmd`/`.bat` without `shell: true` and fails with `EINVAL`, so the harness was unrunnable on the platform this project is developed on. The `electron` package exports the path to the real executable the wrapper runs, which spawns directly on every platform and needs no shell. **A Windows run rewrote the committed baseline.** `path.relative` returns backslashes on Windows, so `config.corpus` was written as `eval\corpus` instead of `eval/corpus`. The file's whole contract is that it is identical on every machine — the CI determinism check diffs it — and a Windows run silently broke that. The label is now normalised to POSIX separators. Verified on Windows with the pinned model: `npm run eval` runs and leaves `docs/eval/baseline-v1.5.json` byte-identical to the committed file.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Unblocks the two eval gates that make the v1.5 harness usable on Windows. No behaviour change on Linux/macOS; no metric changes.
Part of #192 — these are prerequisites, not one of its child issues: none of the evaluation work in that epic can be verified from a Windows checkout until these two are fixed.
The two bugs
1.
npm run evalcould not start on WindowsAll three eval scripts resolve
node_modules/.bin/electron.cmdandspawnit. Node 24 refuses to spawn a.cmd/.batwithoutshell: true(the CVE-2024-27980 mitigation), so the harness was unrunnable on Windows with the Node version this repo declares (>=24 <25).The
electronpackage exports the path to the real executable the.binwrapper runs, so the scripts spawn that directly — no shell, no.cmd, works on every platform.2. A Windows run rewrote the committed baseline
path.relativereturns backslashes on Windows, soconfig.corpuswas written with them. The file's stated contract — in the source comment, ineval/README.md, and in the CI step thatdiffs the JSON — is that it is identical on every machine. A Windows run silently broke that, sonpm run evalon Windows could never produce a cleangit diff. The label is normalised to POSIX separators.How was this tested
npm run typecheck— cleannpm test— 478 passnpm run evalon Windows with the pinned model — runs, and leavesdocs/eval/baseline-v1.5.jsonbyte-identical to the committed file, which neither bug allowed before{"recallAt1":0.833333,"recallAt5":1,"recallAt10":1,"mrr":0.927778,"ndcgAt10":0.94375,"evidencePrecisionAt5":0.213333}docs/eval/baseline-v1.5.mddiffers only in the informational timing line (indexing ms, p50/p95), which is deliberately not frozen and is not diffed by CI.Related
Part of #192.