[andre-j3sus]

Adds an experimental, undocumented parallel option to download/upload steps, and a measurementTargets option for spreading requests across several hosts.

new SpeedTest({
  measurementTargets: ['https://a.example.com', 'https://b.example.com'],
  measurements: [
    { type: 'download', bytes: 1e7, count: 5 },
    { type: 'download', bytes: 2e7, count: 3, parallel: 4 }
  ]
});

• First step: 5 sequential 10MB requests (count: 5), alternating between a and b from a random start. 5 data points of 10MB.
• Second step: 3 batches (count: 3) of 4 concurrent 20MB requests (parallel: 4), so 12 requests, round-robin across a and b. Each batch is one data point of bytes: 8e7 (4 × 20MB), so 3 data points.

Results: sequential steps stay in raw.<type>.results, unchanged. Parallel steps go to a new raw.<type>.parallelResults[parallel][bytes], keyed by parallelism and per-request size, with each bucket recording parallel and the batch total in bytes. That keeps them apart from sequential steps, even at the same total size, so apps can show them separately. The Results getters include both, sorted by bytes per sample, so headline numbers, loaded latency and the __results payload are unchanged for configs where parallel and sequential totals don't overlap. Parallel points get a parallel field, and a missing parallelResults (e.g. results loaded from elsewhere) counts as empty.

count is always the number of data points, so a parallel step makes count × parallel requests. A batch is treated as a single request: it is timed from the first request start to the last completion, with downloads excluding the smallest server time in the batch and uploads ending at the last response. Sequential steps are batches of one through the same code path, which gives the same timing as before. parallel is left out of the README while it's tried on speed.cloudflare.com, and per-request logging is skipped for parallel steps.

• Targets: origins only; the engine appends measurementDownloadPath or measurementUploadPath (default /__down and /__up, query strings allowed), and latency requests use them too. An entry that isn't a bare http(s) origin throws when the engine is created. With a single target, every request in a batch goes to it.
• Missing Timing-Allow-Origin: treated as a misconfigured target, like any failed request: reported through onError and skipped. Only missing resource timing entries count towards the three-in-a-row stop.
• Deprecated: downloadApiUrl and uploadApiUrl, still used when no targets are configured.
• Memory: download responses are drained as streams and only their tail is kept. Uploads still hold each request's body, so their memory grows with bytes × parallel.
• Failures: a failed request aborts the rest of its batch, which counts as a skipped sample. A 429 retries the whole batch, and abortRequestDuration applies to the batch as a whole. Batch requests get a req query param so each request's resource timing entry can be matched to it.

[ask-bonk]

Review: 1 findings.

Adds parallel bandwidth batches, URL round-robin support, aggregated timing/results, and coverage for batch behavior.

Posted 1 inline correctness finding. Unit tests could not run because pnpm is unavailable in the environment.

github run

[clanky-the-clanker]

Superseded by the review of 9f3766ff.

This review, of b8278a8b

💬 Approved with comments — The parallel batch and multi-URL logic looks correct. The only comments are non-blocking: batches skip server-time-delta calibration, and the README's batch timing wording is imprecise.

Findings: 2 info (2 are inline comments on the diff)

_{The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.}

---

📋 Code Quality: 💬 1 info

Batch logic is correct. Batches do not calibrate the server time delta, and they copy the result-saving tail from the sequential path.

Recommendation: 💬 Optional Nits

• [INFO] Batches skip serverTimeDelta calibration and the size-mismatch warning, and they duplicate the sequential result-saving tail (src/engines/BandwidthEngine/BandwidthEngine.ts:763)

View full review

The 429 retry, the abort when a request fails, round-robin URL selection and the req markers all behave as the description says. One note: #runBatch reads #serverTimeDelta but never updates it, and it does not emit the size-mismatch warning that the sequential path does. Its missing-entry handling and its result saving are also copied from the sequential chain.

---

📝 Documentation: 💬 1 info

The README's batch timing description is imprecise for uploads and downloads

Recommendation: 💬 Optional Nits

• [INFO] The parallel row's timing description does not match direction-specific batch timing (README.md:137)

View full review

The new parallel row describes a batch as timed "from the first request start to the last completion". The code is more specific: download batches subtract the smallest server time in the batch, and upload batches end at the last final response.

---

✅ All other checks passed

🔒 Security: ✅ LGTM

No security issues found.

---

📖 AGENTS.md: ✅ LGTM

No issues found.

---

⚡ Performance: ✅ LGTM

Each batch runs at most parallel concurrent requests and aborts as soon as one fails. The refactored sequential path adds no work per request.

---

_{Reviewed commit: b8278a8b}

[clanky-the-clanker]

Superseded by the review of 0a710a7f.

This review, of 9f3766ff

💬 Approved with comments — The parallel batch and measurementTargets logic is correct; the 429 retry, abort-on-failure and round-robin selection all behave as described. Three non-blocking INFO notes: batches skip serverTimeDelta calibration, malformed targets aren't validated, and the README's batch timing wording is imprecise.

Findings: 3 info (2 are inline comments on the diff)

_{The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.}

---

📋 Code Quality: 💬 2 info

The batch and multi-target logic is sound. Two INFO notes: batches diverge from the sequential path, and measurementTargets entries aren't validated.

Recommendation: 💬 Optional Nits

• [INFO] Batches skip serverTimeDelta calibration and the size-mismatch warning, and they duplicate the sequential result-saving code (src/engines/BandwidthEngine/BandwidthEngine.ts:763)
• [INFO] measurementTargets entries are not validated, so a malformed origin throws a TypeError partway through the test (src/utils/measurementTargets.ts:11)

View full review

These behave as the description says:

• A failed request aborts the batch, and later failures check signal.aborted, so each failure is handled once.
• A 429 retries the batch with a fresh abort controller.
• Results are keyed by the batch bytes in the engine, in index.ts and in numMeasurements.
• Loaded-latency engines receive the full list of URLs.

Two notes:

• #runBatch reads #serverTimeDelta but never calibrates it. It also skips the size-mismatch warning, and it copies the sequential path's missing-entry handling and result-saving code.
• resolveApiUrls runs new URL(path, origin) on every target without validating it. A target with no scheme throws a TypeError partway through the test, and an origin that includes a path loses that path without any warning.

---

📝 Documentation: 💬 1 info

The parallel row's timing description still does not match the direction-specific batch timing.

Recommendation: 💬 Optional Nits

• [INFO] The parallel row's timing description does not match the direction-specific batch timing (README.md:138)

View full review

The new parallel row, like the JSDoc in defaultConfig.ts, says a batch is timed "from the first request start to the last completion". The engine is more specific: download batches subtract the smallest server time in the batch, and upload batches end at the last final response headers.

---

✅ All other checks passed

🔒 Security: ✅ LGTM

The integrator configures measurementTargets. Every request still passes its final URL through withAuthorizationHeader, so the HTTPS-only gate on the authorization token applies to each target. The req query parameter is a fixed index.

---

📖 AGENTS.md: ✅ LGTM

No issues found.

---

⚡ Performance: ✅ LGTM

Each batch runs at most parallel concurrent requests and aborts on the first failure. URL resolution and round-robin selection do constant work per request.

---

_{Reviewed commit: 9f3766ff}

[clanky-the-clanker]

Superseded by the review of a037a0bf.

This review, of 0a710a7f

💬 Approved with comments — The earlier findings are fixed and no correctness, security or performance problems were found. One INFO note remains about the parallel JSDoc timing wording.

Findings: 1 info (1 is an inline comment on the diff)

_{The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.}

---

📝 Documentation: 💬 1 info

The parallel JSDoc still uses generic timing wording.

Recommendation: 💬 Optional Nits

• [INFO] The parallel JSDoc omits the direction-specific batch timing that the README describes. (src/config/defaultConfig.ts:18)

View full review

The README row now describes the timing for each direction. The parallel JSDoc in src/config/defaultConfig.ts still says only "from the first request start to the last completion".

---

✅ All other checks passed

🔒 Security: ✅ LGTM

