Repository navigation
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 526126fc-97a2-4353-93c1-807497ff8960
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved identity-handling regressions affect saved-plan restoration, result discovery, capability output validity, and Docker image operations.
5 open issues
What changed in this PR
This PR separates resource identity from filenames and directories, allowing benchmark, experiment, and scenario configs to pin IDs across renames.
Changes:
- Add optional nonblank IDs, with namespaced hashes of relative paths as defaults.
- Resolve selectors by ID first, detect duplicate identities, and retain readable report labels.
- Add regression tests, migration guidance, and a minor changeset.
| File | Description |
|---|---|
| packages/agent-eval/src/scenario/scenario.ts | Add scenario ID hashing and collision detection. |
| packages/agent-eval/src/scenario/scenario.test.ts | Test hashed IDs and selector precedence. |
| packages/agent-eval/src/scenario/load.ts | Resolve explicit or root-relative scenario IDs. |
| packages/agent-eval/src/scenario/load.test.ts | Test identity stability and invalid IDs. |
| packages/agent-eval/src/scenario/list.ts | Pass the configured root and detect collisions. |
| packages/agent-eval/src/scenario/list.test.ts | Update ID expectations and test collisions. |
| packages/agent-eval/src/scenario/get.ts | Support ID-first directory selection. |
| packages/agent-eval/src/scenario/config.ts | Accept optional nonblank IDs. |
| packages/agent-eval/src/path.ts | Add normalized relative-path helpers. |
| packages/agent-eval/src/experiment/report.ts | Separate scenario labels from grouping IDs. |
| packages/agent-eval/src/experiment/plan.test.ts | Test pinned IDs in benchmark experiments. |
| packages/agent-eval/src/experiment/list.ts | Resolve resource IDs and detect collisions. |
| packages/agent-eval/src/experiment/list.test.ts | Cover identity, selectors, and plan restoration. |
| packages/agent-eval/src/experiment/get.ts | Support ID-first filename selection. |
| packages/agent-eval/src/experiment/experiment.ts | Add namespaced experiment ID hashing. |
| packages/agent-eval/src/experiment/config.ts | Accept IDs for both experiment types. |
| packages/agent-eval/src/benchmark/report.ts | Display directory labels instead of scenario IDs. |
| packages/agent-eval/src/benchmark/plan.test.ts | Update plan expectations for hashed IDs. |
| packages/agent-eval/src/benchmark/list.ts | Resolve IDs and check resource collisions. |
| packages/agent-eval/src/benchmark/list.test.ts | Cover pinned IDs, selectors, and collisions. |
| packages/agent-eval/src/benchmark/get.ts | Support ID-first filename selection. |
| packages/agent-eval/src/benchmark/config.ts | Accept optional nonblank IDs. |
| packages/agent-eval/docs/references/scenarios.md | Document scenario identity and selectors. |
| packages/agent-eval/docs/references/plans.md | Explain identity preservation for saved plans. |
| packages/agent-eval/docs/references/experiments.md | Document experiment IDs and selection. |
| packages/agent-eval/docs/references/benchmarks.md | Document benchmark IDs and references. |
| packages/agent-eval/docs/migrations/resource-identity.md | Explain how to preserve existing identities. |
| packages/agent-eval/docs/migrations/README.md | Index the resource identity migration. |
| packages/agent-eval/docs/AGENTS.md | Link the identity migration guidance. |
| docs/scenarios.md | Explain optional scenario IDs. |
| docs/experiments.md | Explain optional experiment IDs. |
| docs/benchmarks.md | Explain optional benchmark IDs. |
| .changeset/cool-readers-glow.md | Declare the minor release and migration link. |
Review effort: Balanced
Comment on lines
+45
to
+48
| }) ?? | ||
| benchmarks.find(candidate => { | ||
| return getConfigPath(benchmarksDirectory, candidate.filepath) === name | ||
| }) |
|
|
||
| benchmarks.push({ | ||
| id: getBenchmarkId(filename), | ||
| id: config.id ?? getBenchmarkId(getConfigPath(benchmarksDirectory, filepath)), |
Comment on lines
+95
to
+99
| assertUniqueScenarioIds( | ||
| capabilities.flatMap(capability => { | ||
| return capability.scenarios | ||
| }), | ||
| ) |
Comment on lines
+72
to
+76
| z.string().check( | ||
| z.minLength(1), | ||
| z.refine(value => { | ||
| return value.trim().length > 0 | ||
| }), |
|
|
||
| const scenario: Scenario = { | ||
| id: name, | ||
| id: config.id ?? getScenarioId(getRelativePath(scenariosDirectory, directory)), |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 526126fc-97a2-4353-93c1-807497ff8960
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 526126fc-97a2-4353-93c1-807497ff8960
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Saved experiment restoration can select a different resource, and report labels can leave distinct scenarios indistinguishable.
7 open issues
Review effort: Balanced
Comment on lines
+31
to
+33
| experiments.find(candidate => { | ||
| return getConfigPath(experimentsDirectory, candidate.filepath) === name | ||
| }) |
| ...values, | ||
| capability, | ||
| scenario: trial.scenario.id, | ||
| scenarioName: path.basename(trial.scenario.directory), |
This branch has not been deployed
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.
This PR adds optional IDs to benchmark, experiment, and scenario configs. Explicit IDs stay unchanged; omitted IDs depend on the resource's path relative to its configured directory.
Filename and directory selectors continue to work. Since default IDs change, the migration guide explains how to pin existing IDs to preserve plans and result history.
Changelog
New
idto benchmark, experiment, and scenario configs.Changed
Benchmark:<relative-path-without-extension>,Experiment:<relative-path-without-extension>, orScenario:<relative-directory>, using/separators.nameno longer overrides identity.Removed