feat: bulk test channels, close #1219#1228
Conversation
![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 a batch health check feature for channels, enabling users to run concurrent test requests and recover healthy channels in bulk. The implementation includes a new ChannelsBulkTestDialog component, updates to the channel management table, and backend support for bulk recovery and flexible error parsing for OpenAI-compatible providers. Review feedback suggests optimizing React state updates by batching changes, improving the efficiency of the concurrent test queue by avoiding shift(), and simplifying the dialog's initialization logic by removing unnecessary state.
| failedChannels.forEach((channel) => { | ||
| setResultStatus(channel, 'idle', { error: undefined, latency: undefined }); | ||
| }); |
There was a problem hiding this comment.
Calling setResultStatus inside a loop triggers multiple state updates and re-renders. While React 18 batches updates in most cases, it is more efficient and cleaner to perform a single state update with the entire batch of changes for the failed channels.
| failedChannels.forEach((channel) => { | |
| setResultStatus(channel, 'idle', { error: undefined, latency: undefined }); | |
| }); | |
| setResults((prev) => { | |
| const next = { ...prev }; | |
| failedChannels.forEach((channel) => { | |
| next[channel.id] = { | |
| ...next[channel.id], | |
| channelID: channel.id, | |
| channelName: channel.name, | |
| status: 'idle', | |
| error: undefined, | |
| latency: undefined, | |
| }; | |
| }); | |
| return next; | |
| }); |
| const queue = [...runnableChannels]; | ||
| const workerCount = Math.min(MAX_CONCURRENT_TESTS, queue.length); | ||
|
|
||
| const workers = Array.from({ length: workerCount }, async () => { | ||
| while (queue.length > 0) { | ||
| const channel = queue.shift(); |
There was a problem hiding this comment.
Using Array.prototype.shift() inside a loop has O(N) complexity, making the overall process O(N^2) relative to the number of channels. While the number of channels is likely small, it is better practice to use a simple index pointer to track the next item in the queue.
const queue = [...runnableChannels];
const workerCount = Math.min(MAX_CONCURRENT_TESTS, queue.length);
let nextIndex = 0;
const workers = Array.from({ length: workerCount }, async () => {
while (nextIndex < queue.length) {
const channel = queue[nextIndex++];
| const { open, setOpen, selectedChannels, resetRowSelection, setSelectedChannels } = useChannels(); | ||
| const testChannel = useTestChannel({ silent: true }); | ||
| const bulkRecoverChannels = useBulkRecoverChannels(); | ||
| const [dialogContent, setDialogContent] = useState<HTMLDivElement | null>(null); |
There was a problem hiding this comment.
The dialogContent state and the associated ref on DialogContent appear to be unnecessary for initializing the results. The isDialogOpen state alone is sufficient to trigger the initialization effect when the dialog opens. Removing this state simplifies the component and avoids potential issues with ref-based effects.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
1 participant
Uh oh!
There was an error while loading. Please reload this page.