feat: add support in the UX for public benchmarks#150
Open
Conversation
rename strings to avoid shortcut key conflict fix: change benchmark job shortcut to [r] "Run Benchmark Job" The previous shortcut "c" conflicted with "copy id" on the detail page, and "n" conflicted with "next page" on the list page. Use "r" for "Run Benchmark Job" on both screens for consistency. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> fix pagenation and loading of benchmark scenarios fmt
james-rl
requested changes
Mar 6, 2026
Contributor
james-rl
left a comment
james-rl
left a comment
There was a problem hiding this comment.
I think we're still going with custom benchmarks rather than private
A few other minor suggestions
Comment on lines
+207
to
+218
| const BENCHMARK_ID_PREFIXES = ["bm_", "bmk_", "bmd_"]; | ||
|
|
||
| function looksLikeBenchmarkId(s: string): boolean { | ||
| return BENCHMARK_ID_PREFIXES.some((p) => s.startsWith(p)); | ||
| } | ||
|
|
||
| // Extract a benchmark ID from strings like "Name (bmd_xxx)" copied from the TUI | ||
| function extractEmbeddedId(s: string): string | null { | ||
| const match = s.match(/\((bm[dk]?_\S+)\)\s*$/); | ||
| return match ? match[1] : null; | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
they always start with bmd_
message BenchmarkDefinitionPb {
option (db_schema_options) = {
db_table_name: "benchmark_definition"
db_table_schema_type: DB_TABLE_SCHEMA
db_type_prefix: "bmd"
};| const [showPopup, setShowPopup] = React.useState(false); | ||
| const [selectedOperation, setSelectedOperation] = React.useState(0); | ||
| const [tab, setTab] = React.useState<"private" | "public">( | ||
| params.tab === "public" ? "public" : "private", |
Contributor
There was a problem hiding this comment.
private should be custom to match our branding for non-public benchmarks
| }); | ||
| } else if (input === "/") { | ||
| search.enterSearchMode(); | ||
| } else if (input === "1" && tab !== "private") { |
Comment on lines
+142
to
+146
| const result = await listScenarioRuns({ | ||
| limit: 100, | ||
| startingAfter, | ||
| benchmarkRunId, | ||
| }); |
Contributor
There was a problem hiding this comment.
this limit seems very low; maybe somewhere in [500, 1k] instead?
| describe("updateCurrentParams", () => { | ||
| it("merges new params into current params without changing screen or history", () => { | ||
| let state = navigate(initialNavigationState, "benchmark-list" as ScreenName, { | ||
| benchmarkId: "bm_1", |
james-rl
reviewed
Mar 6, 2026
| color={!isPublic ? colors.primary : colors.textDim} | ||
| bold={!isPublic} | ||
| > | ||
| [1] Private |
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
2 participants
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.
The main part of this PR adds the ability for RLI to pull from public benchmarks as well as private ones, and hooks these into the various parts of the interactive and command-line UX. There are also a number of fit-and-polish fixes along the way: