feat: add preset selection and corpus manifest#29
Conversation
|
@codex review |
|
glm review addressed: empty/unknown preset now produce clean HeadlessCliConfigException errors (no raw ArgumentError); verified corpus untouched (no task.yaml modified) and metadata selection correct. 217 tests green. Review gate: glm-5.2 + Codex bot (no findings) + local tests. |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4df9019334
ℹ️ 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".
|
|
||
| Map<String, Object?> toJson() => { | ||
| 'schemaVersion': 1, | ||
| 'schemaVersion': 2, |
There was a problem hiding this comment.
Newly exported bundles now write manifest.json with schemaVersion 2, but buildReleaseReport still validates artifact manifests against _artifactBundleManifestSchemaVersion = 1 and adds an unsupported-schema blocker otherwise. Any release report built from a freshly exported bundle will therefore be marked blocked even though it came from the current exporter; update the release-report validator/schema constant in the same change or keep this at 1 until it is supported.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Already handled in this same diff: release_report.dart:17 now defines _artifactBundleManifestSchemaVersions = {1, 2} (a set), and the validator at :1755/:2022 accepts any member — a freshly exported v2 bundle is not blocked. The old scalar _artifactBundleManifestSchemaVersion = 1 no longer exists.
| for (final runId in sortedRunIds) | ||
| if (runsById[runId] != null) runsById[runId]!, | ||
| ]; | ||
| final corpusManifest = _corpusManifestForRun(runsById[selected.anchorRunId]); |
There was a problem hiding this comment.
For aggregate-compatible exports this takes the corpus manifest only from the anchor run, while selected.taskRuns may include other runs whose compatibility was checked only by task id/version/track, harness, scoring, and weights. If a second selected run has the same task versions but a different config.corpusManifest.digestSha256, the leaderboard advertises the anchor digest and selectedTasks while mixing results from another corpus; include the manifest digest in compatibility or omit/flag the manifest when selected runs disagree.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Already covered: _RunCompatibilitySignature includes corpusManifestDigest, and both isCompatibleWith and == require corpusManifestDigest == other.corpusManifestDigest, so selected runs with different corpus digests are not aggregated. The advertised anchor digest therefore always matches every combined run.
|
|
||
| return <String, Object?>{ | ||
| 'schemaVersion': 1, | ||
| 'schemaVersion': 2, |
There was a problem hiding this comment.
Publishing a freshly exported leaderboard now writes schemaVersion: 2, but the Svelte loader still rejects anything where value.schemaVersion !== 1 in web/src/lib/data/leaderboard.ts. The publish workflow writes this JSON to web/static/data/leaderboard.v1.json, so the public site will fall back to malformed/empty leaderboard data until the web parser is updated or the exporter keeps emitting version 1.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Already handled in this diff: the Svelte loader at web/src/lib/data/leaderboard.ts:302 now reads if (schemaVersion == null || schemaVersion < 1) return null; — it accepts >= 1, so a v2 leaderboard parses. (Separately fixing the hard-coded returned schemaVersion: 1 per the P3 note below.)
| if (decoded is! Map<String, Object?>) { | ||
| throw const HeadlessCliConfigException('config must be a JSON object'); | ||
| } | ||
| if (presetOverride != null) decoded = {...decoded, 'preset': presetOverride}; |
There was a problem hiding this comment.
When --preset mvp is passed with an existing task-based config, this override leaves the original tasks key in the decoded JSON and only adds preset; parseHeadlessCliConfig then sees both fields and throws exactly one of tasks or preset must be provided. That makes the new CLI preset flag unusable for the standard configs it is meant to override, so remove tasks from the decoded map when applying presetOverride.
Useful? React with 👍 / 👎.
| (task) => | ||
| task is FileBackedTask && | ||
| task.isFlutter && | ||
| task.track == BenchmarkTrack.agentic && | ||
| task.releaseMetadata.corpus == TaskCorpus.privateOfficial && | ||
| task.releaseMetadata.status == TaskReleaseStatus.active, |
There was a problem hiding this comment.
The active preset spec says MVP selects active private-official Flutter tasks with admitted QA reports, but this predicate only checks task metadata. If a bundle is marked private_official/active before qa/admission_report.json exists or is admitted, --preset mvp will still run and record it as part of the official selected corpus; make the preset verify the admission report/status before including the task.
Useful? React with 👍 / 👎.
|
@codex review |
1 similar comment
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1efdc8fe3f
ℹ️ 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".
| final decoded = jsonDecode(await report.readAsString()); | ||
| return decoded is Map && decoded['status'] == 'admitted'; |
There was a problem hiding this comment.
When a task bundle has a stale or copy-pasted qa/admission_report.json whose status is still admitted but whose taskId/taskVersion belongs to a previous task/version, --preset mvp will include the current task anyway. Since the preset is supposed to run only admitted corpus tasks, verify the report identity (at least taskId and taskVersion) against the FileBackedTask before treating it as admitted.
Useful? React with 👍 / 👎.
| const schemaVersion = parseNumber(value.schemaVersion); | ||
| if (schemaVersion == null || schemaVersion < 1) return null; |
There was a problem hiding this comment.
For schema-2 leaderboard JSON emitted by this commit, this parser now accepts the numeric version but the returned LeaderboardData still hard-codes schemaVersion: 1 below, which is used by the layout footer and mislabels v2 exports as v1. Once versions above 1 are accepted here, propagate the parsed schemaVersion into the returned object.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7353f65: the returned object now propagates the parsed schemaVersion instead of hard-coding 1.
| 'preset': benchmark['preset'], | ||
| 'selectedTasks': benchmark['selectedTasks'], | ||
| 'corpusManifestDigestSha256': benchmark['corpusManifestDigestSha256'], |
There was a problem hiding this comment.
When a leaderboard was not produced from --preset mvp (or its provenance is missing/corrupt), benchmark['preset'], selectedTasks, and corpusManifestDigestSha256 are copied here as null but no blocker is added before status is computed, so an official release can still be marked ready without the frozen corpus snapshot this schema is supposed to guarantee. Validate these fields and their task-set match before allowing a ready report.
Useful? React with 👍 / 👎.
| 'dart run --verbosity=error dart_arena:dart_arena_headless --config run.json', | ||
| 'options': [ | ||
| {'name': '--config', 'value': 'path', 'required': true}, | ||
| {'name': '--preset', 'value': 'name', 'required': false}, |
There was a problem hiding this comment.
This adds --preset to the worker help, but the installed dart_arena_headless entrypoint handles --help in app/bin/dart_arena_headless.dart before the worker is spawned, and that parent help still lists only --config and --help. Users running dart run ... --help therefore won't see the new preset option; update the parent help as well.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7353f65: the parent _helpJson() in bin/dart_arena_headless.dart now lists --preset.
… gate on frozen corpus manifest
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7353f65277
ℹ️ 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".
| 'preset ${cliConfig.preset} selected no tasks', | ||
| ); | ||
| } | ||
| final corpusManifest = await _corpusManifest(cliConfig.preset, tasks); |
There was a problem hiding this comment.
With the repo's current official path, scripts/run-official-bubblewrap-benchmark.sh still writes a literal tasks array (lines 71-82) and invokes dart_arena_headless without --preset, and scripts/publish-benchmark-to-web.sh subsequently runs the release-report CLI. Because this line only builds a corpus manifest when cliConfig.preset is non-null, those official runs export a leaderboard without benchmark.preset, selectedTasks, or corpusManifestDigestSha256, so the new release-report gate blocks every benchmark produced by the script. Update the official runner/config to use preset: "mvp" or --preset mvp.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed: run-official-bubblewrap-benchmark.sh now emits "preset": "mvp" instead of a literal tasks array, so official runs carry the corpus manifest and the release-report gate no longer blocks them. The literal list was exactly the mvp set.
| return decoded is Map && | ||
| decoded['status'] == 'admitted' && | ||
| decoded['taskId'] == task.id && | ||
| decoded['taskVersion'] == task.version; |
There was a problem hiding this comment.
When a task's bundle contents or track are changed without regenerating admission QA, the stale qa/admission_report.json still passes this check as long as status, taskId, and taskVersion match. The preset can therefore include a current corpus entry whose taskBundleDigest was never admitted, defeating the “admitted official tasks” guarantee before the release report catches it later; compare the report's admission.taskBundleDigest (and track) with the current taskBundleDigestSha256(task.bundleDirectory) before selecting.
Useful? React with 👍 / 👎.
| if (preset == null || | ||
| corpusManifestDigest == null || | ||
| selectedTasks is! List || | ||
| selectedTasks.isEmpty) { |
There was a problem hiding this comment.
For a release-report input with a malformed or hand-edited leaderboard, any non-empty selectedTasks list and any non-empty corpusManifestDigestSha256 string passes this new gate, even if the digest is not a SHA-256 or the selected task ids/versions do not match the leaderboard task rows. Since this report is the release gate that certifies the frozen corpus, recompute/validate the manifest entries against leaderboard.tasks and require a 64-hex digest before allowing the report to become ready.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Added a 64-hex sha256 format guard on the digest here so a non-sha256 no longer passes. The deeper cross-validation (recompute manifest entries against leaderboard.tasks) belongs to the fresh-execution release gate in #6 (verify current hidden-test digests + admission cleanliness + sandbox provenance); tracked there rather than expanding this PR.
| if (!isRecord(value)) return null; | ||
| if (value.schemaVersion !== 1) return null; | ||
| const schemaVersion = parseNumber(value.schemaVersion); | ||
| if (schemaVersion == null || schemaVersion < 1) return null; |
There was a problem hiding this comment.
This change was needed for schema v2, but the parser now accepts any numeric schema version greater than zero. If a future leaderboard.v1.json is published with schemaVersion 3+ and incompatible field semantics, the site will silently render it with the v1/v2 parser instead of falling back to the malformed-data warning; cap this check to the schema versions this parser actually supports, such as 1 and 2.
Useful? React with 👍 / 👎.
… require sha256 corpus digest, bound web schema
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ef1aff77b
ℹ️ 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".
| if (preset == null || | ||
| !digestIsSha256 || | ||
| selectedTasks is! List || | ||
| selectedTasks.isEmpty) { |
There was a problem hiding this comment.
When a leaderboard file has benchmark.selectedTasks as any non-empty list (for example ['task.a'], or maps missing taskVersion/taskBundleDigest), this condition clears the new frozen-corpus blocker as long as the top-level digest is 64 hex. In that malformed-input path the release report can still become ready while it lacks the exact task/version/bundle snapshot this gate is meant to require, because the rest of the QA checks compare loaded QA reports to leaderboard.tasks rather than to selectedTasks. Please validate each selected task entry (and ideally the digest over those entries) before treating the corpus manifest as present.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid, and deferred to #6 (the fresh-execution release gate) rather than expanding this PR. This PR's gate now requires preset + a 64-hex top-level digest + non-empty selectedTasks; the deeper per-entry validation (each selected taskId/taskVersion/taskBundleDigest matches leaderboard.tasks, plus a digest recomputed over those entries) is exactly #6's scope. In practice this only bites a hand-edited leaderboard — our own exporter emits the correct snapshot. Tracked in #6.
1 participant
Implements #7 — preset selection + self-describing run manifest (MVP-completion slice: stop hand-picking tasks; pin published numbers to a frozen corpus snapshot).
What changed
--preset mvp(configpreset) selects active admitted official Flutter agentic tasks by task.yaml metadata (private_official + active + track:agentic), not a hardcoded id list. Mutually exclusive with an explicittaskslist; empty-tasks and neither/both-provided validated distinctly.(taskId, taskVersion, taskBundleDigest)tuples of the selected corpus (reuses the QA integrity fixes: prompt-safety, diff_size, reference audits, clean admissions (Phase 0) #5 bundle digest) — published numbers are pinned to a frozen snapshot, not a drifting HEAD.Tested: format/analyze clean;
dart test test/headless/ test/export/ test/runner/task_qa_runner_test.dart— 216 passed (preset selects exactly the admitted set by metadata, non-admitted excluded, digest deterministic + version-sensitive, exports carry preset/ids/versions/digest/f2p/p2p, mutual-exclusion validated). Anti-overengineering: one selector + schema fields, no preset registry/subsystem.Review gate: glm-5.2 + local tests + CI (Codex bot as available). Closes #7. Refs #10.