Every batch request is built through withAuthorizationHeader, so the HTTPS-only authorization gate applies to each measurement target. Targets are validated as bare http(s) origins, and the req parameter is a fixed index.

---

📖 AGENTS.md: ✅ LGTM

No issues found.

---

📋 Code Quality: ✅ LGTM

• #runBatch calls #calibrateServerTimeDelta and #warnSizeMismatch for each request before aggregating. It shares #discardMissingEntry and #recordSample with the sequential path.
• parseOrigin rejects targets that have no scheme, a non-http(s) scheme, a path, a query or a fragment. Targets are resolved in the constructor, so a bad entry fails before the test starts.
• A failed request aborts the rest of its batch, and each failure is handled once. A 429 retries the whole batch with a fresh controller. Resource timing entries are matched by the per-request req URL and requestedAt.

---

⚡ Performance: ✅ LGTM

Each batch runs at most parallel concurrent requests and stops at the first failure. Selecting a target is constant work per request.

---

_{Reviewed commit: 0a710a7f}

[clanky-the-clanker]

Superseded by the review of ff643527.

This review, of a037a0bf

✅ Approved — The only change since the last review fixes the remaining note about the parallel JSDoc timing wording, and this revision has no open findings.

_{The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.}

---

✅ All checks passed

🔒 Security: ✅ LGTM

Every batch request is built through withAuthorizationHeader, so the HTTPS-only authorization gate applies to each measurement target. parseOrigin accepts only bare http(s) origins. The req parameter is a fixed index, and getParallelism turns invalid values into 1.

---

📖 AGENTS.md: ✅ LGTM

No issues found.

---

📋 Code Quality: ✅ LGTM

The parallel JSDoc now matches aggregateBatchTiming. In src/index.ts, results are keyed by getSampleBytes, and the engine passes the same value as bytes, so batch results go into the right buckets. Targets are resolved in the constructor, so an invalid entry throws before the test starts.

---

⚡ Performance: ✅ LGTM

A batch runs at most parallel concurrent requests and aborts the rest at the first failure. Choosing a target is constant work per request, and targets are resolved once.

---

📝 Documentation: ✅ LGTM

The parallel JSDoc now describes the timing for each direction: downloads exclude the smallest server processing time, and uploads end at the last response. This is consistent with the README row.

---

_{Reviewed commit: a037a0bf}

[clanky-the-clanker]

Superseded by the review of 29159fc1.

This review, of ff643527

💬 Approved with comments — No blocking or warning-level issues. One SUGGESTION: the path guard in resolveApiUrls can be bypassed by developer-supplied config.

Findings: 1 suggestion (1 is an inline comment on the diff)

_{The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.}

---

📋 Code Quality: 💬 1 suggestion

The new path options are sound, apart from an incomplete protocol-relative guard in resolveApiUrls.

Recommendation: 💬 Optional Nits

• [SUGGESTION] The // check in resolveApiUrls doesn't stop a path from resolving to an origin outside measurementTargets (src/utils/measurementTargets.ts:34)

View full review

src/utils/measurementTargets.ts:33-41: the guard rejects paths that start with //, but WHATWG URL parsing treats \ as / for http(s) URLs and strips tabs and newlines. So '/\\other.example.com/x' or '/\t/other.example.com' passes the prefix check and resolves to another host. Comparing the resolved URL's origin with the target origin covers every variant.

---

✅ All other checks passed

🔒 Security: ✅ LGTM

Every request, batch requests included, is built through withAuthorizationHeader, so the HTTPS-only authorization gate still applies. parseOrigin accepts only bare http(s) origins. getParallelism turns invalid parallel values into 1.

---

📖 AGENTS.md: ✅ LGTM

No issues found.

---

⚡ Performance: ✅ LGTM

Each batch runs at most parallel concurrent requests, and a failure aborts the rest of the batch. Targets are resolved once in the constructor, and round-robin selection does constant work per request.

---

📝 Documentation: ✅ LGTM

The README rows for the new measurementTargets, measurementDownloadPath and measurementUploadPath options, and the deprecation notes for downloadApiUrl and uploadApiUrl, match the code.

---

_{Reviewed commit: ff643527}

[andre-j3sus]

@carlosefr one design question on per-request logging. The log tokens are tied to the size of each individual request, so a batch logged as a single entry with the combined bytes would fail validation. For now, each request in a batch is logged on its own with its own token, while the combined batch data point is what the results and the final __results submission use.

Are you OK with that, or would you rather log one entry per batch? That would need server-side changes to accept batches.

[clanky-the-clanker]

Superseded by the review of 705aa503.

This review, of 29159fc1

✅ Approved — The earlier path-guard suggestion is fixed, and I found no new problems in the parallel batch, logging or measurement-target changes.

_{The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.}

---

✅ All checks passed

🔒 Security: ✅ LGTM

• Targets and paths: parseOrigin accepts only bare http(s) origins, and resolveApiUrls rejects any path that resolves outside the target's origin.
• Authorization: batch requests still go through withAuthorizationHeader, so the HTTPS-only gate on the token applies to them.
• Parallelism: getParallelism limits invalid parallel values to 1.

---

📖 AGENTS.md: ✅ LGTM

No issues found.

---

📋 Code Quality: ✅ LGTM

• Path guard: src/utils/measurementTargets.ts now rejects any path whose resolved origin differs from the target's. The tests add /\host and tab cases.
• Batch failures: a failed request aborts the rest of its batch. A 429 retry builds a fresh batch controller, and the abort listener is removed in .finally.
• Logging: LoggingBandwidthEngine logs each request's own token and request time from its result payload. This replaces the shared fields that concurrent requests would have overwritten.
• Optimal load size: largestSize uses the per-request bytes of the steps up to the current one.

---

⚡ Performance: ✅ LGTM

Batch responses are drained as streams and only their tail is kept. Each batch runs at most parallel concurrent requests.

---

📝 Documentation: ✅ LGTM

The README rows for parallel, measurementTargets, measurementDownloadPath and measurementUploadPath match the code. The parallel row says that each of the count data points is a batch of parallel requests.

---

_{Reviewed commit: 29159fc1}

[andre-j3sus]

A note for reviewers on latency with multiple targets: idle and loaded latency pings rotate across measurementTargets, like downloads and uploads. Jitter is the difference between consecutive pings, so targets with different round-trip times would show up as extra jitter, and the latency median would blend hosts.

This is deliberate. Targets are meant to be equivalent endpoints of one service, such as shards on the same network, where round-trip times match and the effect is negligible. Rotating also keeps a single degraded target from defining the whole latency result. The docs only say the test iterates over the targets, so this requirement lives with whoever configures them. If non-equivalent targets become a real use case, the follow-up would be computing jitter per target instead of pinning latency to one.

[clanky-the-clanker]

Superseded by the review of 2f0622e7.

This review, of 705aa503

💬 Approved with comments — No correctness or security problems found in the changes since the last review. One INFO note: the packet-loss load generator uses only the first measurement target, and the docs don't say so.

Findings: 1 info (1 is an inline comment on the diff)

_{The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.}

---

📋 Code Quality: 💬 1 info

Changes since the last review are correct; one INFO note about how the packet-loss load generator uses targets.

Recommendation: 💬 Optional Nits

• [INFO] The packet-loss-under-load generator only uses the first measurement target, but the measurementTargets docs say requests round-robin across all targets. (src/index.ts:543)

View full review

• Load generator uses only the first target (src/index.ts:543-544): the packet-loss-under-load engine gets downloadApiUrl[0] and uploadApiUrl[0]. The measurementTargets JSDoc says requests round-robin across the origins and doesn't mention this exception.
• Path guard and origin parsing (src/utils/measurementTargets.ts): paths are resolved against the target and rejected if the origin changes. A placeholder origin catches bad paths when no targets are configured.
• Config defaults (src/index.ts): ??= keeps an explicit undefined from overriding the defaults, and an invalid target throws on construction.

---

✅ All other checks passed

🔒 Security: ✅ LGTM

• Targets: parseOrigin accepts only bare http(s) origins, and resolveApiUrls rejects any path that resolves outside its target's origin.
• Authorization: batch requests go through withAuthorizationHeader, so the HTTPS-only gate on the token still applies.
• Parallelism: getParallelism clamps non-finite values and values below 1 to 1.

---

📖 AGENTS.md: ✅ LGTM

No issues found.

---

⚡ Performance: ✅ LGTM

No issues found.

---

📝 Documentation: ✅ LGTM

No issues found.

---

_{Reviewed commit: 705aa503}

[clanky-the-clanker]

Superseded by the review of 159e9ff7.

This review, of 2f0622e7

💬 Approved with comments — No correctness, security or performance problems in the changes since the last review. One INFO note on the README's description of the missing-timing stop condition.

Findings: 1 info (1 is an inline comment on the diff)

_{The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.}

---

📝 Documentation: 💬 1 info

The README count row doesn't describe how the missing-timing stop condition applies to batches and to restricted timings.

Recommendation: 💬 Optional Nits

• [INFO] The count row's three-in-a-row stop condition doesn't mention parallel batches or entries without detailed timings. (README.md:138)

View full review

• README.md:138: the count row says the test ends after three requests in a row with no resource timing entry. Two cases are missing from that sentence:
  • For a parallel step, the engine counts each batch as one sample, so the test ends after three batches in a row.
  • Entries without detailed timings (no Timing-Allow-Origin) are now discarded the same way.

---

✅ All other checks passed

🔒 Security: ✅ LGTM

• Targets: parseOrigin accepts only bare http(s) origins. resolveApiUrls rejects any path that resolves to a different origin.
• Authorization: each batch request is built by #buildRequest, which calls withAuthorizationHeader, so the HTTPS-only gate on the token still applies.
• Logging: each logged request now takes its token and timing from its own response.

---

📖 AGENTS.md: ✅ LGTM

No issues found.

---

📋 Code Quality: ✅ LGTM

• Load generator target: src/index.ts passes only the first target to the packet-loss load generator. The measurementTargets JSDoc and the README now say so.
• Samples without detailed timing: sequential and batch samples both discard a sample with a missing or restricted entry. Each one counts toward MAX_MISSING_ENTRIES (3), and the counter resets after a good sample. The signal.aborted check runs before the discard, so an aborted batch is not also counted as a missing entry.

---

⚡ Performance: ✅ LGTM

• Memory: batch download responses keep only their last 1 KB. Upload memory grows with bytes × parallel, as the documentation says.
• CPU: the resource-timing lookups and the min/max reductions run over at most parallel entries per batch.

---

_{Reviewed commit: 2f0622e7}

[clanky-the-clanker]

Superseded by the review of a31994be.

This review, of 159e9ff7

💬 Approved with comments — No code problems. One WARNING: the README says parallel steps always end the test when some targets lack Timing-Allow-Origin, which isn't true when parallel is smaller than the number of targets.

Findings: 1 warning (1 is an inline comment on the diff)

_{The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.}

🔧 Fix with your agent

Paste the following into your coding agent to investigate and fix these issues.

Fix the following code review issues found in pull request #176.

Before making changes, review each issue and present a brief summary table:
- For each finding, state whether you agree, disagree, or need clarification
- If you disagree (e.g., the fix would require disproportionate changes
  for minimal ROI, or the finding is factually incorrect), explain why
- If you need to clarify something before deciding, ask those questions
- Then share your plan for which issues to tackle and in what order

## WARNING (1)

### [Documentation] README says every parallel batch includes targets without Timing-Allow-Origin, which is false when `parallel` is smaller than the number of targets.
- **File:** README.md:47

---

📝 Documentation: ⚠️ 1 warning

README overstates when parallel steps end the test because some targets lack Timing-Allow-Origin.

Recommendation: 📝 Minor Comments

• [WARNING] README says every parallel batch includes targets without Timing-Allow-Origin, which is false when parallel is smaller than the number of targets. (README.md:47)

View full review

The measurementTargets row (README.md:47) says that parallel steps end the test when only some targets lack Timing-Allow-Origin, "as every batch includes them". That is only true when parallel is at least the number of targets.

---

✅ All other checks passed

