Sign up
View stargazersSign up

chore(evals): store eval cases as .ndjson so review tools skip them#25782

merged2
main
t3/evals-ndjson-extension
opened5d agobyJJulius Marminge
5 files changed
View on GitHubTwo-way sync enabledPR last imported 5d ago

Macroscope's ignore list skips **/*.ndjson, but the MCP tool-use eval cases used .jsonl, so every case edit got reviewed as code.

Renamed evals/mcp-tool-use/cases/*.jsonl to .ndjson and updated the runner to load *.ndjson cases, plus the --cases example in the README. Run output (runs/*/results.jsonl) is gitignored, so it keeps its name and existing --baseline files still work.

Verified with a --dry-run of one case and vp run check:changed.

Claude Opus 5.5 via Claude Code in T3 Code.

🤖 Generated with Claude Code

Checks

21 successful · 16 skipped
All checks passed
0a32bc1
MmacroscopeappBot5d ago
Synced from GitHub

Approvability

Verdict: Approved at c8fcc5f

Macroscope's review found this PR approvable — This PR mechanically renames evaluation case files to .ndjson and updates the eval runner and documentation, while preserving case contents and .jsonl result handling. Its runtime impact is limited to the private evaluation harness and does not affect product behavior.

No code changes detected at 0a32bc1. Prior analysis still applies.

You can add or adjust custom eligibility rules. Learn more.

CcoderabbitaiBot5d ago
Synced from GitHub

Review in Change Stack →

No actionable comments were generated in the recent review.

External image Recent review info
External image Run configuration
External image Commits

Reviewing files that changed from the base of the PR and between c8fcc5f970d84489b12b3f9f30c48b98fe6dc363 and 0a32bc1520172e6fed02c64230b1eea164371403.

External image Files selected for processing: 5
  • evals/mcp-tool-use/README.md
  • evals/mcp-tool-use/cases/orchestration.ndjson
  • evals/mcp-tool-use/cases/pull-requests.ndjson
  • evals/mcp-tool-use/cases/visual-preview.ndjson
  • evals/mcp-tool-use/run.ts
External image Files with no reviewable changes: 3
  • evals/mcp-tool-use/cases/orchestration.ndjson
  • evals/mcp-tool-use/cases/visual-preview.ndjson
  • evals/mcp-tool-use/cases/pull-requests.ndjson

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.

Walkthrough

The MCP tool-use evaluator now discovers .ndjson case files and reads case and baseline rows with readNdjson. The README documents .ndjson for the --cases flag. Three case files were renamed from .jsonl to .ndjson.

Changes: MCP eval input format

Layer / File(s)Summary
NDJSON reading and documentation
evals/mcp-tool-use/run.ts, evals/mcp-tool-use/README.md, evals/mcp-tool-use/cases/*.ndjson		The evaluator discovers .ndjson case files and reads case and baseline rows with readNdjson. The reader’s line-by-line decoding behavior is unchanged. The README documents .ndjson for --cases, and the case files use the .ndjson extension.

Priority: External image Low

Estimated code review effort: 1 (Trivial) | ~3 minutes

Change: Other

Possibly related PRs

  • pingdotgg/t3code#24635: Adds the MCP eval runner, case files, and --cases documentation. This change updates the case-file extension and loader.

Suggested reviewers: t3dotgg

Merge Risk: External image Minimal · up to 0a32b

The extension rename preserves default case discovery and existing baseline workflows, with no established merge-blocking risk.

Pre-merge checks: Passed 4
Check nameStatusExplanation
Title checkPassedThe title clearly and concisely describes the primary change: storing evaluation cases as .ndjson files so review tools skip them.
Description checkPassedThe description explains the problem, the file renames, the runner and README updates, compatibility details, and focused verification results. It does not use the template headings or explicitly docu…
Linked Issues checkPassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes checkPassedCheck skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description explains the problem, the file renames, the runner and README updates, compatibility details, and focused verification results. It does not use the template headings or explicitly document scope approval, but it is mostly complete and directly related to the change.

Finishing Touches
đź“ť Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

Added 2 commits

  • chore(evals): keep gitignored run results as .jsonl0a32bc1
  • chore(evals): store eval cases as .ndjson so review tools skip themdbadb20