feat: add checkout selectors on hardware page#1904
Conversation
db1a452 to
d6051f2
Compare
e62fd2c to
4cb4128
Compare
4cb4128 to
8313347
Compare
There was a problem hiding this comment.
Pull request overview
Adds a hardware “revision selector” flow so the hardware listing can be filtered by a specific tree/repo/branch/commit, wiring new backend endpoints to a new selector UI and persisting the selection in URL search params.
Changes:
- Introduces backend APIs for (1) available hardware selectors (tree/branch/revision) and (2) hardware listing filtered by a selected revision.
- Adds frontend selection state/resolution utilities plus a new selector UI (comboboxes) integrated into the hardware table header.
- Extends router search schemas + querystring minification to persist tree/repo/branch/commit selection in the URL.
Reviewed changes
Copilot reviewed 22 out of 23 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| dashboard/src/utils/search.ts | Adds minified query param keys for revision selection fields. |
| dashboard/src/types/hardware.ts | Adds selector/selection TypeScript types for the new APIs and UI state. |
| dashboard/src/types/general.ts | Extends SearchParamsKeys with selection-related search params. |
| dashboard/src/routes/_main/hardware/v2/route.tsx | Allows revision selection fields in the v2 hardware route search schema. |
| dashboard/src/routes/_main/hardware/route.tsx | Allows revision selection fields in the v1 hardware route search schema. |
| dashboard/src/pages/Hardware/hardwareTableUtils.ts | Extracts hardware-details link search-building logic (dropping selection params). |
| dashboard/src/pages/Hardware/HardwareTable.tsx | Integrates selector UI into table header and supports a configurable empty-state message id. |
| dashboard/src/pages/Hardware/hardwareSelection.ts | Adds selection encode/decode + “resolve latest valid selection” helper logic. |
| dashboard/src/pages/Hardware/HardwareRevisionSelectors.tsx | New tree/branch/revision combobox selector UI component. |
| dashboard/src/pages/Hardware/HardwareListingPageV2.tsx | Switches v2 listing to selector-driven fetching and URL-updating handlers. |
| dashboard/src/locales/messages/index.ts | Adds i18n strings for selector labels and revision-specific empty/reset messaging. |
| dashboard/src/components/ui/popover.tsx | Adds Radix popover UI primitive wrapper used by the combobox UI. |
| dashboard/src/components/ui/command.tsx | Adds cmdk-based command UI primitive wrapper used by the combobox UI. |
| dashboard/src/api/hardware.ts | Adds hooks to fetch selectors and fetch listing-by-revision. |
| dashboard/pnpm-lock.yaml | Locks newly added @radix-ui/react-popover and cmdk dependencies. |
| dashboard/package.json | Adds @radix-ui/react-popover and cmdk dependencies. |
| backend/kernelCI_app/views/hardwareSelectorsView.py | New selectors endpoint view (sanitizes raw DB rows into response model). |
| backend/kernelCI_app/views/hardwareByRevisionView.py | New listing-by-revision endpoint view. |
| backend/kernelCI_app/urls.py | Adds new API routes and extends cache wrapper to accept per-route timeout. |
| backend/kernelCI_app/typeModels/hardwareSelectors.py | Pydantic models for selectors endpoint params/response. |
| backend/kernelCI_app/typeModels/hardwareListingByRevision.py | Pydantic models for listing-by-revision endpoint params. |
| backend/kernelCI_app/tests/unitTests/url_patterns_test.py | Adds URL pattern test for the new selectors route. |
| backend/kernelCI_app/queries/hardware.py | Adds SQL queries for selectors and listing-by-revision. |
Files not reviewed (1)
- dashboard/pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
6595cd4 to
ce60bfc
Compare
0a2a1e2 to
06beef9
Compare
|
|
||
| type HardwareListingRoutes = '/hardware' | '/hardware/v1' | '/hardware/v2'; | ||
|
|
||
| const noop = (): void => {}; |
There was a problem hiding this comment.
I am usually for the null object pattern, but isnt more common in javascript we use an undefined for "nothing done here"?
| issueVersion: 'iv', | ||
| logOpen: 'l', | ||
| treeName: 't', | ||
| gitRepositoryUrl: 'hgu', |
There was a problem hiding this comment.
nit: why 'h' for prefixing the minified parameters?
There was a problem hiding this comment.
I think there was some conflict happening before, but we can remove this
| def get_hardware_selectors(origin: str) -> list[dict]: | ||
| params = {"origin": origin} | ||
|
|
||
| query = """ |
There was a problem hiding this comment.
since we are already joining with builds and tests, cant we bring only the revisions associated with the specified hardware we are tracking?
There was a problem hiding this comment.
I believe this might help as well, pre select only a subset of builds.
WITH hwbuilds AS (
SELECT DISTINCT
builds.id,
builds.checkout_id,
tests.environment_misc->>'platform' as platform
FROM tests
INNER JOIN builds ON builds.id = tests.build_id
WHERE tests.origin = 'maestro'
AND tests.start_time > (NOW() - INTERVAL '5 days')
)
SELECT * FROM checkouts
INNER JOIN hwbuilds ON hwbuilds.checkout_id = checkouts.id;
There was a problem hiding this comment.
Sorry, forgot to retrieve all commits, we need a full scan on checkouts at least.
WITH hwbuilds AS (
SELECT DISTINCT
builds.id,
builds.checkout_id,
tests.environment_misc->>'platform' as platform
FROM tests
INNER JOIN builds ON builds.id = tests.build_id
WHERE tests.origin = 'maestro'
AND tests.start_time > (NOW() - INTERVAL '5 days')
),
hwtrees as (select hwbuilds.platform, checkouts.git_repository_url, checkouts.tree_name FROM checkouts
INNER JOIN hwbuilds ON hwbuilds.checkout_id = checkouts.id)
select distinct hwtrees.platform, checkouts.git_repository_url, checkouts.tree_name, checkouts.git_repository_branch, checkouts.git_commit_hash
from checkouts inner join hwtrees on hwtrees.tree_name = checkouts.tree_name and hwtrees.git_repository_url = checkouts.git_repository_url;
There was a problem hiding this comment.
It is just a rough idea what what we could do, it can certainly be improved.
There was a problem hiding this comment.
But this way we would be limiting by builds tested within the last 5 days, no?
There was a problem hiding this comment.
I am actually selecting hardware tested in the last 5 days (something we are already doing in the listing). From those, we are selecting all trees related with those tests, and all commits from those trees.
There was a problem hiding this comment.
Yeah, I was thinking about scenarios where some origins don't have tests for 5 days. In that case, the page could end up looking strangely empty, since we would still have old commits to show. But it's still a nice workaround.
There was a problem hiding this comment.
Got it. I was under the impression that we were already limited by the trees with checkouts in the last few days.
There was a problem hiding this comment.
I got a slighlty better query with gemini, going roughly from 36s -> 23s. Mine had some unecessary steps.
SELECT DISTINCT
c.git_repository_url,
c.tree_name,
c.git_repository_branch,
c.git_commit_hash,
c.git_commit_name,
c.start_time
FROM checkouts c
INNER JOIN (
-- Step 1: Find ONLY unique active stream keys first
SELECT
c2.tree_name,
c2.git_repository_url
FROM tests t
INNER JOIN builds b ON b.id = t.build_id
INNER JOIN checkouts c2 ON b.checkout_id = c2.id
WHERE t.origin = 'maestro'
AND t.start_time > (NOW() - INTERVAL '5 days')
AND (t.environment_misc -> 'platform') IS NOT NULL
GROUP BY c2.tree_name, c2.git_repository_url
) active_streams
ON c.tree_name = active_streams.tree_name
AND c.git_repository_url = active_streams.git_repository_url;
| export const useHardwareListingV2 = ( | ||
| startTimestampInSeconds: number, | ||
| endTimestampInSeconds: number, | ||
| export const useHardwareSelectors = ( |
There was a problem hiding this comment.
Is it feasible to make this call async?
This way the page could load and it might reduce the perceived latency on the more expensive endpoint.
There was a problem hiding this comment.
We can't, cause the table below depends on this selector to be ready before fetching data
29bf1f7 to
c91c0e1
Compare
…date hardware-by-revision path cache duration
a40ae80 to
46930cc
Compare
| return cursor.fetchall() | ||
|
|
||
|
|
||
| def get_hardware_selectors(origin: str) -> list[dict]: |
There was a problem hiding this comment.
we should include a redis cache
| selectors_raw = get_hardware_selectors(origin=query_params.origin) | ||
|
|
||
| try: | ||
| result = self._sanitize_records(selectors_raw=selectors_raw) |
There was a problem hiding this comment.
I am getting some sanitization error for hardware listing
pydantic_core._pydantic_core.ValidationError: 1 validation error for HardwareSelectorBranch
git_repository_branch
Input should be a valid string [type=string_type, input_value=None, input_type=NoneType]
For further information visit https://errors.pydantic.dev/2.10/v/string_type
Allows the user to select a specific tree/branch/commit on the hardware page to filter data
Closes #1901