⚡ Bolt: Parallelize OpenAI model surface discovery#27
Conversation
Replaces sequential iteration with concurrent request fetching in collectZimaOSV1ModelEntries while preserving state side-effect sequences. Co-authored-by: bobdivx <6737167+bobdivx@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![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 optimizes the collectZimaOSV1ModelEntries function by parallelizing calls to fetchZimaOSJson using Promise.all to reduce latency. Feedback suggests addressing the redundant execution of configuration discovery logic caused by concurrent calls and refining the state update logic to prevent inconsistent metadata when the final request in the sequence fails.
| const responses = await Promise.all( | ||
| V1_MODELS_PATHS.map((path) => fetchZimaOSJson(email, path)) | ||
| ); |
There was a problem hiding this comment.
Parallelizing the calls to fetchZimaOSJson results in redundant execution of the configuration discovery logic (token retrieval, gateway candidate discovery, Docker socket checks, and file system probes) for each path. While the network requests are correctly parallelized, these local setup operations are now performed multiple times concurrently. Consider refactoring the gateway client to allow passing pre-resolved configuration or implementing a cache for these discovery operations to avoid unnecessary I/O and CPU overhead.
| V1_MODELS_PATHS.map((path) => fetchZimaOSJson(email, path)) | ||
| ); | ||
|
|
||
| for (const r of responses) { |
There was a problem hiding this comment.
By iterating through the responses in order, the code correctly mimics the original sequential behavior where lastStatus, lastError, and lastRaw are updated by each path. However, this means the final return values will always reflect the result of the last path in V1_MODELS_PATHS, even if it failed (e.g., 404) and an earlier path succeeded. This can lead to inconsistent metadata where entries is populated but lastStatus indicates an error. Consider refining this logic to prioritize metadata from successful responses.
1 participant
💡 What
Replaced a sequential
for...ofloop incollectZimaOSV1ModelEntrieswith a concurrentPromise.allapproach to fetch model entries from multiple paths simultaneously, then iterating the resolved responses array in-order to populate the Map and assign state variables.🎯 Why
Previously,
V1_MODELS_PATHSrequests were executed synchronously. This forced the application to wait for one path to resolve (or timeout) before starting the request for the next path, inflating the total discovery latency linearly with the number of target endpoints.📊 Impact
Reduces the overall latency of the
/v1/modelsprobe to the duration of the single slowest request rather than the sum of all requests, cutting discovery time by ~50% when both paths are active.🔬 Measurement
No breaking changes. The unit test suite and linters passed seamlessly. The optimization correctly updates state variables like
lastStatusandlastRawmimicking the original code's behavior due to deterministic array iteration ordering.PR created automatically by Jules for task 14171873267222267122 started by @bobdivx