Skip to content

feat(UI): Device selector#839

Open
ActiveChooN wants to merge 3 commits intomainfrom
dkalinin/device-selector
Open

feat(UI): Device selector#839
ActiveChooN wants to merge 3 commits intomainfrom
dkalinin/device-selector

Conversation

@ActiveChooN
Copy link
Copy Markdown
Contributor

@ActiveChooN ActiveChooN commented Mar 12, 2026

Pull Request

Description

Added device selector
Closes #837

Type of Change

  • feat - New feature
  • 🐞 fix - Bug fix
  • 📚 docs - Documentation
  • ♻️ refactor - Code refactoring
  • 🧪 test - Tests
  • 🔧 chore - Maintenance

Related Issues

Breaking Changes


Examples

Screenshots

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 12, 2026

📊 Test coverage report

Metric Coverage
Lines 48.8%
Functions 58.1%
Branches 89.2%
Statements 48.8%

@ActiveChooN ActiveChooN marked this pull request as ready for review March 31, 2026 08:20
Copilot AI review requested due to automatic review settings March 31, 2026 08:20
},
{
onSuccess: async () => {
await queryClient.invalidateQueries({
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of invalidating queries here in onSuccess we could also do it in mutation's meta property (the only benefit would be that queryClient and getQueryKey would not be necessary).

* SPDX-License-Identifier: Apache-2.0
*/

export interface DeviceOption {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

most probably that type will come from @/api once server supprts it.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new Device selector UI feature to the React frontend and places it in the main toolbar, intended to let users choose an inference accelerator at the project level (Issue #837).

Changes:

  • Introduces a DeviceSelector component and supporting hook/types under features/device-selector/.
  • Adds a project update mutation hook intended to persist the selected device via PUT /api/v1/projects/{project_id}.
  • Renders the selector in the toolbar next to the existing Sources/Sinks controls.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
application/ui/src/features/device-selector/device-selector.types.ts Adds DeviceOption type for picker items.
application/ui/src/features/device-selector/device-selector.component.tsx Adds the DeviceSelector UI (Suspense + Picker).
application/ui/src/features/device-selector/api/use-update-project-device.ts Adds mutation hook for persisting device selection + invalidation.
application/ui/src/features/device-selector/api/use-get-devices.ts Adds a devices hook (currently mock data).
application/ui/src/components/toolbar/toolbar.component.tsx Places the selector into the toolbar layout.

Comment on lines +27 to +33
const { data: _project } = useCurrentProject();
const updateDevice = useUpdateProjectDevice();

const items: DeviceOption[] = [AUTO_DEVICE, ...devices];

// TODO: Replace with actual project device when API is ready
const selectedDevice = 'auto';
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

useCurrentProject() result is currently unused, and selectedDevice is hard-coded to 'auto' while Picker is controlled via selectedKey. As a result, the picker selection will never visually change (it will always show Auto) and it won’t reflect the project’s persisted device value. Derive the selected key from project.device (fallback to auto) and/or keep local state that updates on selection / mutation success, and remove the unused _project alias if not needed.

Suggested change
const { data: _project } = useCurrentProject();
const updateDevice = useUpdateProjectDevice();
const items: DeviceOption[] = [AUTO_DEVICE, ...devices];
// TODO: Replace with actual project device when API is ready
const selectedDevice = 'auto';
const { data: project } = useCurrentProject();
const updateDevice = useUpdateProjectDevice();
const items: DeviceOption[] = [AUTO_DEVICE, ...devices];
const selectedDevice = project?.device ?? 'auto';

Copilot uses AI. Check for mistakes.
Comment on lines +8 to +16
// TODO: Replace with $api.useSuspenseQuery('get', '/api/v1/system/devices', ...) once the endpoint is available.
const MOCK_DEVICES: DeviceOption[] = [
{ id: 'cpu', name: 'CPU' },
{ id: 'xpu', name: 'Intel GPU' },
{ id: 'cuda', name: 'NVIDIA GPU' },
];

export const useGetDevices = (): DeviceOption[] => {
return MOCK_DEVICES;
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This hook currently returns a hard-coded MOCK_DEVICES list. The issue/PR acceptance criteria requires fetching available devices from GET /system/devices and always including an Auto option, so this should be implemented against the real endpoint (and the UI should handle loading/error states accordingly) rather than shipping a mock.

Copilot uses AI. Check for mistakes.
Comment on lines +15 to +28
// TODO: Replace with proper data type once the API is ready.
const mutation = $api.useMutation('put', '/api/v1/projects/{project_id}', {
meta: {
error: { notify: true },
},
});

const updateDevice = (device: string): void => {
mutation.mutate(
{
// eslint-disable-next-line @typescript-eslint/no-explicit-any
body: { device } as any,
params: { path: { project_id: projectId } },
},
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

body: { device } as any defeats the OpenAPI typing and can mask mismatches with the backend contract. Since the backend PUT /projects/{project_id} update schema supports device, prefer reusing the existing typed useUpdateProject hook (or use ProjectUpdateType directly) so the payload is type-checked and you don’t need an any cast.

Copilot uses AI. Check for mistakes.
Comment on lines +17 to +41
export const DeviceSelector = () => {
return (
<Suspense fallback={<Loading size={'S'} />}>
<DeviceSelectorContent />
</Suspense>
);
};

const DeviceSelectorContent = () => {
const devices = useGetDevices();
const { data: _project } = useCurrentProject();
const updateDevice = useUpdateProjectDevice();

const items: DeviceOption[] = [AUTO_DEVICE, ...devices];

// TODO: Replace with actual project device when API is ready
const selectedDevice = 'auto';

const handleSelectionChange = (key: Key | null): void => {
const next = String(key);

if (key !== null && next !== selectedDevice) {
updateDevice.mutate(next);
}
};
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are existing UI tests for similar toolbar pickers, but this new selector has no tests covering its core behavior (rendering Auto + device options, reflecting project.device, and issuing the PUT update + invalidating the project query on change). Adding a device-selector.component.test.tsx (MSW-mocked) would prevent regressions.

Copilot generated this review using guidance from repository custom instructions.
Comment on lines +44 to +50
<Picker
label={'Device'}
aria-label={'device'}
labelPosition={'side'}
labelAlign={'end'}
selectedKey={selectedDevice}
onSelectionChange={handleSelectionChange}
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aria-label is currently set to a generic lowercase 'device'. For better screen reader output and consistency with other labels in this codebase, use a more descriptive label (e.g., "Device" or "Device selector"), or rely on the visible label prop if Picker already wires it up correctly.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Task]: Add device selector

3 participants