Conversation
… simplified promisePool Co-authored-by: felickz <1760475+felickz@users.noreply.github.com>
Co-authored-by: felickz <1760475+felickz@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Apply changes based on review comments
Address review feedback: deterministic tests, cached normalization, simplified promisePool
Feb 9, 2026
There was a problem hiding this comment.
Pull request overview
This PR addresses three specific review comments from PR actions#1045 related to test reliability, performance optimization, and API clarity. The changes improve code quality without introducing any functional changes.
Changes:
- Replaced non-deterministic test delay (
Math.random() * 10) with fixed 5ms delay to prevent flaky tests - Extended
patchInfostructure to cache pre-normalized values (ecoLower,pkgLower,normalizedRange) computed once during API fetch - Simplified
promisePoolfunction signature fromPromise<T[]>toPromise<void>by removing unused results array and generic type parameter
Reviewed changes
Copilot reviewed 2 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/summary.ts | Simplified promisePool signature to Promise, added pre-normalized cache fields to patchInfo structure, and updated rendering loop to use cached values |
| dist/index.js | Compiled JavaScript output matching the TypeScript changes in src/summary.ts |
| tests/summary.test.ts | Changed concurrency test from random delay to deterministic 5ms delay |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
Resolves 3 outstanding review comments from PR actions#1045 review thread regarding test reliability, performance optimization, and API clarity.
Deterministic test delays
Replaced
Math.random() * 10with fixed5msdelay in concurrency test mock. Non-deterministic delays cause flaky tests on slower runners and intermittent CI failures.Pre-normalized cache values
Extended
patchInfostructure to storeecoLower,pkgLower, andnormalizedRangecomputed once during API fetch:Rendering loop now uses cached values instead of repeating
toLowerCase()andreplace()for each vulnerability row.Simplified promisePool signature
Changed from
Promise<T[]>toPromise<void>and removed unused results array. Return values were never consumed; generic type parameter was misleading.Related Issues
Addresses review comments in actions#1045 (review)
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.