chore(ci): enhance ci performance profiling and add new scripts - #197
chore(ci): enhance ci performance profiling and add new scripts#197coryrylan wants to merge 1 commit into
Conversation
- Introduced a new CI performance audit document to analyze and optimize the CI pipeline. - Added a `ci:profile` script to facilitate performance profiling. - Updated `package.json` and `pnpm-lock.yaml` to include new dependencies and scripts. - Enhanced existing scripts for better performance tracking and reporting. - Updated various project configurations to improve build and test processes. Signed-off-by: Cory Rylan <crylan@nvidia.com>
github-actions
Bot
added
scope(core)
scope(ci)
scope(internals)
scope(docs)
scope(styles)
dependencies
📝 WalkthroughWalkthroughThe pull request adds a cold CI profiling workflow and optimization guidance, updates CI and build configuration, refines site rendering behavior, fixes complete element-tag matching, and changes Next.js and Nuxt starter pipelines. ChangesCI profiling workflow
Site rendering and version data
Tooling and starter pipelines
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant ProfileCLI
participant Mise
participant WireitCI
participant Metrics
ProfileCLI->>Mise: run ci:reset and cold CI
Mise->>WireitCI: execute CI with cache disabled
WireitCI-->>ProfileCLI: stream Wireit timing logs
ProfileCLI->>Metrics: aggregate samples and write reports
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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/core/package.jsonESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. projects/internals/tools/src/internal/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.
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/profile-ci/SKILL.md:
- Around line 31-36: Update the profiling instructions around ci:profile and
ci:reset to require explicit user approval after reviewing git clean -ndX
output, rather than treating the preview as consent. Allow an explicit
destructive-reset flag as an alternative, and do not invoke ci:reset until one
of these approvals is provided.
In `@projects/internals/ci/ci-profile.js`:
- Around line 203-205: The getMetadata() function reads Wireit’s package
metadata too early, before ci:reset reinstalls dependencies. Keep initial
worktree metadata separate from the Wireit version lookup, then read
node_modules/wireit/package.json only after the first reset or at profiling
completion, while preserving the initial worktree state.
In `@projects/internals/tools/src/internal/utils.ts`:
- Line 31: Update the tag-matching regular expression used to build elementTags
so the captured custom-element name must be immediately followed by whitespace,
"/", or ">". Preserve matching valid hyphenated tag names while excluding
namespaced or otherwise extended names such as nve-button:example.
🪄 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: 03f706fe-f624-4a8d-bd6a-cd0aec21c9dc
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (18)
.agents/skills/profile-ci/SKILL.md.agents/skills/profile-ci/references/optimization-playbook.mdconfig/vale/styles/config/vocabularies/Elements/accept.txtpackage.jsonprojects/core/package.jsonprojects/internals/ci/ci-profile.jsprojects/internals/ci/package.jsonprojects/internals/tools/src/internal/utils.test.tsprojects/internals/tools/src/internal/utils.tsprojects/internals/vite/src/configs/build.bundle.jsprojects/site/eleventy.config.jsprojects/site/src/_11ty/utils/version.jsprojects/site/src/examples/index.11ty.jsprojects/site/src/examples/index.test.tsprojects/starters/nextjs/eslint.config.jsprojects/starters/nextjs/package.jsonprojects/starters/nuxt/package.jsonprojects/styles/package.json
💤 Files with no reviewable changes (1)
- projects/styles/package.json
| 3. Stop and ask before profiling if that preview includes user data, local assets, secrets, or other non-reproducible files. The profiler runs `pnpm run ci:reset`, which deletes ignored files and reinstalls dependencies before every sample. | ||
| 4. Never stash, commit, discard, or clean tracked changes merely to make the profiler accept the worktree. | ||
| 5. Use the clean command when the worktree is clean: | ||
|
|
||
| ```shell | ||
| mise exec -- pnpm run ci:profile |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Require explicit approval before the destructive reset.
The ignored-file preview is not consent to delete every ignored path. Require confirmation after git clean -ndX (or an explicit destructive-reset flag) before invoking ci:reset; otherwise the skill can delete local data based on its own classification.
🧰 Tools
🪛 SkillSpector (2.4.4)
[warning] 114: [EA2] Autonomous Decision Making: Skill enables autonomous high-impact decisions without human-in-the-loop verification. Critical operations (destructive commands, financial transactions, data deletion) should require explicit user confirmation.
Remediation: Add human-in-the-loop confirmation for destructive, irreversible, or high-impact operations. Never auto-execute commands that modify files, send data, or alter system state.
(Excessive Agency (EA2))
🤖 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/profile-ci/SKILL.md around lines 31 - 36, Update the
profiling instructions around ci:profile and ci:reset to require explicit user
approval after reviewing git clean -ndX output, rather than treating the preview
as consent. Allow an explicit destructive-reset flag as an alternative, and do
not invoke ci:reset until one of these approvals is provided.
Source: Linters/SAST tools
| function getMetadata() { | ||
| const cpuList = cpus(); | ||
| const wireitPackage = JSON.parse(readFileSync(path.join(ROOT_DIR, 'node_modules/wireit/package.json'), 'utf8')); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
jq -r '.devDependencies.wireit // .dependencies.wireit // empty' package.json
if test -f node_modules/wireit/package.json; then
jq -r '.version' node_modules/wireit/package.json
else
echo 'Wireit is unavailable before installation.'
fiRepository: NVIDIA/elements
Length of output: 169
Delay the Wireit version lookup until after ci:reset. getMetadata() reads node_modules/wireit/package.json before dependencies are reinstalled, so a clean checkout can fail with ENOENT before the reset that installs Wireit runs. Keep the initial worktree state separately and read the installed version after the first reset or at the end of profiling.
🤖 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/ci/ci-profile.js` around lines 203 - 205, The
getMetadata() function reads Wireit’s package metadata too early, before
ci:reset reinstalls dependencies. Keep initial worktree metadata separate from
the Wireit version lookup, then read node_modules/wireit/package.json only after
the first reset or at profiling completion, while preserving the initial
worktree state.
| } | ||
|
|
||
| export function getElementImports(html: string, elements: Element[], lazy = false) { | ||
| const elementTags = new Set(Array.from(html.matchAll(/<([a-z][\w.-]*-[\w.-]+)/g), match => match[1])); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Require a complete tag-name boundary.
The regex can extract nve-button from malformed or namespaced markup such as <nve-button:example>, causing an import even though the complete tag is not nve-button. Add a lookahead for whitespace, /, or > after the captured name.
Suggested fix
- const elementTags = new Set(Array.from(html.matchAll(/<([a-z][\w.-]*-[\w.-]+)/g), match => match[1]));
+ const elementTags = new Set(
+ Array.from(html.matchAll(/<([a-z][\w.-]*-[\w.-]+)(?=[\s/>])/g), match => match[1])
+ );📝 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.
| const elementTags = new Set(Array.from(html.matchAll(/<([a-z][\w.-]*-[\w.-]+)/g), match => match[1])); | |
| const elementTags = new Set( | |
| Array.from(html.matchAll(/<([a-z][\w.-]*-[\w.-]+)(?=[\s/>])/g), match => match[1]) | |
| ); |
🤖 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/tools/src/internal/utils.ts` at line 31, Update the
tag-matching regular expression used to build elementTags so the captured
custom-element name must be immediately followed by whitespace, "/", or ">".
Preserve matching valid hyphenated tag names while excluding namespaced or
otherwise extended names such as nve-button:example.
The median cold
pnpm run ciduration is 84.20 seconds, down 19.25 seconds (18.6%) from the previous 103.45-second audit. All three samples completed 130 scripts without an incomplete command.ci:profilescript to facilitate performance profiling.package.jsonandpnpm-lock.yamlto include new dependencies and scripts.Summary by CodeRabbit
New Features
Bug Fixes
Improvements