Conversation
WalkthroughThe buildOpenApiBinaries function in js/app/scripts/generate-api-schema.ts was restructured to implement batching behavior. A MAX_CONCURRENCY constant set to 6 was added. The function now accepts an options object containing checkMode. When check mode is enabled, crates are processed in sequential batches of up to six crates, each running a separate cargo build command. The conditional logic for including the document_cognition_service_models binary was moved from a single global check to per-batch evaluation. Logging was updated to report progress per batch during batching operations. The main() function was modified to pass the checkMode option when calling buildOpenApiBinaries. Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@js/app/scripts/generate-api-schema.ts`:
- Around line 183-184: The comment above the call to
buildOpenApiBinaries(crateNames, { checkMode }) is outdated: update it to say
that binaries are built in a single cargo invocation in normal mode but when
checkMode is true the builds are split into multiple batched cargo invocations;
reference crateNames and checkMode in the comment so readers know the behavior
depends on that flag and the call site is buildOpenApiBinaries.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 241e8957-59c7-4a42-8e25-a5f119c14d2a
📒 Files selected for processing (1)
js/app/scripts/generate-api-schema.ts
| // Phase 1: Build all binaries in a single cargo invocation (parallelized by cargo) | ||
| await buildOpenApiBinaries(crateNames); | ||
| await buildOpenApiBinaries(crateNames, { checkMode }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Update the comment to reflect the new batching behavior.
The comment states "Build all binaries in a single cargo invocation" but this is no longer accurate when checkMode is true—the build now runs in multiple batched invocations.
📝 Suggested comment update
- // Phase 1: Build all binaries in a single cargo invocation (parallelized by cargo)
+ // Phase 1: Build all binaries (batched in CI to limit resource usage, single invocation otherwise)
await buildOpenApiBinaries(crateNames, { checkMode });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Phase 1: Build all binaries in a single cargo invocation (parallelized by cargo) | |
| await buildOpenApiBinaries(crateNames); | |
| await buildOpenApiBinaries(crateNames, { checkMode }); | |
| // Phase 1: Build all binaries (batched in CI to limit resource usage, single invocation otherwise) | |
| await buildOpenApiBinaries(crateNames, { checkMode }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@js/app/scripts/generate-api-schema.ts` around lines 183 - 184, The comment
above the call to buildOpenApiBinaries(crateNames, { checkMode }) is outdated:
update it to say that binaries are built in a single cargo invocation in normal
mode but when checkMode is true the builds are split into multiple batched cargo
invocations; reference crateNames and checkMode in the comment so readers know
the behavior depends on that flag and the call site is buildOpenApiBinaries.
Building too many services at once was causing CI to get stuck (I think). Going to try this to see if it fixes