feat(action): ⚡️ Add parallel batch and dimension execution#8
Conversation
AI 代码审查汇总PR: #8 (feat(action): ⚡️ Add parallel batch and dimension execution) 总体评价共发现 7 条可执行问题，建议优先处理 CRITICAL/HIGH。 主要问题（按严重级别）
可执行建议
潜在风险
测试建议
文件级覆盖说明
无法 inline 的已处理项
覆盖状态
未覆盖文件清单:
无 patch 文件覆盖清单:
轮次与预算
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -171,6 +171,7 @@ function loadConfig() { | |||
| coverageFirstRoundPrimaryOnly: parseBooleanInput('coverage_first_round_primary_only', true), | |||
There was a problem hiding this comment.
自动审查已完成；聚合后没有可稳定定位的具体 inline 问题。
| .map((path) => pendingFileByPath.get(path)) | ||
| .filter(Boolean); | ||
| // Pre-allocate budget: each batch gets at most roundDimensions.length calls | ||
| const estimatedCallsPerBatch = roundDimensions.length; |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
|
||
| // Phase 2: run remaining dimensions in parallel via semaphore | ||
| if (parallelDimensions.length > 0) { | ||
| const dimPromises = parallelDimensions.map((dimension) => |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| */ | ||
| async function executeBatch({ | ||
| batch, batchIndex, batchCount, round, roundDimensions, | ||
| pendingFileByPath, reviewerAgents, config, projectGuidance, semaphore |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| const semaphore = createSemaphore(config.maxConcurrency); | ||
| const batchCount = plannedBatches.length; | ||
|
|
||
| // Pre-allocate budget: each batch may use up to roundDimensions.length calls, |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| @@ -171,6 +171,7 @@ function loadConfig() { | |||
| coverageFirstRoundPrimaryOnly: parseBooleanInput('coverage_first_round_primary_only', true), | |||
There was a problem hiding this comment.
自动审查已完成；聚合后没有可稳定定位的具体 inline 问题。
Add configurable concurrency limiting for sub-agent and batch processing via a new `max_concurrency` input. - Introduce `createSemaphore` to cap concurrent async review runs - Refactor batch execution to run dimension reviews in parallel (with general dimension executed first) - Add `max_concurrency` parsing and validation to config - Update tests for default, custom, invalid `max_concurrency`, and semaphore behavior
jorben
force-pushed
the
feat/parallel-support
branch
from
c0395f6 to
163b140
Compare
| * Returns { run(asyncFn) → Promise } that ensures at most `concurrency` | ||
| * async functions execute simultaneously. | ||
| */ | ||
| function createSemaphore(concurrency) { |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| * general dimension runs first (to collect recommendedExtraDimensions), | ||
| * then remaining dimensions run in parallel via the shared semaphore. | ||
| */ | ||
| async function executeBatch({ |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
|
||
| // Pre-allocate budget: each batch may use up to roundDimensions.length calls, | ||
| // with a 2x multiplier to account for potential structured-output repair retries. | ||
| const estimatedCallsPerBatch = roundDimensions.length * 2; |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| executionDimensions.push(roundDimensions[0]); | ||
| } | ||
| const batchPromises = batchesToRun.map((batch, batchIndex) => | ||
| semaphore.run(() => executeBatch({ |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| // --- Merge batch results into global state --- | ||
| for (let i = 0; i < batchSettled.length; i += 1) { | ||
| const outcome = batchSettled[i]; | ||
| if (outcome.status === 'rejected') { |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| coverageFirstRoundPrimaryOnly: parseBooleanInput('coverage_first_round_primary_only', true), | ||
| autoMinimizeOutdatedComments: parseBooleanInput('auto_minimize_outdated_comments', true), | ||
| maxRounds: parsePositiveIntInput('max_rounds', 8), | ||
| maxConcurrency: parsePositiveIntInput('max_concurrency', 4), |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| * Returns { run(asyncFn) → Promise } that ensures at most `concurrency` | ||
| * async functions execute simultaneously. | ||
| */ | ||
| function createSemaphore(concurrency) { |
There was a problem hiding this comment.
[HIGH] createSemaphore 信号量缺少单元测试覆盖
新增的 createSemaphore 函数是一个关键的并发控制原语,已导出供测试使用,但未发现对应的单元测试文件变更。信号量逻辑包含队列管理、异步执行顺序、错误传播等复杂场景,需要充分测试。
建议: 在 test/index.test.js 中添加 createSemaphore 的专项测试:并发限制生效、队列顺序保持、错误正确传播、边界值(concurrency=0,1)、连续调用稳定性。
风险: 未经测试的并发控制可能导致竞态条件、死锁或资源泄漏,在高并发生产环境中难以排查。
置信度: 0.95
| * general dimension runs first (to collect recommendedExtraDimensions), | ||
| * then remaining dimensions run in parallel via the shared semaphore. | ||
| */ | ||
| async function executeBatch({ |
There was a problem hiding this comment.
[MEDIUM] executeBatch 并行执行逻辑缺少集成测试
executeBatch 重构了批量执行逻辑,包含 general 维度先行、其余维度并行、信号量控制等新特性,但未发现相应的测试覆盖。
建议: 添加 executeBatch 的单元测试:模拟 reviewerAgents 返回不同结果、验证 general 维度优先执行、验证并行维度正确并发、验证结果正确聚合。
风险: 复杂的并行执行逻辑未经测试可能导致维度执行顺序错误、结果丢失或状态不一致。
置信度: 0.85
| const estimatedCallsPerBatch = roundDimensions.length * 2; | ||
| let budgetAllowedBatches = batchCount; | ||
| if (estimatedCallsPerBatch > 0) { | ||
| const budgetRemaining = config.maxModelCalls - modelCalls; |
There was a problem hiding this comment.
[MEDIUM] 预算分配时未考虑当前轮次已消耗的调用数
在并行执行批次前计算剩余预算时，modelCalls 尚未包含当前正在执行的批次的调用数，因为 Promise.allSettled 是在计算预算之后才等待结果。这意味着在并行场景下，如果多个批次同时执行，它们各自的调用数不会被计入预算检查，可能导致总调用数超过 maxModelCalls 限制。
建议: 考虑在并行执行前为每个批次预留预算，或在批次执行完成后动态检查总调用数是否超限。可以在 executeBatch 内部检查 modelCalls 并在超限时提前返回。
风险: 可能导致模型调用总次数超出配置的 maxModelCalls 限制，增加成本或触发 API 限流。
置信度: 0.85
|
|
||
| // Pre-allocate budget: each batch may use up to roundDimensions.length calls, | ||
| // with a 2x multiplier to account for potential structured-output repair retries. | ||
| const estimatedCallsPerBatch = roundDimensions.length * 2; |
There was a problem hiding this comment.
[MEDIUM] 预算预分配逻辑缺少边界测试
预算预分配使用 2x 乘数因子估算每批次调用次数,该逻辑影响并行批次数量的计算,但缺少边界条件测试。
建议: 添加测试用例:roundDimensions 为空、预算刚好不够一个批次、预算足够部分批次、预算充足全部批次等场景。
风险: 边界条件处理不当可能导致预算耗尽时仍启动批次,或过于保守导致利用率不足。
置信度: 0.80
| * Returns { run(asyncFn) → Promise } that ensures at most `concurrency` | ||
| * async functions execute simultaneously. | ||
| */ | ||
| function createSemaphore(concurrency) { |
There was a problem hiding this comment.
[LOW] 信号量缺少对并发参数的边界校验
createSemaphore 函数没有对 concurrency 参数进行有效性校验。如果传入 0 或负数，active < concurrency 条件将始终为真（0）或永不满足（负数），导致队列无限增长或永远阻塞。
建议: 在函数开头添加校验：if (typeof concurrency !== 'number' || concurrency < 1) throw new Error('concurrency must be a positive integer'); 或至少设置默认值。
风险: 如果 config.maxConcurrency 配置错误（如 0 或负数），可能导致死锁或资源耗尽。
置信度: 0.80
| const recommendationReason = String(generalResult.output.recommendationReason || '').trim(); | ||
|
|
||
| if (appendable.length > 0) { | ||
| executionDimensions.push(...appendable); |
There was a problem hiding this comment.
[LOW] 并行执行时 recommendedExtraDimensions 的动态追加可能影响其他批次
在并行执行场景下，每个批次的 executeBatch 会独立执行 general 维度并可能修改 executionDimensions 数组（追加推荐的额外维度）。虽然 executionDimensions 是每个批次的局部变量，但如果 reviewerAgents 对象是共享的，且 recommendedExtraDimensions 推荐的维度在并行批次间存在竞争，可能导致日志混乱或重复创建 reviewer agent。不过由于 reviewerAgents 的写入使用了 || 短路逻辑，实际风险较低。
建议: 此问题风险较低，但建议在文档中说明并行批次间维度推荐的独立性，或考虑在日志中标注批次 ID 以便于追踪。
风险: 可能导致日志中维度推荐信息混淆，但不影响功能正确性。
置信度: 0.75
| })) | ||
| ); | ||
|
|
||
| const dimSettled = await Promise.allSettled(dimPromises); |
There was a problem hiding this comment.
[LOW] Promise.allSettled 错误处理路径缺少测试
维度并行执行使用 Promise.allSettled,其中 rejected 分支增加 subAgentRuns 但未收集其他结果,该路径需要测试验证。
建议: 添加测试模拟维度执行抛出异常,验证 rejected 状态被正确处理、subAgentRuns 计数正确、其他成功维度结果被正常收集。
置信度: 0.75
2 participants
Summary
Promise.allSettled) and dimension-level (general-first, then remaining dimensions in parallel)createSemaphoreconcurrency limiter to cap total in-flight API callsmax_concurrencyaction input (default4) for user-tunable parallelismChanges
Core (
src/index.js)createSemaphore(n): inline Promise-based semaphore (no new dependencies)runDimension(): extracted function for single dimension review callexecuteBatch(): extracted function orchestrating general-first → parallel dimensionsfortoPromise.allSettledover semaphore-guarded batches[B{n}/{total}]prefix for parallel batch identificationConfig (
action.yml,src/config.js)max_concurrencyinput withparsePositiveIntInputvalidationTests
Test Plan
npm run check— syntax OKnpm test— 118/118 pass (111 existing + 7 new)max_concurrency=1produces identical results to previous serial behaviorNotes
max_concurrency=1is a safe fallback that restores serial executioncoverageStateupdates)🤖 Generated with TiyCode