Conversation
|
@Th-Shivam is attempting to deploy a commit to the Superagent Team on Vercel. A member of the Team first needs to authorize it. |
Contributor License AgreementAll contributors are covered by a CLA. |
|
🚨 Contributor flagged. Click here for more info: Superagent Dashboard |
There was a problem hiding this comment.
🟡 Changes recommended
The Python concurrency helper still schedules one task per item via asyncio.gather, which can keep large numbers of pending tasks in memory and undermine the intended backpressure for very large inputs.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds optional per-call concurrency limits to guard() in both the TypeScript and Python SDKs to prevent unbounded fan-out when analyzing many text chunks or PDF pages, while preserving existing ordering and aggregation behavior when the option is omitted.
Changes:
- TypeScript: add
maxConcurrencytoGuardOptions, validate it, and execute chunk/page analysis via a bounded worker pool. - Python: add
max_concurrencytoGuardOptionsandguard(), validate it, and apply concurrency limiting for chunk/page analysis. - Add targeted tests in both SDKs covering concurrency limiting, ordering, and invalid option values.
File summaries
| File | Description |
|---|---|
| sdk/typescript/tests/guard.test.ts | Adds tests for chunk concurrency limiting, ordering, and invalid maxConcurrency. |
| sdk/typescript/tests/guard-file.test.ts | Adds test for PDF-page concurrency limiting. |
| sdk/typescript/src/types.ts | Extends GuardOptions with maxConcurrency. |
| sdk/typescript/src/client.ts | Implements bounded concurrency mapping and wires maxConcurrency into chunk/PDF execution. |
| sdk/python/tests/test_guard.py | Adds tests for chunk/PDF concurrency limiting, ordering, and invalid max_concurrency. |
| sdk/python/src/safety_agent/types.py | Extends GuardOptions with max_concurrency. |
| sdk/python/src/safety_agent/client.py | Adds _map_with_concurrency, validates max_concurrency, and uses it for chunk/PDF analysis. |
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| semaphore = asyncio.Semaphore(max_concurrency or len(items)) | ||
|
|
||
| async def run(item: T) -> R: | ||
| async with semaphore: | ||
| return await mapper(item) | ||
|
|
||
| return list(await asyncio.gather(*(run(item) for item in items))) |
|
@Th-Shivam thanks for PR will have a look at it later today/tomorrow. |
|
@homanp Thanks, really appreciate it! Looking forward to your feedback. Happy to make any changes needed based on your review. |
|
@homanp Hello Sir, I hope you are doing well , I just wanted to have a quick follow up, is there any update on this pr ? |
|
Hello @homanp sir , I hope you are doing well , Is there any update or any changes required on this pr ? |
Description
Closes #1161.
guard()previously used unboundedPromise.all()in the TypeScript SDK and unboundedasyncio.gather()in the Python SDK for text chunks and PDF pages. This adds optional per-call concurrency controls:maxConcurrencyfor TypeScript andmax_concurrencyfor Python.Both SDKs now support bounded execution for chunk and PDF-page analysis while preserving TypeScript/Python parity, input/result ordering, existing aggregation semantics, token usage accounting, and error behavior. Omitting the option preserves the existing legacy full-fan-out behavior. No default concurrency value was introduced.
Type of Change
Testing
npm run buildpassedcompileallpassedgit diff --checkpassedThe Python suite reported the existing Daytona SDK deprecation warning.
Checklist
Note
Low Risk
Backward-compatible optional API on guard parallelization only; classification semantics unchanged and covered by new unit tests.
Overview
Adds optional concurrency caps to
guard()in the Python and TypeScript SDKs so large text chunking and multi-page PDF analysis no longer always fan out with unbounded parallelism.Callers can pass
max_concurrency(Python) ormaxConcurrency(TypeScript) onguard()/GuardOptions. When set, parallel_guard_single_text/guardSingleTextwork is routed through new helpers (_map_with_concurrency/mapWithConcurrency) instead ofasyncio.gather/Promise.all. Omitting the option keeps the previous behavior (all chunks/pages at once). Invalid values (non–positive integers, including booleans) raise before any provider calls.Aggregation (OR block logic), token usage totals, and result ordering are unchanged; tests assert peak in-flight calls, ordering of aggregated reasoning, and PDF page paths.
Reviewed by Cursor Bugbot for commit a77785a. Configure here.