chore(docs): whats new page and skill#187
Open
coryrylan wants to merge 1 commit into
Open
Conversation
github-actions
Bot
added
scope(ci)
scope(internals)
scope(docs)
dependencies
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (27)
📝 WalkthroughWalkthroughAdds a local release-evidence collector and skill guide, introduces monthly What’s New pages with RSS/Atom feeds, updates metadata and sitemap generation, limits minification to HTML, and improves registry fetch error context. ChangesRelease evidence workflow
What’s New site publishing
Registry fallback message
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Eleventy
participant updatesFeedPlugin
participant UpdateCollections
participant FeedTemplates
participant PublicOutput
Eleventy->>updatesFeedPlugin: register update collections and templates
updatesFeedPlugin->>UpdateCollections: select and sort tagged updates
UpdateCollections->>FeedTemplates: provide normalized entry data
FeedTemplates-->>Eleventy: render RSS and Atom XML
Eleventy->>updatesFeedPlugin: invoke after hook
updatesFeedPlugin->>PublicOutput: write feed.xml and atom.xml
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. 🔧 ESLint
projects/internals/tools/src/api/utils.test.tsParsing error: error TS5012: Cannot read file '/tsconfig.json': ENOENT: no such file or directory, open '/tsconfig.json'. projects/site/eleventy.config.jsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. projects/site/package.jsonESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox.
Comment |
There was a problem hiding this comment.
Pull request overview
This PR enhances the Elements documentation site with a new “What’s New” section (index + monthly posts), adds an updates RSS/Atom feed for discovery and syndication, and introduces a new summarize-releases agent skill (plus a local evidence-collection script) to generate monthly release summaries from local git tags/changelogs.
Changes:
- Add “What’s New” index + monthly update pages, with a shared layout that renders update chrome and recent-updates navigation.
- Add updates RSS/Atom feed generation and publish the generated feeds to the Vite public output directory.
- Improve metadata/date handling (shared date normalization utilities, JSON-LD schema tweaks for update posts, sitemap lastmod behavior) and tighten HTML minification to avoid non-HTML outputs.
Reviewed changes
Copilot reviewed 23 out of 24 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| projects/site/src/docs/whats-new/index.11ty.js | Adds the “What’s New” landing page that lists update cards and links to the feeds. |
| projects/site/src/docs/whats-new/06-2026.md | Adds June 2026 monthly update content and metadata. |
| projects/site/src/docs/whats-new/05-2026.md | Adds May 2026 monthly update content and metadata. |
| projects/site/src/docs/whats-new/04-2026.md | Adds April 2026 monthly update content and metadata. |
| projects/site/src/_11ty/utils/content-dates.js | Introduces shared date normalization utilities for metadata/sitemap/feed generation. |
| projects/site/src/_11ty/transforms/html-minify.test.ts | Adds tests for minifying HTML while preserving XML outputs. |
| projects/site/src/_11ty/transforms/html-minify.js | Restricts minification to HTML outputs only (avoids minifying XML feeds). |
| projects/site/src/_11ty/plugins/updates-feed.test.ts | Adds tests for updates feed collection behavior and publishing feeds to the public dir. |
| projects/site/src/_11ty/plugins/updates-feed.js | Adds Eleventy plugin that generates RSS + Atom feeds from the updates collection. |
| projects/site/src/_11ty/plugins/sitemap-xml.test.ts | Extends sitemap tests to validate lastmod emission behavior. |
| projects/site/src/_11ty/plugins/sitemap-xml.js | Updates sitemap generation to include lastmod only from explicit modification metadata/structured data. |
| projects/site/src/_11ty/layouts/whats-new.test.ts | Adds tests for the shared What’s New layout (dates, recent updates, feed link). |
| projects/site/src/_11ty/layouts/whats-new.11ty.js | Adds shared layout helpers for update dates, recent updates, and RSS link rendering. |
| projects/site/src/_11ty/layouts/page.11ty.js | Updates header branding text/aria-label for the site header. |
| projects/site/src/_11ty/layouts/metadata.test.ts | Updates metadata tests for feed discovery links and BlogPosting schema for updates. |
| projects/site/src/_11ty/layouts/metadata.js | Refactors date handling via shared utils and emits BlogPosting schema for update posts. |
| projects/site/src/_11ty/layouts/docs.css | Minor layout tweak (width: 100% on specific heading styling). |
| projects/site/src/_11ty/layouts/common.js | Adds feed discovery <link rel="alternate"> tags and updates docs nav to include What’s New. |
| projects/site/package.json | Adds @11ty/eleventy-plugin-rss dependency used by the new feed plugin. |
| projects/site/eleventy.config.js | Registers the updates feed plugin in Eleventy config. |
| projects/internals/tools/src/api/utils.ts | Improves npm registry fetch error messaging by including the resolved URL. |
| pnpm-lock.yaml | Locks the new RSS plugin dependency and related dependency updates. |
| .agents/skills/summarize-releases/SKILL.md | Adds the new summarize-releases skill documentation and workflow steps. |
| .agents/skills/summarize-releases/scripts/collect-releases.js | Adds the collector script that builds the evidence packet from local tags/changelogs. |
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 @.agents/skills/summarize-releases/scripts/collect-releases.js:
- Around line 468-503: Document each exported function listed in the review,
including input types, return values, and relevant constraints; make
createEvidence explicitly describe its local Git clone dependency and evidence
packet shape. Add structured, leveled logging at warning/skip decision points
such as the warnings.push calls in createEvidence, while preserving the warnings
in the returned evidence object.
- Around line 468-503: Add a companion test file for the parsing and date helper
functions used by createEvidence, covering month-rollover period calculations,
changelog-heading extraction, and commit-link parsing. Exercise representative
normal and boundary inputs directly through the exported or accessible helper
symbols, without changing createEvidence behavior.
- Around line 187-212: Update parseReleaseNoteCommits so the subject is derived
from a version of each changelog line with all commit-link markdown removed,
rather than removing only the current match[0]. Reuse that cleaned line for
every match while preserving the existing bullet, trailing-whitespace, SHA, and
URL handling.
In `@projects/internals/tools/src/api/utils.ts`:
- Line 217: Update both failure-path assertions in the tests for the API fetch
utility to expect the singular message “Could not fetch latest version from” and
include the requested URL context, matching the message constructed in the fetch
failure path.
In `@projects/site/src/_11ty/utils/content-dates.js`:
- Around line 1-6: Update normalizeContentDate to validate Date instances with a
non-NaN timestamp before calling toISOString(), returning null for invalid Date
objects while preserving the existing behavior for valid Date instances and date
strings.
🪄 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: 1490f0ee-99c8-4650-8229-04e461a5c9b1
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (23)
.agents/skills/summarize-releases/SKILL.md.agents/skills/summarize-releases/scripts/collect-releases.jsprojects/internals/tools/src/api/utils.tsprojects/site/eleventy.config.jsprojects/site/package.jsonprojects/site/src/_11ty/layouts/common.jsprojects/site/src/_11ty/layouts/docs.cssprojects/site/src/_11ty/layouts/metadata.jsprojects/site/src/_11ty/layouts/metadata.test.tsprojects/site/src/_11ty/layouts/page.11ty.jsprojects/site/src/_11ty/layouts/whats-new.11ty.jsprojects/site/src/_11ty/layouts/whats-new.test.tsprojects/site/src/_11ty/plugins/sitemap-xml.jsprojects/site/src/_11ty/plugins/sitemap-xml.test.tsprojects/site/src/_11ty/plugins/updates-feed.jsprojects/site/src/_11ty/plugins/updates-feed.test.tsprojects/site/src/_11ty/transforms/html-minify.jsprojects/site/src/_11ty/transforms/html-minify.test.tsprojects/site/src/_11ty/utils/content-dates.jsprojects/site/src/docs/whats-new/04-2026.mdprojects/site/src/docs/whats-new/05-2026.mdprojects/site/src/docs/whats-new/06-2026.mdprojects/site/src/docs/whats-new/index.11ty.js
coryrylan
force-pushed
the
topic-summary-skill
branch
from
July 22, 2026 21:59
cddbaa2 to
688eb6a
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 26 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Comments suppressed due to low confidence (1)
projects/site/src/docs/whats-new/index.11ty.js:34
- The inline CSS uses a nested style rule (
* { ... }inside.update-card { ... }). This relies on CSS Nesting support and can fail silently in environments that don’t support it, leaving descendants without the intended cursor styling. Use an explicit descendant selector instead to keep the output broadly compatible.
.update-card {
cursor: pointer;
* {
cursor: pointer;
}
}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/site/src/_11ty/layouts/common.js`:
- Around line 174-176: Align the parent navigation entry in the common layout by
either changing its href to the About landing URL or renaming its “About” label
to “What’s New” to match `/docs/whats-new/`; keep the existing child navigation
and selection behavior unchanged.
In `@projects/site/src/_11ty/layouts/whats-new.11ty.js`:
- Around line 42-54: Update renderUpdateDates in
projects/site/src/_11ty/layouts/whats-new.11ty.js (lines 42-54) to render
“Updated” only when dateModified is chronologically later than datePublished,
while preserving publication-only output when it is earlier or equal. Add a
modified-before-published test in
projects/site/src/_11ty/layouts/whats-new.test.ts (lines 46-54) asserting that
“Updated” is omitted.
- Around line 7-15: The RSS link in renderUpdatesFeedLink currently ignores the
deployed base path; update its href to use the existing getSiteUrl or equivalent
base-path-aware helper while preserving the feed.xml target. In
projects/site/src/_11ty/layouts/whats-new.11ty.js lines 7-15, make this URL
change; in projects/site/src/_11ty/layouts/whats-new.test.ts lines 87-93, update
the assertion to expect the deployed base-path-aware feed URL.
In `@projects/site/src/_11ty/plugins/sitemap-xml.js`:
- Around line 30-32: Update the dateModified extraction in the sitemap
generation loop around JSON_LD_SCRIPT to allow optional whitespace after the
property name’s colon, while preserving the existing normalized date behavior.
Add a regression case covering pretty-printed JSON-LD with `"dateModified":
"..."` and verify that the generated sitemap includes the corresponding lastmod
value.
🪄 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: e8169cc4-2dca-45ac-a518-0d3b6c7f8500
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (25)
.agents/skills/summarize-releases/SKILL.md.agents/skills/summarize-releases/scripts/collect-releases.jsprojects/internals/tools/src/api/utils.test.tsprojects/internals/tools/src/api/utils.tsprojects/site/eleventy.config.jsprojects/site/package.jsonprojects/site/src/_11ty/layouts/common.jsprojects/site/src/_11ty/layouts/docs.cssprojects/site/src/_11ty/layouts/metadata.jsprojects/site/src/_11ty/layouts/metadata.test.tsprojects/site/src/_11ty/layouts/page.11ty.jsprojects/site/src/_11ty/layouts/whats-new.11ty.jsprojects/site/src/_11ty/layouts/whats-new.test.tsprojects/site/src/_11ty/plugins/sitemap-xml.jsprojects/site/src/_11ty/plugins/sitemap-xml.test.tsprojects/site/src/_11ty/plugins/updates-feed.jsprojects/site/src/_11ty/plugins/updates-feed.test.tsprojects/site/src/_11ty/transforms/html-minify.jsprojects/site/src/_11ty/transforms/html-minify.test.tsprojects/site/src/_11ty/utils/content-dates.jsprojects/site/src/_11ty/utils/content-dates.test.tsprojects/site/src/docs/whats-new/04-2026.mdprojects/site/src/docs/whats-new/05-2026.mdprojects/site/src/docs/whats-new/06-2026.mdprojects/site/src/docs/whats-new/index.11ty.js
Comment on lines
+30
to
+32
| for (const script of (result.content ?? '').matchAll(JSON_LD_SCRIPT)) { | ||
| const structuredDate = /"dateModified":"([^"]+)"/.exec(script[1])?.[1]; | ||
| if (structuredDate) return normalizeContentDate(structuredDate); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Accept valid whitespace in JSON-LD dateModified.
Line 31 only matches minified JSON. Pretty-printed JSON-LD such as "dateModified": "..." silently omits <lastmod>.
Proposed fix
- const structuredDate = /"dateModified":"([^"]+)"/.exec(script[1])?.[1];
+ const structuredDate = /"dateModified"\s*:\s*"([^"]+)"/.exec(script[1])?.[1];Add a regression case using formatted JSON-LD.
📝 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
| for (const script of (result.content ?? '').matchAll(JSON_LD_SCRIPT)) { | |
| const structuredDate = /"dateModified":"([^"]+)"/.exec(script[1])?.[1]; | |
| if (structuredDate) return normalizeContentDate(structuredDate); | |
| for (const script of (result.content ?? '').matchAll(JSON_LD_SCRIPT)) { | |
| const structuredDate = /"dateModified"\s*:\s*"([^"]+)"/.exec(script[1])?.[1]; | |
| if (structuredDate) return normalizeContentDate(structuredDate); |
🧰 Tools
🪛 OpenGrep (1.25.0)
[ERROR] 31-31: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🤖 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/_11ty/plugins/sitemap-xml.js` around lines 30 - 32, Update
the dateModified extraction in the sitemap generation loop around JSON_LD_SCRIPT
to allow optional whitespace after the property name’s colon, while preserving
the existing normalized date behavior. Add a regression case covering
pretty-printed JSON-LD with `"dateModified": "..."` and verify that the
generated sitemap includes the corresponding lastmod value.
johnyanarella
approved these changes
Jul 24, 2026
coryrylan
force-pushed
the
topic-summary-skill
branch
from
July 24, 2026 04:06
688eb6a to
a5495e0
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 26 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Comments suppressed due to low confidence (1)
projects/site/src/docs/whats-new/index.11ty.js:30
- The CSS in this <style> block uses nested selectors (
* { ... }inside.update-card { ... }), which is not valid in standard CSS parsing and will be ignored by browsers without a preprocessor / correct CSS Nesting syntax. Flatten the selector so descendant cursor styles apply reliably.
* {
cursor: pointer;
}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/site/src/_11ty/transforms/html-minify.js`:
- Around line 6-16: Add tests for htmlMinifyTransform that omit outputPath and
exercise the isHtmlOutput content-sniffing fallback: verify full HTML content is
minified while non-document content remains unchanged. Keep the existing
explicit-outputPath coverage intact.
🪄 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: 623b3fc6-5b05-48a1-a407-e1d68656705c
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (25)
.agents/skills/summarize-releases/SKILL.md.agents/skills/summarize-releases/scripts/collect-releases.jsprojects/internals/tools/src/api/utils.test.tsprojects/internals/tools/src/api/utils.tsprojects/site/eleventy.config.jsprojects/site/package.jsonprojects/site/src/_11ty/layouts/common.jsprojects/site/src/_11ty/layouts/docs.cssprojects/site/src/_11ty/layouts/metadata.jsprojects/site/src/_11ty/layouts/metadata.test.tsprojects/site/src/_11ty/layouts/page.11ty.jsprojects/site/src/_11ty/layouts/whats-new.11ty.jsprojects/site/src/_11ty/layouts/whats-new.test.tsprojects/site/src/_11ty/plugins/sitemap-xml.jsprojects/site/src/_11ty/plugins/sitemap-xml.test.tsprojects/site/src/_11ty/plugins/updates-feed.jsprojects/site/src/_11ty/plugins/updates-feed.test.tsprojects/site/src/_11ty/transforms/html-minify.jsprojects/site/src/_11ty/transforms/html-minify.test.tsprojects/site/src/_11ty/utils/content-dates.jsprojects/site/src/_11ty/utils/content-dates.test.tsprojects/site/src/docs/whats-new/04-2026.mdprojects/site/src/docs/whats-new/05-2026.mdprojects/site/src/docs/whats-new/06-2026.mdprojects/site/src/docs/whats-new/index.11ty.js
coryrylan
force-pushed
the
topic-summary-skill
branch
from
July 24, 2026 04:27
a5495e0 to
8d78420
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 28 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Comments suppressed due to low confidence (1)
projects/site/src/docs/whats-new/index.11ty.js:31
- The nested
* { ... }rule uses CSS nesting syntax, which will be ignored in browsers that don’t support CSS nesting (and may invalidate the whole.update-cardrule block depending on parser). Use a standard descendant selector instead so the cursor styling is applied consistently.
* {
cursor: pointer;
}
}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 @.agents/skills/summarize-releases/scripts/collect-releases.js:
- Around line 187-212: Update parseReleaseNoteCommits so each line’s subject is
computed by removing all commit-link matches once, then reuse that cleaned
subject for every match on the line; do not derive it by replacing only the
current match[0]. Preserve the existing repository escaping and linkPattern
construction, since REPOSITORY is fixed and safely escaped.
- Around line 468-503: Document the exported/orchestration functions parseArgs,
getPeriod, getPageMetadata, parseElementsTag, parseConventionalCommit,
extractChangelogSection, parseReleaseNoteCommits, collectGitReleases,
loadGitCommit, collectChanges, createEvidence, and formatMarkdown with their
inputs, outputs, and constraints. Add structured, leveled logging at
warning/skip decisions currently recorded through warnings.push(...), while
preserving the returned warnings. Add companion tests covering parsing and
date-helper behavior, including month rollover, changelog-heading extraction,
and commit-link parsing.
🪄 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: feb56008-68ed-4f34-8444-c023b656e5be
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (27)
.agents/skills/summarize-releases/SKILL.md.agents/skills/summarize-releases/scripts/collect-releases.jsprojects/internals/tools/src/api/utils.test.tsprojects/internals/tools/src/api/utils.tsprojects/site/eleventy.config.jsprojects/site/package.jsonprojects/site/src/_11ty/layouts/common.jsprojects/site/src/_11ty/layouts/docs.cssprojects/site/src/_11ty/layouts/metadata.jsprojects/site/src/_11ty/layouts/metadata.test.tsprojects/site/src/_11ty/layouts/page.11ty.jsprojects/site/src/_11ty/layouts/whats-new.11ty.jsprojects/site/src/_11ty/layouts/whats-new.test.tsprojects/site/src/_11ty/plugins/llms-txt.test.tsprojects/site/src/_11ty/plugins/sitemap-xml.jsprojects/site/src/_11ty/plugins/sitemap-xml.test.tsprojects/site/src/_11ty/plugins/updates-feed.jsprojects/site/src/_11ty/plugins/updates-feed.test.tsprojects/site/src/_11ty/transforms/html-minify.jsprojects/site/src/_11ty/transforms/html-minify.test.tsprojects/site/src/_11ty/utils/content-dates.jsprojects/site/src/_11ty/utils/content-dates.test.tsprojects/site/src/docs/whats-new/04-2026.mdprojects/site/src/docs/whats-new/05-2026.mdprojects/site/src/docs/whats-new/06-2026.mdprojects/site/src/docs/whats-new/index.11ty.jsprojects/site/src/examples/index.test.ts
Comment on lines
+187
to
+212
| function parseReleaseNoteCommits(notes, repository = REPOSITORY) { | ||
| const escapedRepository = repository.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); | ||
| const linkPattern = new RegExp( | ||
| `\\[([0-9a-f]{7,40})\\]\\((https://github\\.com/${escapedRepository}/commit/([0-9a-f]{7,40}))\\)`, | ||
| 'gi' | ||
| ); | ||
| const commits = []; | ||
|
|
||
| for (const line of notes.split(/\r?\n/)) { | ||
| for (const match of line.matchAll(linkPattern)) { | ||
| const subject = line | ||
| .replace(/^\s*[-*+]\s+/, '') | ||
| .replace(match[0], '') | ||
| .replace(/[\s()]+$/g, '') | ||
| .trim(); | ||
|
|
||
| commits.push({ | ||
| sha: match[3].toLowerCase(), | ||
| subject, | ||
| url: match[2] | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| return commits; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Multiple commit links on one changelog line still corrupt the computed subject.
This was flagged in a prior review and remains unresolved: line.replace(match[0], '') only strips the current match, so when a line contains more than one commit link, the leftover markdown from the other link(s) stays in subject for every commit found on that line.
Separately, the ast-grep non-literal-regexp/ReDoS hint on this construction is a false positive here — repository is a fixed module constant (REPOSITORY), fully escaped, not attacker-controlled input.
🐛 Proposed fix: strip all links from the line once, reuse for every match
for (const line of notes.split(/\r?\n/)) {
- for (const match of line.matchAll(linkPattern)) {
- const subject = line
- .replace(/^\s*[-*+]\s+/, '')
- .replace(match[0], '')
- .replace(/[\s()]+$/g, '')
- .trim();
-
- commits.push({
- sha: match[3].toLowerCase(),
- subject,
- url: match[2]
- });
- }
+ const matches = [...line.matchAll(linkPattern)];
+
+ if (matches.length === 0) {
+ continue;
+ }
+
+ const subject = line
+ .replace(/^\s*[-*+]\s+/, '')
+ .replace(linkPattern, '')
+ .replace(/[\s()]+$/g, '')
+ .trim();
+
+ for (const match of matches) {
+ commits.push({
+ sha: match[3].toLowerCase(),
+ subject,
+ url: match[2]
+ });
+ }
}🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 188-191: Detects non-literal values in regular expressions
Context: new RegExp(
\\[([0-9a-f]{7,40})\\]\\((https://github\\.com/${escapedRepository}/commit/([0-9a-f]{7,40}))\\),
'gi'
)
Note: [CWE-1333] Inefficient Regular Expression Complexity (ReDoS via non-literal RegExp).
(detect-non-literal-regexp)
🤖 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 @.agents/skills/summarize-releases/scripts/collect-releases.js around lines
187 - 212, Update parseReleaseNoteCommits so each line’s subject is computed by
removing all commit-link matches once, then reuse that cleaned subject for every
match on the line; do not derive it by replacing only the current match[0].
Preserve the existing repository escaping and linkPattern construction, since
REPOSITORY is fixed and safely escaped.
Comment on lines
+468
to
+503
| async function createEvidence(options, { now = new Date() } = {}) { | ||
| const period = getPeriod(options, now); | ||
| const gitRoot = await findGitRoot(options.repoDir); | ||
|
|
||
| if (!gitRoot) { | ||
| throw new Error('Run this script from an NVIDIA Elements Git clone or pass --repo-dir.'); | ||
| } | ||
|
|
||
| const { releases, warnings } = await collectGitReleases({ | ||
| gitRoot, | ||
| includePrereleases: options.includePrereleases, | ||
| period | ||
| }); | ||
| const { changes, unenrichedCount } = await collectChanges({ gitRoot, releases }); | ||
|
|
||
| if (releases.length > 0 && changes.length === 0) { | ||
| warnings.push('The selected changelog sections did not contain commit links.'); | ||
| } | ||
|
|
||
| if (unenrichedCount > 0) { | ||
| warnings.push(`${unenrichedCount} changelog commit(s) were not available in the local clone.`); | ||
| } | ||
|
|
||
| return { | ||
| changes, | ||
| generatedAt: new Date(now).toISOString(), | ||
| gitRoot, | ||
| page: getPageMetadata(period, now), | ||
| period, | ||
| releasePageUrl: `${REPOSITORY_URL}/releases`, | ||
| releases, | ||
| repository: REPOSITORY, | ||
| source: 'local-git-tags-and-changelogs', | ||
| warnings | ||
| }; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Still missing docstrings, structured logging, and test coverage for this agent skill's evidence collector.
None of the exported/orchestration functions (parseArgs, getPeriod, getPageMetadata, parseElementsTag, parseConventionalCommit, extractChangelogSection, parseReleaseNoteCommits, collectGitReleases, loadGitCommit, collectChanges, createEvidence, formatMarkdown) document inputs/outputs/constraints, and warning/skip decisions (warnings.push(...)) are only bundled into the final object rather than surfaced via structured, leveled logs. There is also still no companion test file for the parsing/date helpers (month rollover, changelog-heading extraction, commit-link parsing) in this skill tree.
As per coding guidelines, "Document agent capabilities, constraints, and expected inputs/outputs in code comments or docstrings" and "Use structured logging to track agent decision-making processes and state changes for debugging and monitoring."
🤖 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 @.agents/skills/summarize-releases/scripts/collect-releases.js around lines
468 - 503, Document the exported/orchestration functions parseArgs, getPeriod,
getPageMetadata, parseElementsTag, parseConventionalCommit,
extractChangelogSection, parseReleaseNoteCommits, collectGitReleases,
loadGitCommit, collectChanges, createEvidence, and formatMarkdown with their
inputs, outputs, and constraints. Add structured, leveled logging at
warning/skip decisions currently recorded through warnings.push(...), while
preserving the returned warnings. Add companion tests covering parsing and
date-helper behavior, including month rollover, changelog-heading extraction,
and commit-link parsing.
Source: Coding guidelines
Add new summarize-releases skill for generating monthly release summaries and implement related scripts and tests. Enhance site configuration to support updates feed and improve metadata handling for recent updates. Signed-off-by: Cory Rylan <crylan@nvidia.com>
coryrylan
force-pushed
the
topic-summary-skill
branch
from
July 24, 2026 19:54
8d78420 to
9f7f1ee
Compare
Comment on lines
+48
to
+50
| <nve-card-content> | ||
| <p nve-text="body">${entry.data.description}</p> | ||
| </nve-card-content> |
Comment on lines
+120
to
+125
| data: { | ||
| ...post.data, | ||
| ...(atom ? { summary: post.data.description } : {}), | ||
| dateModified: modified, | ||
| datePublished: published | ||
| }, |
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.
Add new summarize-releases skill for generating monthly release summaries and implement related scripts and tests. Enhance site configuration to support updates feed and improve metadata handling for recent updates.
Summary by CodeRabbit
lastmodwhen modification metadata is available.lastmod, and date/metadata utilities.