fix: preserve scorecard errors and revision integrity - #16
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
🟡 Changes recommended
There is a TypeScript type mismatch in classifyResult and a potential UTC-midnight flake in the new summarize test that should be fixed before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Tightens skillcheck summarize/result reduction semantics so transport errors don’t become scored failures, and partial reruns can’t silently merge rows from a different skills-tree revision into a scorecard.
Changes:
- Route
reduceResults()through the existingclassifyResult()to skip ungraded transport errors while retaining graded assertion failures. - Add provenance validation after merging carried scorecard rows, rejecting mixed-revision scorecards unless
--allow-mixed(and avoiding overwrite on rejection). - Update docs and release workflow to reflect the new behavior and publish on a GitHub-hosted runner for npm provenance.
File summaries
| File | Description |
|---|---|
| test/cli.test.ts | Adds regression tests for transport-error skipping, malformed rows, and mixed-revision retention behavior in summarize. |
| src/cli.ts | Reuses classifyResult() during reduction and validates mixed revisions after merge before writing scorecards. |
| docs/usage.md | Documents skipping transport errors and mixed-revision rejection behavior (including “no overwrite on rejection”). |
| .github/workflows/release.yml | Switches publish runner to GitHub-hosted Ubuntu to satisfy npm provenance constraints. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8a9dbf01a9
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1fab785a49
ℹ️ 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".
There was a problem hiding this comment.
✨ CLEAN
The PR routes scorecard reduction through classifyResult, refuses to merge a skipped rerun into an existing same-date row, and rejects mixed revisions after merge. Tests cover those paths, and the release runner change matches npm's GitHub-hosted provenance requirement.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 98f0a57dd5
ℹ️ 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".
Problem
Transport errors could become scored failures, and partial reruns could silently retain rows from a different skills revision.
Solution
Use the existing result classifier and validate provenance after merging retained rows, before writing the scorecard. Preserve graded failures, explicit mixed-revision overrides, and the previous file on rejection. Reject skipped reruns that overlap retained rows so an old score cannot silently survive a failed rerun. Persist attempted identities until graded output and provenance are complete, including when the child writes no result file. Move only npm publishing to a GitHub-hosted runner because provenance rejects self-hosted runners.
Proof
Regression fixtures cover transport errors, graded failures, malformed rows, mixed-revision rejection without overwrite, override, and complete replacement. The uncached full gate passed 50 source tests and the packed-consumer install/lint test. actionlint and zizmor passed; independent Astra review is clean. No paid evaluations ran.