🔒 Security: ✅ LGTM

• Targets: parseOrigin accepts only bare http(s) origins. resolveApiUrls rejects any path that resolves to a different origin, including //host and /\host.
• Authorization: every batch request goes through withAuthorizationHeader, so the HTTPS-only gate on the token still applies.
• Input: parallel values that are not finite numbers of at least 1 fall back to 1.

---

📖 AGENTS.md: ✅ LGTM

No issues found.

---

📋 Code Quality: ✅ LGTM

No correctness or error-handling problems found.

• Batch path: the abort timer carries over across 429 retries. An aborted batch does not count as a missing entry. Samples with missing or restricted timing are discarded and count toward the three-in-a-row limit.
• Logging: each logged request takes its token and server time from its own response. This removes the shared state that concurrent requests would overwrite.
• Results keying: results, the side-latency merge and the optimal-load-size computation all use the same sample-bytes key.

---

⚡ Performance: ✅ LGTM

• Downloads: batch download responses are drained as streams, and only the last 1 KB of each is kept.
• Uploads: upload memory grows with bytes × parallel, as documented.
• Abort listener: it is removed after each batch.
• Target resolution: targets are resolved once, at construction.

---

_{Reviewed commit: 159e9ff7}

[clanky-the-clanker]

Superseded by the review of 3167cfd8.

This review, of a31994be

💬 Approved with comments — The README WARNING from the last review is fixed and the code has no problems. One INFO note remains: the count row's first sentence doesn't cover parallel steps.

Findings: 1 info (1 is an inline comment on the diff)

_{The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.}

---

📝 Documentation: 💬 1 info

The previous WARNING is fixed. One INFO: the count row still says count is a number of requests.

Recommendation: 💬 Optional Nits

• [INFO] The count row's first sentence calls it the number of requests, but for a parallel step it is the number of batches. (README.md:138)

View full review

• The measurementTargets row now describes partial Timing-Allow-Origin coverage correctly, including the case where parallel is smaller than the number of targets.
• README.md:138: the count row opens with "The number of requests to perform". For a parallel step, count is the number of batches.

---

✅ All other checks passed

🔒 Security: ✅ LGTM

• Targets: parseOrigin accepts only bare http(s) origins.
• Paths: resolveApiUrls rejects any path that resolves to a different origin, including //host, /\host and tab variants.
• Authorization: every request in a batch goes through withAuthorizationHeader, so the HTTPS-only gate on the token still applies.

---

📖 AGENTS.md: ✅ LGTM

No issues found.

---

📋 Code Quality: ✅ LGTM

• Batch failures: a failed request aborts only its own batch, so the error handler runs once per batch.
• 429 retries: a retry reuses the set's abort controller and timer. The abort listener is removed in finally.
• Results keying: results, the side-latency merge and the optimal-load-size calculation all use the same sample-bytes key.
• Logging: each logged request takes its token and server time from its own response. There is no longer shared state for concurrent requests to overwrite.

---

⚡ Performance: ✅ LGTM

No issues found.

---

_{Reviewed commit: a31994be}

[clanky-the-clanker]

Superseded by the review of 6da5018d.

This review, of 3167cfd8

✅ Approved — The last open documentation note is fixed, and I found nothing new in the code, the security checks, performance or the docs.

_{The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.}

---

✅ All checks passed

🔒 Security: ✅ LGTM

• Targets: parseOrigin accepts only bare http(s) origins.
• Paths: resolveApiUrls rejects any path that resolves to a different origin, including //host, /\host and tab variants.
• Authorization: every request in a batch goes through withAuthorizationHeader, so the token is still withheld from non-HTTPS endpoints.

---

📖 AGENTS.md: ✅ LGTM

No issues found.

---

📋 Code Quality: ✅ LGTM

• Batch failures and retries: a failed request aborts only its own batch. A 429 retry reuses the set's abort timer and controller, and the abort listener is removed in finally.
• Per-request logging: logging goes through onRequestResult. Each logged request takes its token and server time from its own response.
• Results keying: numMeasurements, sideLatency and the engine's results all use the same sample-bytes key. The optimal load size is worked out from the per-request bytes.
• Target validation: targets and paths are resolved once, in the constructor, so a bad target or path fails when the engine is created.

