Add multi-dataset model run prediction uploads - #472
Open
luke-e-schaefer wants to merge 1 commit into
Open
Conversation
Stacks on update-nuc-sdk-for-new-eval-stuff-pt1 (#467). The SDK-visible half of the scaleapi change that gives a model run a resolved *set* of datasets instead of one declared dataset. `Dataset.upload_predictions_for_model_run(model_run_id, predictions, ...)` posts to the new `dataset/{dataset_id}/modelRun/{model_run_id}/uploadPredictions` route, which adds this dataset to the run's set. That is what lets one run be scored against a benchmark whose items span several datasets. `upload_predictions` is untouched and still cannot widen a run — it identifies the run by (dataset, model), so it finds the run already on this dataset or creates a new one. Keeping the two separate mirrors the server, where the existing route deliberately kept its never-widen contract and widening got its own endpoint. `PredictionUploader` now accepts `dataset_id` together with `model_run_id` (previously an assertion rejected the pair) and routes on which identifiers are present. The other two forms are unchanged. Docstring corrections the server change makes necessary: - `create_benchmark_evaluation_v2` and `Benchmark.create_evaluation_v2` said the run's predictions "must cover items from the benchmark's datasets". The server used to enforce that with a 400; it no longer does, and uncovered members score as false negatives. - `ModelRun.predict` infers its dataset from the run, so it fails for a multi-dataset run. Noted, pointing at the new method. Left on the old route: it is deprecated, and switching it would silently turn a stale `dataset_id` passed to `get_model_run()` into a widening upload. Verified: 9 new mock-based tests in tests/test_multi_dataset_model_runs.py pinning all three routes plus the async route, trained_slice_id forwarding and duplicate-id rejection. 68 tests pass across the eval/benchmark/preset/ leaderboard suites. black and isort clean on the changed files. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
luke-e-schaefer
marked this pull request as ready for review
August 3, 2026 16:01
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.
Summary — v0.20.0: multi-dataset model runs
Stacked on #467 (
update-nuc-sdk-for-new-eval-stuff-pt1) — review that first; this PR's diff is only the 7 files below. The SDK-visible half of the scaleapi change that gives a model run a resolved set of datasets instead of one declared dataset.The one new method
Posts to
dataset/{dataset_id}/modelRun/{model_run_id}/uploadPredictions, addingdataset_bto the run's dataset set. That is what lets a single run be scored against a benchmark whose items span several datasets — previouslycreate_benchmark_evaluation_v2returned a 400 for exactly that case.Same
update/asynchronous/batch_size/ file-batching /trained_slice_idarguments asupload_predictions, and it runs the same duplicate-id check.What is deliberately not changed
Dataset.upload_predictionsstill cannot widen a run, and that's the point. It identifies the run by(dataset, model), so it finds the run already on this dataset or creates a new one. Widening got its own endpoint rather than loosening this one — mirroring the server, where the existing route kept its never-widen contract exactly as it was.ModelRun.predictalso stays on the old route. It's deprecated, and switching it would silently turn a staledataset_idpassed toget_model_run()into a widening upload. Its docstring now says it fails for multi-dataset runs and points at the new method.Routing
PredictionUploadernow acceptsdataset_idtogether withmodel_run_id— previously an assertion rejected the pair — and picks the endpoint from which identifiers are present:dataset_id+model_run_iddataset/{ds}/modelRun/{run}/uploadPredictionsdataset_id+model_iddataset/{ds}/model/{model}/uploadPredictionsmodel_run_idalonemodelRun/{run}/predictThe two pre-existing forms route exactly as before.
Access, and the sharp edge
Widening requires write on this dataset and on every dataset the run already covers. That's stricter than it looks necessary, for a reason worth knowing: a run is visible only to users who can read all of its datasets, so adding a dataset to a run can remove that run from a collaborator's view. The set also only ever grows — a later upload never drops a dataset, so it can't widen who can read the run.
Docstring corrections
Both are wrong as of the server change, not merely incomplete:
create_benchmark_evaluation_v2/Benchmark.create_evaluation_v2said the run's predictions "must cover items from the benchmark's datasets". The server enforced that with a 400; it no longer does. Coverage may be partial or empty, and uncovered members score as false negatives.ModelRun.predict— see above.Tests
tests/test_multi_dataset_model_runs.py— 9 mock-based tests pinning all three routes (the whole difference between them is the route), the async route,trained_slice_idforwarding, and that the new entry point runs the duplicate-id check rather than bypassing it.Ran locally: 68 passed across the new file plus the benchmark / eval-v2 / preset / leaderboard suites.
blackandisortclean on the changed files. Diff is purely additive — no reformatting churn from a newer localblack.🤖 Generated with Claude Code
Greptile Summary
Adds multi-dataset model-run prediction uploads.
Dataset.upload_predictions_for_model_run, including synchronous and asynchronous upload paths.PredictionUploaderrouting to support dataset and model-run identifiers together.ModelRun.predictbehavior.Confidence Score: 5/5
The PR appears safe to merge, with no concrete changed-code defects identified.
The new synchronous and asynchronous paths preserve the existing prediction payload, batching, duplicate-ID validation, and job-handling behavior while selecting the intended dataset-and-model-run endpoint.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[PredictionUploader identifiers] --> B{dataset_id and model_run_id?} B -->|Yes| C[dataset/dataset_id/modelRun/model_run_id/uploadPredictions] B -->|No| D{model_run_id only?} D -->|Yes| E[modelRun/model_run_id/predict] D -->|No| F[dataset/dataset_id/model/model_id/uploadPredictions] C --> G[Run may span multiple datasets] E --> H[Deprecated single-dataset inference] F --> I[Resolve or create run for dataset and model]Reviews (1): Last reviewed commit: "Add multi-dataset model run prediction u..." | Re-trigger Greptile