Conversation
There was a problem hiding this comment.
Pull request overview
This PR refines benchmark experiment sorting by centralizing comparison logic in AutoBeReplayComputer, improving vendor/model ordering semantics, and adding targeted tests for vendor comparison behavior.
Changes:
- Added
AutoBeReplayComputer.compareandcompareVendorsto sort by aggregate score (desc) then vendor/model. - Updated benchmark publishing script to use the shared comparator.
- Added a new test suite covering vendor/model comparison ordering rules.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/benchmark/src/replay/AutoBeReplayComputer.ts | Introduces shared comparators for benchmark sorting and a helper for parsing numeric parts. |
| test/src/archive/publish.ts | Replaces inline sort logic with AutoBeReplayComputer.compare for consistency. |
| test/src/features/benchmark/test_benchmark_compare_vendors.ts | Adds tests validating vendor/model sorting rules (params, variants, vendor ordering, numeric versions). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export const compareVendors = (a: string, b: string): number => { | ||
| const pa = a.split("-"); | ||
| const pb = b.split("-"); | ||
| const len = Math.max(pa.length, pb.length); |
There was a problem hiding this comment.
compareVendors splits the full vendor/model slug on -, which also splits vendor names that contain hyphens (e.g. z-ai/...) and mixes vendor/model into the same token stream. This can produce incorrect vendor-level alphabetical ordering and inconsistent comparisons. Consider splitting on / first (compare vendor segment(s) separately), then apply - tokenization only to the model portion.
| const n = Number(t); | ||
| return isNaN(n) ? null : n; |
There was a problem hiding this comment.
parsePart only recognizes numeric parts that are plain numbers or ...b/a...b. Real vendor/model slugs in the repo include patterns like moonshotai/kimi-k2.5 and minimax/minimax-m2.7, where the version token has a leading letter (k2.5, m2.7). Those won’t be parsed as numbers, so compareVendors falls back to lexicographic (ascending) ordering for versions, which contradicts the intended “higher versions first” behavior. Extend parsing to handle a leading alphabetic prefix + numeric version and add coverage for these cases.
| const n = Number(t); | |
| return isNaN(n) ? null : n; | |
| const direct: number = Number(t); | |
| if (!isNaN(direct)) return direct; | |
| const prefixed: RegExpMatchArray | null = t.match( | |
| /^([A-Za-z]+)(\d+(?:\.\d+)?)$/, | |
| ); | |
| if (prefixed !== null) { | |
| const numeric: number = Number(prefixed[2]); | |
| return isNaN(numeric) ? null : numeric; | |
| } | |
| return null; |
This pull request introduces improvements to the sorting logic for benchmark experiments, specifically focusing on how vendor/model strings are compared and ordered. It refactors the comparison logic into reusable functions, adds comprehensive tests for vendor comparison, and updates the main sorting call to use the new logic.
Sorting and Comparison Logic Improvements:
comparefunction toAutoBeReplayComputerthat sorts benchmark results first by aggregate score (descending), then by vendor/model string using the newcompareVendorsfunction. (packages/benchmark/src/replay/AutoBeReplayComputer.tspackages/benchmark/src/replay/AutoBeReplayComputer.tsL19-R45)compareVendorsfunction, which parses vendor/model strings to compare model families, parameter sizes, and versions in a meaningful order (e.g., larger parameter sizes and higher versions come first; vendors are sorted alphabetically otherwise). Also added a helperparsePartfunction for parsing string segments as numbers. (packages/benchmark/src/replay/AutoBeReplayComputer.ts[1] [2]AutoBeReplayComputer.comparefunction for sorting experiments, ensuring consistent and correct ordering. (test/src/archive/publish.tstest/src/archive/publish.tsL47-R47)Testing Enhancements:
compareVendors, covering various real-world and edge cases to ensure correct sorting by parameter size, vendor, version, and model variant. (test/src/features/benchmark/test_benchmark_compare_vendors.tstest/src/features/benchmark/test_benchmark_compare_vendors.tsR1-R95)Interface Update:
IAutoBePlaygroundBenchmarkinterface where needed to support the new comparison logic. (packages/benchmark/src/replay/AutoBeReplayComputer.tspackages/benchmark/src/replay/AutoBeReplayComputer.tsR5)