---

⚡ Performance: ✅ LGTM

Batch download bodies are read as streams and only their tail is kept, so concurrent downloads don't hold their payloads in memory. Uploads still hold each request's body; the README says so.

---

📝 Documentation: ✅ LGTM

• README.md:138: the count row now calls count the number of samples (requests, or batches in a parallel step) and says the step makes count * parallel requests.
• The parallel row and the JSDoc both describe batch timing per direction: downloads leave out the smallest server time, and uploads end at the last response.

---

_{Reviewed commit: 3167cfd8}

[clanky-the-clanker]

Superseded by the review of cd537b25.

This review, of 6da5018d

✅ Approved — I found no correctness, security, performance or documentation issues in the changes since the last review.

_{The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.}

---

✅ All checks passed

🔒 Security: ✅ LGTM

• Targets and paths: parseOrigin and the origin check in resolveApiUrls are unchanged and run when the engine is created.
• Authorization: each request's options are built with withAuthorizationHeader for that request's own URL.
• Logging: parallel steps send nothing new to the results endpoint.

---

📖 AGENTS.md: ✅ LGTM

No issues found.

---

📋 Code Quality: ✅ LGTM

• Shared request path: sequential and parallel samples now use the same code. For a batch of one, the timing reduces to the old per-request formulas.
• Failures: a failed request aborts only its own batch, and each failure is handled once.
• Retries and pausing: a 429 retry keeps the set's controller and timer, and pause() still reaches the batch.
• Logging: logging is skipped for parallel steps through getParallelism(measurement) > 1.
• Optimal load size: it is now worked out from the per-request bytes of steps whose sample key is in the results.

---

⚡ Performance: ✅ LGTM

• Download memory: download bodies are read as streams, and only a 1 KB tail is kept.
• Batch cost: batch timing and the resource timing lookups are linear in the batch size.
• Abort listeners: they are removed after each batch.

---

📝 Documentation: ✅ LGTM

The README and the defaultConfig.ts JSDoc match the current behaviour.

---

_{Reviewed commit: 6da5018d}

[andre-j3sus]

Reviewer note: results are keyed by a sample's total bytes (bytes × parallel), so a parallel step whose total matches a sequential step's bytes shares its results bucket. For example, { bytes: 1e6, parallel: 4 } and { bytes: 4e6 } both key on 4000000, and their points (and loaded latency) can't be told apart. I've left this undocumented along with parallel itself, so when picking parallel step sizes, keep their totals distinct from the sequential ones.

[andre-j3sus]

Follow-up to the note above: as of cd537b2 parallel steps are stored in their own parallelResults[parallel][bytes] map, so they no longer share a bucket with a sequential step of the same total size. The getters still include both.

[clanky-the-clanker]

✅ Approved — No correctness, security, performance or documentation issues in the changes since the last review. Storing parallel results separately is consistent with how the getters and the optimal-size logic read them.

_{The decision is advisory: this review is a comment and never approves, blocks or requests changes on the pull request.}

---

✅ All checks passed

🔒 Security: ✅ LGTM

The changes since the last review don't touch target or path validation, authorization headers or what is logged.

---

📖 AGENTS.md: ✅ LGTM

No issues found.

---

📋 Code Quality: ✅ LGTM

• Storage keys: the bucket key in parallelResults[parallel][requestBytes] matches the key the optimal-size check reads.
• Combined buckets: #getBuckets sorts the combined buckets by bytes per sample and treats a missing parallelResults as {}.
• Other calculations: the extra argument passed in #calcGetter is ignored by the non-bandwidth calculations.
• Tests: the new tests cover the changed behaviour.

---

⚡ Performance: ✅ LGTM

Merging and sorting the buckets is linear in the number of steps. I found no new performance concerns.

---

📝 Documentation: ✅ LGTM

The parallelResults layout is internal to the experimental, undocumented parallel option. The existing README and JSDoc stay accurate.

---

_{Reviewed commit: cd537b25}