auto-picker for the nvidia - #4
Conversation
- Fix ThemePicker import path (../ThemePicker.js -> ../theme/ThemePicker.js) - Add proper typing for useAppState callbacks to fix 'implicitly any' errors - Fix function declaration order by moving getModelLabel outside of JSX - Add nullish coalescing for potentially undefined state values
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces automatic building on startup in bin/claudex, updates several package dependencies, adds a new interactive step to select NVIDIA NIM models in the provider setup wizard, and refactors state selectors and imports in the settings configuration. The review feedback highlights a critical issue where the startup build check will crash in production because the src directory is missing, an authentication issue when fetching NVIDIA models because the API key is not yet set in the environment, and missing cursor offset state handling in the model search input.
| if (!existsSync(distPath) || newestSrcTime(srcDir) > statSync(distPath).mtimeMs) { | ||
| console.error('claudex: building...') | ||
| execSync('bun run build', { cwd: rootDir, stdio: 'inherit' }) | ||
| } |
There was a problem hiding this comment.
In the published npm package, the src directory is excluded (per files in package.json). As a result, srcDir will not exist on end-user machines. This causes newestSrcTime(srcDir) to throw an error and return Infinity, which always triggers the bun run build block on every startup, crashing the CLI for all production users. Adding a guard to check if srcDir exists before attempting to check for updates or rebuild fixes this.
if (existsSync(srcDir) && (!existsSync(distPath) || newestSrcTime(srcDir) > statSync(distPath).mtimeMs)) {
console.error('claudex: building...')
execSync('bun run build', { cwd: rootDir, stdio: 'inherit' })
}
| React.useEffect(() => { | ||
| let cancelled = false | ||
|
|
||
| void (async () => { | ||
| try { | ||
| const models = await listNvidiaModels(baseUrl ?? undefined) | ||
| if (!cancelled) { | ||
| if (models.length === 0) { | ||
| setStatus({ | ||
| state: 'error', | ||
| message: | ||
| 'No models found. Please check your API key and network connection.', | ||
| }) | ||
| } else { | ||
| setStatus({ | ||
| state: 'ready', | ||
| defaultValue: models[0], | ||
| options: models.map(model => ({ | ||
| label: model, | ||
| value: model, | ||
| description: 'NVIDIA NIM model', | ||
| })), | ||
| }) | ||
| } | ||
| } | ||
| } catch (error) { | ||
| if (!cancelled) { | ||
| setStatus({ | ||
| state: 'error', | ||
| message: | ||
| error instanceof Error | ||
| ? `Failed to fetch models: ${error.message}` | ||
| : 'Failed to fetch models', | ||
| }) | ||
| } | ||
| } | ||
| })() | ||
|
|
||
| return () => { | ||
| cancelled = true | ||
| } | ||
| }, [apiKey, baseUrl]) |
There was a problem hiding this comment.
The listNvidiaModels function fetches available models from the NVIDIA API but does not accept an apiKey parameter; instead, it internally reads from process.env.NVIDIA_API_KEY. Since process.env.NVIDIA_API_KEY is not yet set during this step of the wizard, the request is sent without authentication, causing the model fetch to fail for first-time setups. Temporarily setting process.env.NVIDIA_API_KEY during the fetch and restoring it afterwards resolves this issue.
React.useEffect(() => {
let cancelled = false
const originalApiKey = process.env.NVIDIA_API_KEY
void (async () => {
try {
if (apiKey) {
process.env.NVIDIA_API_KEY = apiKey
}
const models = await listNvidiaModels(baseUrl ?? undefined)
if (!cancelled) {
if (models.length === 0) {
setStatus({
state: 'error',
message:
'No models found. Please check your API key and network connection.',
})
} else {
setStatus({
state: 'ready',
defaultValue: models[0],
options: models.map(model => ({
label: model,
value: model,
description: 'NVIDIA NIM model',
})),
})
}
}
} catch (error) {
if (!cancelled) {
setStatus({
state: 'error',
message:
error instanceof Error
? `Failed to fetch models: ${error.message}`
: 'Failed to fetch models',
})
}
} finally {
if (originalApiKey === undefined) {
delete process.env.NVIDIA_API_KEY
} else {
process.env.NVIDIA_API_KEY = originalApiKey
}
}
})()
return () => {
cancelled = true
}
}, [apiKey, baseUrl])
| } | ||
| | { state: 'error'; message: string } | ||
| >({ state: 'loading' }) | ||
| const [searchTerm, setSearchTerm] = React.useState('') |
There was a problem hiding this comment.
Define a cursorOffset state to properly track and update the cursor position in the search input field.
const [searchTerm, setSearchTerm] = React.useState('')
const [cursorOffset, setCursorOffset] = React.useState(0)
| <TextInput | ||
| placeholder="Search models..." | ||
| value={searchTerm} | ||
| onChange={setSearchTerm} | ||
| columns={30} | ||
| cursorOffset={searchTerm.length} | ||
| onChangeCursorOffset={(offset) => { | ||
| // This is required by the TextInput component but we don't need to use it | ||
| }} | ||
| focus | ||
| /> |
There was a problem hiding this comment.
Pass the cursorOffset state and its setter to the TextInput component to enable proper cursor navigation and editing.
<TextInput
placeholder="Search models..."
value={searchTerm}
onChange={value => {
setSearchTerm(value)
setCursorOffset(value.length)
}}
columns={30}
cursorOffset={cursorOffset}
onChangeCursorOffset={setCursorOffset}
focus
/>
1 participant
the nvidia api shows the available models for the current api-key in the session