chore(docs): add adoption metrics generation and service#156
Open
coryrylan wants to merge 1 commit into
Open
Conversation
github-actions
Bot
added
scope(internals)
scope(docs)
dependencies
📝 WalkthroughWalkthroughAdds an adoption metrics generation pipeline and site dashboard, and switches the metrics page release chart to weekly milestones. ChangesAdoption metrics pipeline
Adoption dashboard
Weekly release metrics
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Biome (2.5.0)projects/internals/metadata/static/adoption.jsonFile contains syntax errors that prevent linting: Line 1: String values must be double quoted.; Line 1: String values must be double quoted.; Line 1: End of file expected; Line 3: End of file expected projects/internals/metadata/static/releases.jsonFile contains syntax errors that prevent linting: Line 1: String values must be double quoted.; Line 1: String values must be double quoted.; Line 1: End of file expected; Line 2: End of file expected; Line 2: String values must be double quoted.; Line 3: String values must be double quoted.; Line 3: End of file expected 🔧 OpenGrep (1.23.0)projects/internals/metadata/src/index.ts┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m [00.15][ERROR]: unable to find a config; path projects/internals/metadata/src/services/adoption.service.test.ts┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m [00.18][ERROR]: unable to find a config; path projects/internals/metadata/src/tasks/adoption.ts┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m [00.16][ERROR]: unable to find a config; path
🔧 ESLint
projects/internals/metadata/src/services/adoption.service.test.tsParsing error: error TS5012: Cannot read file '/tsconfig.json': ENOENT: no such file or directory, open '/tsconfig.json'. projects/internals/metadata/src/services/releases.service.test.tsParsing error: error TS5012: Cannot read file '/tsconfig.json': ENOENT: no such file or directory, open '/tsconfig.json'. projects/internals/metadata/src/tasks/adoption.utils.test.tsParsing error: error TS5012: Cannot read file '/tsconfig.json': ENOENT: no such file or directory, open '/tsconfig.json'.
🔧 markdownlint-cli2 (0.22.1)projects/internals/metadata/README.mdmarkdownlint-cli2 wrapper config was not available before execution Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| const releasesReportDate = new Intl.DateTimeFormat('en-US', { dateStyle: 'medium', timeStyle: 'long' }).format( | ||
| new Date(releases.created) | ||
| ); | ||
| const adoptionReportDate = new Intl.DateTimeFormat('en-US', { dateStyle: 'medium', timeStyle: 'long' }).format( |
- Introduced `AdoptionService` to fetch and cache public adoption metrics from npm, jsDelivr, and GitHub. - Added `adoption.ts` task to generate adoption metrics and save them to `static/adoption.json`. - Implemented utility functions for parsing adoption data from various sources. - Updated `package.json` scripts to include a new `generate:adoption` command. - Enhanced documentation to reflect the new adoption metrics functionality. Signed-off-by: Cory Rylan <crylan@nvidia.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@projects/internals/metadata/README.md`:
- Line 41: The README entry for AdoptionService overstates runtime behavior by
implying a CDN/fallback path that the actual AdoptionService implementation does
not provide. Update the documentation to accurately reflect the current
local-only loading from static/adoption.json, or implement the described
dual-loading/CDN fallback in adoption.service.ts and then document that behavior
consistently. Use the AdoptionService symbol and its adoption.json loading
behavior as the source of truth.
In `@projects/internals/metadata/src/services/adoption.service.test.ts`:
- Line 15: The test in AdoptionService.getData is asserting shared object
identity with toBe(adoption), which ties the test to the service returning the
same cached reference. Update the expectation to verify equivalent contents
instead of the exact object instance, so the test still passes if
AdoptionService.getData returns a defensive clone or frozen copy.
In `@projects/internals/metadata/src/services/adoption.service.ts`:
- Around line 36-42: The getData method in AdoptionService is returning the
cached `#adoption` object by reference, which lets callers mutate shared state.
Update getData so it returns a defensive copy or an immutable/frozen payload
before exposing it, while keeping the internal `#adoption` cache private and
unchanged for later reads. Use the AdoptionService.#adoption field and getData
method as the entry points for the fix.
In `@projects/internals/metadata/src/tasks/adoption.ts`:
- Around line 7-9: The adoption export in getAdoptionData/writeFileSync is using
a cwd-dependent path, so resolve the output location relative to this module
instead of "./static/adoption.json". Update the adoption task to derive the
static file path from the current file/module location and write the JSON there
so it always lands where the service reads it, regardless of where the task is
run from.
In `@projects/internals/metadata/src/tasks/adoption.utils.ts`:
- Around line 358-369: Cap the GitHub stargazer page fetch burst in the adoption
task by replacing the unbounded Promise.all fan-out in the stargazer pagination
logic with a bounded/concurrent approach. Update the pagination fetch flow
around parseGitHubPaginationTotal and fetchPublicJson so only a limited number
of page requests run at once, while still collecting all pages into stargazers.
- Around line 387-415: The adoption summary branch in `buildAdoptionStatus` is
discarding successful npm/CDN data whenever `parseNpmRegistry` returns null.
Change the `!registry` path so it still uses
`parseNpmDownloads(downloadsResult.data)` and
`parseJsDelivrStats(cdnResult.data, ...)` when those results are OK, and only
fall back to `createEmptyNpmDownloads()` / `createEmptyCdnStats()` for the
sources that actually failed; keep `errors` and status handling based on
`registryResult`, `downloadsResult`, and `cdnResult`.
In `@projects/internals/metadata/static/adoption.json`:
- Around line 1-3: The checked-in adoption data artifact is a Git LFS pointer
instead of valid JSON, so `AdoptionService.getData()` cannot parse it in
environments without LFS materialization. Replace the pointer contents with the
actual JSON payload (or change `AdoptionService.getData()` to load from a
non-LFS source) so the imported `adoption.json` is valid at runtime and can be
parsed reliably.
In `@projects/site/src/docs/metrics/adoption-data.test.ts`:
- Around line 101-154: The adoption-data tests are missing coverage for the
untested helper paths, so add a case that exercises getReleaseAdoptionTimeline
and a CDN share case with unsorted versions. Update the adoptionData test suite
to assert marker alignment and leading-version selection through
getReleaseAdoptionTimeline and getCdnVersionShareRows, using the existing
adoptionFixture-style helpers so regressions in sorting or latest-version
detection are caught.
In `@projects/site/src/docs/metrics/adoption-data.ts`:
- Around line 117-121: The helper getTopNonLatestVersionData is selecting the
first non-latest version instead of the true leading one, so the adoption row
can report the wrong CDN version and totals. Update this function to choose the
non-latest entry with the highest share/request count from
packageData.cdn.versions, rather than using find. Keep the latestVersion
exclusion, but compare the remaining versions by the same metric used elsewhere
in the adoption data model so the returned AdoptionCdnVersionLike is the actual
leader.
In `@projects/site/src/docs/metrics/index.11ty.js`:
- Around line 19-21: The adoption status line is reusing the releases timestamp
instead of the adoption data timestamp, so the metrics page can show the wrong
freshness for adoption totals/charts. Update the metrics template in the docs
page to use the adoption-specific formatted date variable created from
adoption.created (the adoptionReportDate symbol) wherever the adoption section’s
status line is rendered, and keep releasesReportDate only for release data.
In `@projects/site/src/docs/metrics/release-data.test.ts`:
- Around line 15-45: The current `getWeeklyReleaseMilestones` test only covers
straightforward same-month dates, so add a regression case in
`release-data.test.ts` that exercises a week boundary with releases on a
Sunday/Monday rollover and across a month or year change. Extend the
`describe('release chart data')` coverage with a new `it(...)` that asserts the
correct `weekKey`, `formattedDate`, `releasesThisWeek`, and `cumulativeTotal`
for `getWeeklyReleaseMilestones`, using `releaseFixture`-style inputs that prove
Monday-based bucketing stays correct across boundaries.
In `@projects/site/src/docs/metrics/release-data.ts`:
- Around line 23-27: The weekly date label formatter in WEEK_START_FORMATTER
drops the year, causing repeated labels across calendar years. Update the
formatting used by formattedDate in release-data.ts so week labels include the
year for both the chart axis and tooltip title, and apply the same change
anywhere the shared weekly label format is reused.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 94968d36-e81c-4234-bffc-3d761c9ac3cd
📒 Files selected for processing (23)
projects/internals/metadata/README.mdprojects/internals/metadata/package.jsonprojects/internals/metadata/src/index.tsprojects/internals/metadata/src/services/adoption.service.test.tsprojects/internals/metadata/src/services/adoption.service.tsprojects/internals/metadata/src/services/releases.service.test.tsprojects/internals/metadata/src/tasks/adoption.tsprojects/internals/metadata/src/tasks/adoption.utils.test.tsprojects/internals/metadata/src/tasks/adoption.utils.tsprojects/internals/metadata/src/types.tsprojects/internals/metadata/static/adoption.jsonprojects/internals/metadata/static/releases.jsonprojects/internals/metadata/vitest.config.tsprojects/site/package.jsonprojects/site/src/_11ty/layouts/common.jsprojects/site/src/docs/metrics/adoption-data.test.tsprojects/site/src/docs/metrics/adoption-data.tsprojects/site/src/docs/metrics/adoption.tsprojects/site/src/docs/metrics/bundle-explorer.11ty.jsprojects/site/src/docs/metrics/index.11ty.jsprojects/site/src/docs/metrics/index.tsprojects/site/src/docs/metrics/release-data.test.tsprojects/site/src/docs/metrics/release-data.ts
| expect(adoption.packages.length).toBeGreaterThan(0); | ||
| expect(adoption.totals.packages).toBe(adoption.packages.length); | ||
| expect(adoption.github.repository).toBe('NVIDIA/elements'); | ||
| expect(await AdoptionService.getData()).toBe(adoption); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Avoid asserting shared object identity here.
toBe(adoption) makes this test depend on the service leaking its internal cache reference, which blocks a defensive clone/freeze fix. Assert equal contents instead.
Suggested change
- expect(await AdoptionService.getData()).toBe(adoption);
+ expect(await AdoptionService.getData()).toEqual(adoption);📝 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.
Suggested change
| expect(await AdoptionService.getData()).toBe(adoption); | |
| expect(await AdoptionService.getData()).toEqual(adoption); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@projects/internals/metadata/src/services/adoption.service.test.ts` at line
15, The test in AdoptionService.getData is asserting shared object identity with
toBe(adoption), which ties the test to the service returning the same cached
reference. Update the expectation to verify equivalent contents instead of the
exact object instance, so the test still passes if AdoptionService.getData
returns a defensive clone or frozen copy.
Comment on lines
+36
to
+42
| static async getData(): Promise<AdoptionSummary> { | ||
| if (AdoptionService.#adoption.created === '') { | ||
| AdoptionService.#adoption = (await import('../../static/adoption.json', { with: { type: 'json' } })) | ||
| .default as AdoptionSummary; | ||
| } | ||
|
|
||
| return AdoptionService.#adoption; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Do not expose the mutable cache object directly.
Returning AdoptionService.#adoption by reference lets any caller mutate packages, totals, or github and poison every later read from this process. Return a defensive clone or freeze the cached payload before handing it out.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@projects/internals/metadata/src/services/adoption.service.ts` around lines 36
- 42, The getData method in AdoptionService is returning the cached `#adoption`
object by reference, which lets callers mutate shared state. Update getData so
it returns a defensive copy or an immutable/frozen payload before exposing it,
while keeping the internal `#adoption` cache private and unchanged for later
reads. Use the AdoptionService.#adoption field and getData method as the entry
points for the fix.
Comment on lines
+7
to
+9
| const adoption = await getAdoptionData(); | ||
|
|
||
| writeFileSync('./static/adoption.json', JSON.stringify(adoption, null, 2)); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Resolve the output file relative to this module.
./static/adoption.json depends on cwd. Running this task from the repo root or a workspace runner writes to the wrong place or throws, while the service always reads the module-relative static file.
Suggested fix
const adoption = await getAdoptionData();
-writeFileSync('./static/adoption.json', JSON.stringify(adoption, null, 2));
+writeFileSync(new URL('../../static/adoption.json', import.meta.url), JSON.stringify(adoption, null, 2));📝 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.
Suggested change
| const adoption = await getAdoptionData(); | |
| writeFileSync('./static/adoption.json', JSON.stringify(adoption, null, 2)); | |
| const adoption = await getAdoptionData(); | |
| writeFileSync(new URL('../../static/adoption.json', import.meta.url), JSON.stringify(adoption, null, 2)); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@projects/internals/metadata/src/tasks/adoption.ts` around lines 7 - 9, The
adoption export in getAdoptionData/writeFileSync is using a cwd-dependent path,
so resolve the output location relative to this module instead of
"./static/adoption.json". Update the adoption task to derive the static file
path from the current file/module location and write the JSON there so it always
lands where the service reads it, regardless of where the task is run from.
Comment on lines
+358
to
+369
| const totalPages = parseGitHubPaginationTotal(firstPage.headers.get('link'), 1); | ||
| const remainingPages = await Promise.all( | ||
| Array.from({ length: Math.max(totalPages - 1, 0) }, (_, index) => | ||
| fetchPublicJson( | ||
| `https://api.github.com/repos/${repository}/stargazers?per_page=100&page=${index + 2}`, | ||
| 'github', | ||
| { | ||
| ...headers, | ||
| Accept: 'application/vnd.github.star+json' | ||
| } | ||
| ) | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Cap GitHub stargazer pagination concurrency.
Promise.all fires one request per remaining page immediately. As the repo grows, that burst can hit GitHub secondary rate limits and leave stargazers partial even though the task keeps running.
Suggested fix
- const remainingPages = await Promise.all(
- Array.from({ length: Math.max(totalPages - 1, 0) }, (_, index) =>
- fetchPublicJson(
- `https://api.github.com/repos/${repository}/stargazers?per_page=100&page=${index + 2}`,
- 'github',
- {
- ...headers,
- Accept: 'application/vnd.github.star+json'
- }
- )
- )
- );
+ const remainingPages: PublicJsonResult[] = [];
+ for (let page = 2; page <= totalPages; page += 1) {
+ remainingPages.push(
+ await fetchPublicJson(`https://api.github.com/repos/${repository}/stargazers?per_page=100&page=${page}`, 'github', {
+ ...headers,
+ Accept: 'application/vnd.github.star+json'
+ })
+ );
+ }📝 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.
Suggested change
| const totalPages = parseGitHubPaginationTotal(firstPage.headers.get('link'), 1); | |
| const remainingPages = await Promise.all( | |
| Array.from({ length: Math.max(totalPages - 1, 0) }, (_, index) => | |
| fetchPublicJson( | |
| `https://api.github.com/repos/${repository}/stargazers?per_page=100&page=${index + 2}`, | |
| 'github', | |
| { | |
| ...headers, | |
| Accept: 'application/vnd.github.star+json' | |
| } | |
| ) | |
| ) | |
| const totalPages = parseGitHubPaginationTotal(firstPage.headers.get('link'), 1); | |
| const remainingPages: PublicJsonResult[] = []; | |
| for (let page = 2; page <= totalPages; page += 1) { | |
| remainingPages.push( | |
| await fetchPublicJson(`https://api.github.com/repos/${repository}/stargazers?per_page=100&page=${page}`, 'github', { | |
| ...headers, | |
| Accept: 'application/vnd.github.star+json' | |
| }) | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@projects/internals/metadata/src/tasks/adoption.utils.ts` around lines 358 -
369, Cap the GitHub stargazer page fetch burst in the adoption task by replacing
the unbounded Promise.all fan-out in the stargazer pagination logic with a
bounded/concurrent approach. Update the pagination fetch flow around
parseGitHubPaginationTotal and fetchPublicJson so only a limited number of page
requests run at once, while still collecting all pages into stargazers.
Comment on lines
+101
to
+154
| describe('adoption chart data', () => { | ||
| it('should create package download totals for a stacked trend chart', () => { | ||
| const trend = getPackageDownloadTrend(adoptionFixture); | ||
|
|
||
| expect(trend.labels).toEqual(['2026-06-01', '2026-06-02']); | ||
| expect(trend.packages).toEqual([ | ||
| { name: 'lint', total: 40, values: [25, 15] }, | ||
| { name: 'core', total: 15, values: [5, 10] }, | ||
| { name: 'code', total: 5, values: [0, 5] } | ||
| ]); | ||
| }); | ||
|
|
||
| it('should expose latest version share rows for CDN adoption', () => { | ||
| expect(getCdnVersionShareRows(adoptionFixture)).toEqual([ | ||
| { | ||
| name: 'core', | ||
| latestVersion: '2.0.2', | ||
| latestVersionShare: 30, | ||
| latestVersionRequests: 30, | ||
| topVersion: '2.0.1', | ||
| topVersionShare: 65, | ||
| topNonLatestVersion: '2.0.1', | ||
| topNonLatestVersionShare: 65, | ||
| topNonLatestVersionRequests: 65, | ||
| otherVersionShare: 5, | ||
| otherVersionRequests: 5, | ||
| cdnRequests: 100 | ||
| }, | ||
| { | ||
| name: 'code', | ||
| latestVersion: '2.0.1', | ||
| latestVersionShare: 100, | ||
| latestVersionRequests: 10, | ||
| topVersion: '2.0.1', | ||
| topVersionShare: 100, | ||
| topNonLatestVersion: 'none', | ||
| topNonLatestVersionShare: 0, | ||
| topNonLatestVersionRequests: 0, | ||
| otherVersionShare: 0, | ||
| otherVersionRequests: 0, | ||
| cdnRequests: 10 | ||
| } | ||
| ]); | ||
| }); | ||
|
|
||
| it('should expose npm and CDN channel mix points', () => { | ||
| expect(getChannelMixPoints(adoptionFixture)).toEqual([ | ||
| { name: 'core', status: 'published', x: 15, y: 100 }, | ||
| { name: 'lint', status: 'published', x: 40, y: 0 }, | ||
| { name: 'code', status: 'published', x: 5, y: 10 }, | ||
| { name: 'media', status: 'unavailable', x: 0, y: 0 } | ||
| ]); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Add coverage for the untested adoption helper paths.
This suite never exercises getReleaseAdoptionTimeline, and the CDN share case only uses pre-sorted versions, so regressions in marker alignment or leading-version selection would still pass.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@projects/site/src/docs/metrics/adoption-data.test.ts` around lines 101 - 154,
The adoption-data tests are missing coverage for the untested helper paths, so
add a case that exercises getReleaseAdoptionTimeline and a CDN share case with
unsorted versions. Update the adoptionData test suite to assert marker alignment
and leading-version selection through getReleaseAdoptionTimeline and
getCdnVersionShareRows, using the existing adoptionFixture-style helpers so
regressions in sorting or latest-version detection are caught.
Comment on lines
+117
to
+121
| function getTopNonLatestVersionData( | ||
| packageData: AdoptionPackageLike, | ||
| latestVersion: string | ||
| ): AdoptionCdnVersionLike | undefined { | ||
| return packageData.cdn.versions.find(version => version.version !== latestVersion); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Pick the actual leading non-latest CDN version.
This returns the first non-latest entry, not the highest-share one. If packageData.cdn.versions is not already sorted, the CDN adoption row will report the wrong version and request totals.
Suggested fix
function getTopNonLatestVersionData(
packageData: AdoptionPackageLike,
latestVersion: string
): AdoptionCdnVersionLike | undefined {
- return packageData.cdn.versions.find(version => version.version !== latestVersion);
+ return packageData.cdn.versions
+ .filter(version => version.version !== latestVersion)
+ .sort((a, b) => b.share - a.share || b.total - a.total)[0];
}📝 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.
Suggested change
| function getTopNonLatestVersionData( | |
| packageData: AdoptionPackageLike, | |
| latestVersion: string | |
| ): AdoptionCdnVersionLike | undefined { | |
| return packageData.cdn.versions.find(version => version.version !== latestVersion); | |
| function getTopNonLatestVersionData( | |
| packageData: AdoptionPackageLike, | |
| latestVersion: string | |
| ): AdoptionCdnVersionLike | undefined { | |
| return packageData.cdn.versions | |
| .filter(version => version.version !== latestVersion) | |
| .sort((a, b) => b.share - a.share || b.total - a.total)[0]; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@projects/site/src/docs/metrics/adoption-data.ts` around lines 117 - 121, The
helper getTopNonLatestVersionData is selecting the first non-latest version
instead of the true leading one, so the adoption row can report the wrong CDN
version and totals. Update this function to choose the non-latest entry with the
highest share/request count from packageData.cdn.versions, rather than using
find. Keep the latestVersion exclusion, but compare the remaining versions by
the same metric used elsewhere in the adoption data model so the returned
AdoptionCdnVersionLike is the actual leader.
Comment on lines
+19
to
+21
| const adoptionReportDate = new Intl.DateTimeFormat('en-US', { dateStyle: 'medium', timeStyle: 'long' }).format( | ||
| new Date(adoption.created) | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Don't stamp the adoption section with the releases timestamp.
The page now renders adoption totals and charts, but the status line still uses releasesReportDate only. If adoption.created drifts, the dashboard advertises the wrong freshness for the new data.
Suggested fix
- <p nve-text="body muted sm" nve-layout="pad-y:xs">
- Metrics last updated on ${releasesReportDate}.
- </p>
+ <p nve-text="body muted sm" nve-layout="pad-y:xs">
+ Releases last updated on ${releasesReportDate}. Adoption data last updated on ${adoptionReportDate}.
+ </p>Also applies to: 106-108
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@projects/site/src/docs/metrics/index.11ty.js` around lines 19 - 21, The
adoption status line is reusing the releases timestamp instead of the adoption
data timestamp, so the metrics page can show the wrong freshness for adoption
totals/charts. Update the metrics template in the docs page to use the
adoption-specific formatted date variable created from adoption.created (the
adoptionReportDate symbol) wherever the adoption section’s status line is
rendered, and keep releasesReportDate only for release data.
Comment on lines
+15
to
+45
| describe('release chart data', () => { | ||
| it('should create weekly cumulative release milestones', () => { | ||
| expect(getWeeklyReleaseMilestones(releaseFixture)).toEqual([ | ||
| { | ||
| weekKey: '2026-06-01', | ||
| formattedDate: 'Week of Jun 1', | ||
| releasesThisWeek: 2, | ||
| cumulativeTotal: 2, | ||
| releases: [ | ||
| { name: 'core-v2.0.0', date: '2026-06-01T10:00:00.000Z' }, | ||
| { name: 'core-v2.0.1', date: '2026-06-03T10:00:00.000Z' } | ||
| ] | ||
| }, | ||
| { | ||
| weekKey: '2026-06-08', | ||
| formattedDate: 'Week of Jun 8', | ||
| releasesThisWeek: 2, | ||
| cumulativeTotal: 4, | ||
| releases: [ | ||
| { name: 'themes-v2.0.0', date: '2026-06-08T10:00:00.000Z' }, | ||
| { name: 'styles-v2.0.0', date: '2026-06-14T10:00:00.000Z' } | ||
| ] | ||
| }, | ||
| { | ||
| weekKey: '2026-06-15', | ||
| formattedDate: 'Week of Jun 15', | ||
| releasesThisWeek: 1, | ||
| cumulativeTotal: 5, | ||
| releases: [{ name: 'core-v2.0.2', date: '2026-06-15T10:00:00.000Z' }] | ||
| } | ||
| ]); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Add a week-boundary regression case.
This only exercises dates that already fall neatly inside the same month. The fragile part of getWeeklyReleaseMilestones is the Monday/Sunday rollover and cross-year bucketing.
Suggested test
describe('release chart data', () => {
it('should create weekly cumulative release milestones', () => {
expect(getWeeklyReleaseMilestones(releaseFixture)).toEqual([
// ...
]);
});
+
+ it('should bucket Sunday releases into the preceding UTC week across year boundaries', () => {
+ expect(
+ getWeeklyReleaseMilestones([
+ { name: 'dec-release', date: '2026-12-31T10:00:00.000Z' },
+ { name: 'jan-release', date: '2027-01-03T10:00:00.000Z' }
+ ])
+ ).toMatchObject([
+ {
+ weekKey: '2026-12-28',
+ releasesThisWeek: 2,
+ cumulativeTotal: 2
+ }
+ ]);
+ });
});📝 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.
Suggested change
| describe('release chart data', () => { | |
| it('should create weekly cumulative release milestones', () => { | |
| expect(getWeeklyReleaseMilestones(releaseFixture)).toEqual([ | |
| { | |
| weekKey: '2026-06-01', | |
| formattedDate: 'Week of Jun 1', | |
| releasesThisWeek: 2, | |
| cumulativeTotal: 2, | |
| releases: [ | |
| { name: 'core-v2.0.0', date: '2026-06-01T10:00:00.000Z' }, | |
| { name: 'core-v2.0.1', date: '2026-06-03T10:00:00.000Z' } | |
| ] | |
| }, | |
| { | |
| weekKey: '2026-06-08', | |
| formattedDate: 'Week of Jun 8', | |
| releasesThisWeek: 2, | |
| cumulativeTotal: 4, | |
| releases: [ | |
| { name: 'themes-v2.0.0', date: '2026-06-08T10:00:00.000Z' }, | |
| { name: 'styles-v2.0.0', date: '2026-06-14T10:00:00.000Z' } | |
| ] | |
| }, | |
| { | |
| weekKey: '2026-06-15', | |
| formattedDate: 'Week of Jun 15', | |
| releasesThisWeek: 1, | |
| cumulativeTotal: 5, | |
| releases: [{ name: 'core-v2.0.2', date: '2026-06-15T10:00:00.000Z' }] | |
| } | |
| ]); | |
| describe('release chart data', () => { | |
| it('should create weekly cumulative release milestones', () => { | |
| expect(getWeeklyReleaseMilestones(releaseFixture)).toEqual([ | |
| { | |
| weekKey: '2026-06-01', | |
| formattedDate: 'Week of Jun 1', | |
| releasesThisWeek: 2, | |
| cumulativeTotal: 2, | |
| releases: [ | |
| { name: 'core-v2.0.0', date: '2026-06-01T10:00:00.000Z' }, | |
| { name: 'core-v2.0.1', date: '2026-06-03T10:00:00.000Z' } | |
| ] | |
| }, | |
| { | |
| weekKey: '2026-06-08', | |
| formattedDate: 'Week of Jun 8', | |
| releasesThisWeek: 2, | |
| cumulativeTotal: 4, | |
| releases: [ | |
| { name: 'themes-v2.0.0', date: '2026-06-08T10:00:00.000Z' }, | |
| { name: 'styles-v2.0.0', date: '2026-06-14T10:00:00.000Z' } | |
| ] | |
| }, | |
| { | |
| weekKey: '2026-06-15', | |
| formattedDate: 'Week of Jun 15', | |
| releasesThisWeek: 1, | |
| cumulativeTotal: 5, | |
| releases: [{ name: 'core-v2.0.2', date: '2026-06-15T10:00:00.000Z' }] | |
| } | |
| ]); | |
| }); | |
| it('should bucket Sunday releases into the preceding UTC week across year boundaries', () => { | |
| expect( | |
| getWeeklyReleaseMilestones([ | |
| { name: 'dec-release', date: '2026-12-31T10:00:00.000Z' }, | |
| { name: 'jan-release', date: '2027-01-03T10:00:00.000Z' } | |
| ]) | |
| ).toMatchObject([ | |
| { | |
| weekKey: '2026-12-28', | |
| releasesThisWeek: 2, | |
| cumulativeTotal: 2 | |
| } | |
| ]); | |
| }); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@projects/site/src/docs/metrics/release-data.test.ts` around lines 15 - 45,
The current `getWeeklyReleaseMilestones` test only covers straightforward
same-month dates, so add a regression case in `release-data.test.ts` that
exercises a week boundary with releases on a Sunday/Monday rollover and across a
month or year change. Extend the `describe('release chart data')` coverage with
a new `it(...)` that asserts the correct `weekKey`, `formattedDate`,
`releasesThisWeek`, and `cumulativeTotal` for `getWeeklyReleaseMilestones`,
using `releaseFixture`-style inputs that prove Monday-based bucketing stays
correct across boundaries.
Comment on lines
+23
to
+27
| const WEEK_START_FORMATTER = new Intl.DateTimeFormat('en-US', { | ||
| month: 'short', | ||
| day: 'numeric', | ||
| timeZone: 'UTC' | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Include the year in weekly labels.
formattedDate is reused for the chart axis and tooltip title, but it currently drops the year. Once this series spans multiple calendar years, users will see repeated labels like Week of Jun 1 for different points.
Suggested fix
const WEEK_START_FORMATTER = new Intl.DateTimeFormat('en-US', {
month: 'short',
day: 'numeric',
+ year: 'numeric',
timeZone: 'UTC'
});Also applies to: 42-43
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@projects/site/src/docs/metrics/release-data.ts` around lines 23 - 27, The
weekly date label formatter in WEEK_START_FORMATTER drops the year, causing
repeated labels across calendar years. Update the formatting used by
formattedDate in release-data.ts so week labels include the year for both the
chart axis and tooltip title, and apply the same change anywhere the shared
weekly label format is reused.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
projects/site/src/docs/metrics/adoption-data.test.ts (1)
4-154: 📐 Maintainability & Code Quality | 🟠 MajorAdd a case with unsorted CDN versions and release markers.
This suite still passes even with the
getTopNonLatestVersionData()bug because the fixture already puts the leading non-latest version first, andgetReleaseAdoptionTimeline()is still unexercised. Add one unsortedcdn.versionscase and one release-marker case so those paths stop regressing silently.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@projects/site/src/docs/metrics/adoption-data.test.ts` around lines 4 - 154, The adoption-data tests do not cover the buggy ordering and release-marker paths, so add coverage in adoption-data.test for getTopNonLatestVersionData and getReleaseAdoptionTimeline. Extend the existing adoptionFixture with one package whose cdn.versions are deliberately unsorted so the top non-latest version must be derived correctly, and add a separate case with publishDates/release markers to exercise the timeline logic. Keep the expectations aligned with getCdnVersionShareRows, getPackageDownloadTrend, and getChannelMixPoints so regressions in those helper paths are caught.projects/site/src/docs/metrics/adoption-data.ts (1)
117-121: 🎯 Functional Correctness | 🟠 MajorPick the actual leading non-latest CDN version.
This still returns the first non-latest entry, not the non-latest entry with the highest adoption.
getCdnVersionShareRow()then reports the wrong version name, share, and request totals wheneverpackageData.cdn.versionsis unsorted.Suggested fix
function getTopNonLatestVersionData( packageData: AdoptionPackageLike, latestVersion: string ): AdoptionCdnVersionLike | undefined { - return packageData.cdn.versions.find(version => version.version !== latestVersion); + return packageData.cdn.versions + .filter(version => version.version !== latestVersion) + .sort((a, b) => b.share - a.share || b.total - a.total)[0]; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@projects/site/src/docs/metrics/adoption-data.ts` around lines 117 - 121, The getTopNonLatestVersionData helper currently returns the first non-latest entry from packageData.cdn.versions, which can make getCdnVersionShareRow report the wrong version when the list is unsorted. Update getTopNonLatestVersionData to select the non-latest AdoptionCdnVersionLike with the highest adoption/share instead of using find, and keep the existing latestVersion filter so the downstream version name, share, and request totals come from the true leading non-latest CDN version.projects/site/src/docs/metrics/index.11ty.js (1)
14-21: 🎯 Functional Correctness | 🟡 MinorUse
adoptionReportDatein the page or drop it.The new adoption timestamp is still only computed here. Because the template status line continues to render
releasesReportDate, the adoption charts/totals advertise the wrong freshness while this variable stays dead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@projects/site/src/docs/metrics/index.11ty.js` around lines 14 - 21, The adoption freshness timestamp is computed in the metrics page but never used, while the status line still reflects the releases timestamp. Update the docs template in the metrics page so the rendered freshness/status text uses adoptionReportDate where the adoption data is being presented, or remove adoptionReportDate entirely if it is not needed. Use the metrics page’s existing data-loading and formatting logic around AdoptionService.getData and the report date variables to locate the fix.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@projects/internals/metadata/src/services/releases.service.test.ts`:
- Around line 13-14: The test in releases.service.test.ts is pinned to a
specific generated release tag, which makes it brittle against normal metadata
refreshes. Update the assertion in the releases service test to validate the
deserialized release record shape and expected fields through the releases
service output, rather than checking for a hardcoded package name in
releases.data.some. Keep the length check if needed, but replace the
version-specific name assertion with generic shape-based expectations tied to
releases.data.
In `@projects/internals/metadata/src/tasks/adoption.utils.ts`:
- Around line 178-209: fetchPublicJson currently awaits fetch without any
deadline, so a stalled public API request can block adoption updates
indefinitely. Add a timeout around the network read in fetchPublicJson, using an
AbortController or equivalent, and ensure the request is aborted and reported as
a failed source with a clear timeout message. Keep the existing ok/error shape
and preserve the current handling for non-ok responses and caught network
errors.
- Around line 146-164: The catch in getPackageFile is swallowing all failures
and treating malformed or unreadable package.json files as absent packages.
Update getPackageFile so it only returns null for a missing manifest (ENOENT)
when reading the package.json URL, and rethrow any parse or other I/O errors.
Keep the existing package filtering logic in getPackageFile, but make the error
handling precise so real `@nvidia-elements/`* workspaces are not silently dropped.
---
Duplicate comments:
In `@projects/site/src/docs/metrics/adoption-data.test.ts`:
- Around line 4-154: The adoption-data tests do not cover the buggy ordering and
release-marker paths, so add coverage in adoption-data.test for
getTopNonLatestVersionData and getReleaseAdoptionTimeline. Extend the existing
adoptionFixture with one package whose cdn.versions are deliberately unsorted so
the top non-latest version must be derived correctly, and add a separate case
with publishDates/release markers to exercise the timeline logic. Keep the
expectations aligned with getCdnVersionShareRows, getPackageDownloadTrend, and
getChannelMixPoints so regressions in those helper paths are caught.
In `@projects/site/src/docs/metrics/adoption-data.ts`:
- Around line 117-121: The getTopNonLatestVersionData helper currently returns
the first non-latest entry from packageData.cdn.versions, which can make
getCdnVersionShareRow report the wrong version when the list is unsorted. Update
getTopNonLatestVersionData to select the non-latest AdoptionCdnVersionLike with
the highest adoption/share instead of using find, and keep the existing
latestVersion filter so the downstream version name, share, and request totals
come from the true leading non-latest CDN version.
In `@projects/site/src/docs/metrics/index.11ty.js`:
- Around line 14-21: The adoption freshness timestamp is computed in the metrics
page but never used, while the status line still reflects the releases
timestamp. Update the docs template in the metrics page so the rendered
freshness/status text uses adoptionReportDate where the adoption data is being
presented, or remove adoptionReportDate entirely if it is not needed. Use the
metrics page’s existing data-loading and formatting logic around
AdoptionService.getData and the report date variables to locate the fix.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 747b1c9f-5296-4bce-b07b-affa57aedd3a
📒 Files selected for processing (23)
projects/internals/metadata/README.mdprojects/internals/metadata/package.jsonprojects/internals/metadata/src/index.tsprojects/internals/metadata/src/services/adoption.service.test.tsprojects/internals/metadata/src/services/adoption.service.tsprojects/internals/metadata/src/services/releases.service.test.tsprojects/internals/metadata/src/tasks/adoption.tsprojects/internals/metadata/src/tasks/adoption.utils.test.tsprojects/internals/metadata/src/tasks/adoption.utils.tsprojects/internals/metadata/src/types.tsprojects/internals/metadata/static/adoption.jsonprojects/internals/metadata/static/releases.jsonprojects/internals/metadata/vitest.config.tsprojects/site/package.jsonprojects/site/src/_11ty/layouts/common.jsprojects/site/src/docs/metrics/adoption-data.test.tsprojects/site/src/docs/metrics/adoption-data.tsprojects/site/src/docs/metrics/adoption.tsprojects/site/src/docs/metrics/bundle-explorer.11ty.jsprojects/site/src/docs/metrics/index.11ty.jsprojects/site/src/docs/metrics/index.tsprojects/site/src/docs/metrics/release-data.test.tsprojects/site/src/docs/metrics/release-data.ts
Comment on lines
+13
to
+14
| expect(releases.data.length).toBeGreaterThan(1); | ||
| expect(releases.data.some(release => release.name === '@nvidia-elements/core-v2.0.2')).toBe(true); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Avoid pinning this service test to one generated release tag.
Line 14 couples the test to the current contents of static/releases.json, so a normal metadata refresh can fail CI even when deserialization still works. Assert the record shape instead of a specific versioned package name.
Suggested change
- expect(releases.data.length).toBeGreaterThan(1);
- expect(releases.data.some(release => release.name === '`@nvidia-elements/core-v2.0.2`')).toBe(true);
+ expect(releases.data.length).toBeGreaterThan(0);
+ expect(releases.data).toEqual(
+ expect.arrayContaining([
+ expect.objectContaining({
+ name: expect.any(String),
+ date: expect.any(String)
+ })
+ ])
+ );📝 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.
Suggested change
| expect(releases.data.length).toBeGreaterThan(1); | |
| expect(releases.data.some(release => release.name === '@nvidia-elements/core-v2.0.2')).toBe(true); | |
| expect(releases.data.length).toBeGreaterThan(0); | |
| expect(releases.data).toEqual( | |
| expect.arrayContaining([ | |
| expect.objectContaining({ | |
| name: expect.any(String), | |
| date: expect.any(String) | |
| }) | |
| ]) | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@projects/internals/metadata/src/services/releases.service.test.ts` around
lines 13 - 14, The test in releases.service.test.ts is pinned to a specific
generated release tag, which makes it brittle against normal metadata refreshes.
Update the assertion in the releases service test to validate the deserialized
release record shape and expected fields through the releases service output,
rather than checking for a hardcoded package name in releases.data.some. Keep
the length check if needed, but replace the version-specific name assertion with
generic shape-based expectations tied to releases.data.
Comment on lines
+146
to
+164
| async function getPackageFile(directoryUrl: URL): Promise<WorkspacePackage | null> { | ||
| try { | ||
| const packageJson: unknown = JSON.parse(await readFile(new URL('package.json', directoryUrl), 'utf8')); | ||
| const name = getString(packageJson, 'name'); | ||
| const workspaceVersion = getString(packageJson, 'version'); | ||
| const isPrivate = isRecord(packageJson) && packageJson.private === true; | ||
|
|
||
| if (!name?.startsWith('@nvidia-elements/') || !workspaceVersion || isPrivate) { | ||
| return null; | ||
| } | ||
|
|
||
| return { | ||
| name, | ||
| workspaceVersion | ||
| }; | ||
| } catch { | ||
| // Not every projects directory entry is an npm package. | ||
| return null; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Only ignore missing manifests here.
This catch turns malformed or unreadable package.json files into “not a package”, so a real @nvidia-elements/* workspace can silently disappear from the adoption snapshot and undercount totals.packages. Only treat ENOENT as skippable and rethrow parse/I/O errors.
Suggested fix
async function getPackageFile(directoryUrl: URL): Promise<WorkspacePackage | null> {
try {
const packageJson: unknown = JSON.parse(await readFile(new URL('package.json', directoryUrl), 'utf8'));
const name = getString(packageJson, 'name');
const workspaceVersion = getString(packageJson, 'version');
const isPrivate = isRecord(packageJson) && packageJson.private === true;
@@
return {
name,
workspaceVersion
};
- } catch {
- // Not every projects directory entry is an npm package.
- return null;
+ } catch (error) {
+ if (error instanceof Error && 'code' in error && error.code === 'ENOENT') {
+ // Not every projects directory entry is an npm package.
+ return null;
+ }
+
+ throw error;
}
}📝 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.
Suggested change
| async function getPackageFile(directoryUrl: URL): Promise<WorkspacePackage | null> { | |
| try { | |
| const packageJson: unknown = JSON.parse(await readFile(new URL('package.json', directoryUrl), 'utf8')); | |
| const name = getString(packageJson, 'name'); | |
| const workspaceVersion = getString(packageJson, 'version'); | |
| const isPrivate = isRecord(packageJson) && packageJson.private === true; | |
| if (!name?.startsWith('@nvidia-elements/') || !workspaceVersion || isPrivate) { | |
| return null; | |
| } | |
| return { | |
| name, | |
| workspaceVersion | |
| }; | |
| } catch { | |
| // Not every projects directory entry is an npm package. | |
| return null; | |
| } | |
| async function getPackageFile(directoryUrl: URL): Promise<WorkspacePackage | null> { | |
| try { | |
| const packageJson: unknown = JSON.parse(await readFile(new URL('package.json', directoryUrl), 'utf8')); | |
| const name = getString(packageJson, 'name'); | |
| const workspaceVersion = getString(packageJson, 'version'); | |
| const isPrivate = isRecord(packageJson) && packageJson.private === true; | |
| if (!name?.startsWith('`@nvidia-elements/`') || !workspaceVersion || isPrivate) { | |
| return null; | |
| } | |
| return { | |
| name, | |
| workspaceVersion | |
| }; | |
| } catch (error) { | |
| if (error instanceof Error && 'code' in error && error.code === 'ENOENT') { | |
| // Not every projects directory entry is an npm package. | |
| return null; | |
| } | |
| throw error; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@projects/internals/metadata/src/tasks/adoption.utils.ts` around lines 146 -
164, The catch in getPackageFile is swallowing all failures and treating
malformed or unreadable package.json files as absent packages. Update
getPackageFile so it only returns null for a missing manifest (ENOENT) when
reading the package.json URL, and rethrow any parse or other I/O errors. Keep
the existing package filtering logic in getPackageFile, but make the error
handling precise so real `@nvidia-elements/`* workspaces are not silently dropped.
Comment on lines
+178
to
+209
| export async function fetchPublicJson( | ||
| url: string, | ||
| source: AdoptionSource, | ||
| headers: Record<string, string> = {} | ||
| ): Promise<PublicJsonResult> { | ||
| try { | ||
| const response = await fetch(url, { headers }); | ||
|
|
||
| if (!response.ok) { | ||
| const statusText = response.statusText.trim(); | ||
| return { | ||
| ok: false, | ||
| error: getSourceError( | ||
| source, | ||
| response.status, | ||
| statusText ? `${response.status} ${statusText}` : `HTTP ${response.status}` | ||
| ) | ||
| }; | ||
| } | ||
|
|
||
| return { | ||
| ok: true, | ||
| data: await response.json(), | ||
| headers: response.headers | ||
| }; | ||
| } catch (error) { | ||
| // Network endpoints can fail independently of the generator process. | ||
| return { | ||
| ok: false, | ||
| error: getSourceError(source, null, error instanceof Error ? error.message : 'unknown network error') | ||
| }; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Bound these public API calls with a timeout.
Every network read here can hang indefinitely. One stalled npm/jsDelivr/GitHub socket blocks the whole adoption task and prevents adoption.json from being refreshed.
Suggested fix
export async function fetchPublicJson(
url: string,
source: AdoptionSource,
headers: Record<string, string> = {}
): Promise<PublicJsonResult> {
+ const controller = new AbortController();
+ const timeoutId = setTimeout(() => controller.abort(), 15_000);
+
try {
- const response = await fetch(url, { headers });
+ const response = await fetch(url, {
+ headers,
+ signal: controller.signal
+ });
@@
} catch (error) {
// Network endpoints can fail independently of the generator process.
return {
ok: false,
- error: getSourceError(source, null, error instanceof Error ? error.message : 'unknown network error')
+ error: getSourceError(
+ source,
+ null,
+ error instanceof Error && error.name === 'AbortError'
+ ? 'request timed out'
+ : error instanceof Error
+ ? error.message
+ : 'unknown network error'
+ )
};
+ } finally {
+ clearTimeout(timeoutId);
}
}📝 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.
Suggested change
| export async function fetchPublicJson( | |
| url: string, | |
| source: AdoptionSource, | |
| headers: Record<string, string> = {} | |
| ): Promise<PublicJsonResult> { | |
| try { | |
| const response = await fetch(url, { headers }); | |
| if (!response.ok) { | |
| const statusText = response.statusText.trim(); | |
| return { | |
| ok: false, | |
| error: getSourceError( | |
| source, | |
| response.status, | |
| statusText ? `${response.status} ${statusText}` : `HTTP ${response.status}` | |
| ) | |
| }; | |
| } | |
| return { | |
| ok: true, | |
| data: await response.json(), | |
| headers: response.headers | |
| }; | |
| } catch (error) { | |
| // Network endpoints can fail independently of the generator process. | |
| return { | |
| ok: false, | |
| error: getSourceError(source, null, error instanceof Error ? error.message : 'unknown network error') | |
| }; | |
| } | |
| export async function fetchPublicJson( | |
| url: string, | |
| source: AdoptionSource, | |
| headers: Record<string, string> = {} | |
| ): Promise<PublicJsonResult> { | |
| const controller = new AbortController(); | |
| const timeoutId = setTimeout(() => controller.abort(), 15_000); | |
| try { | |
| const response = await fetch(url, { | |
| headers, | |
| signal: controller.signal | |
| }); | |
| if (!response.ok) { | |
| const statusText = response.statusText.trim(); | |
| return { | |
| ok: false, | |
| error: getSourceError( | |
| source, | |
| response.status, | |
| statusText ? `${response.status} ${statusText}` : `HTTP ${response.status}` | |
| ) | |
| }; | |
| } | |
| return { | |
| ok: true, | |
| data: await response.json(), | |
| headers: response.headers | |
| }; | |
| } catch (error) { | |
| // Network endpoints can fail independently of the generator process. | |
| return { | |
| ok: false, | |
| error: getSourceError( | |
| source, | |
| null, | |
| error instanceof Error && error.name === 'AbortError' | |
| ? 'request timed out' | |
| : error instanceof Error | |
| ? error.message | |
| : 'unknown network error' | |
| ) | |
| }; | |
| } finally { | |
| clearTimeout(timeoutId); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@projects/internals/metadata/src/tasks/adoption.utils.ts` around lines 178 -
209, fetchPublicJson currently awaits fetch without any deadline, so a stalled
public API request can block adoption updates indefinitely. Add a timeout around
the network read in fetchPublicJson, using an AbortController or equivalent, and
ensure the request is aborted and reported as a failed source with a clear
timeout message. Keep the existing ok/error shape and preserve the current
handling for non-ok responses and caught network errors.
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
2 participants
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.
AdoptionServiceto fetch and cache public adoption metrics from npm, jsDelivr, and GitHub.adoption.tstask to generate adoption metrics and save them tostatic/adoption.json.package.jsonscripts to include a newgenerate:adoptioncommand.Summary by CodeRabbit