ci(e2e): report runner wait and execution time#7493
Conversation
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
|
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:
📝 WalkthroughWalkthroughScorecard job summaries now calculate queue and execution timing, classify runner sizes, and render the ten longest jobs in a markdown table. Tests cover formatting, truncation, fallback summaries, and runner-label redaction. E2E reporting documentation describes the updated output and runtime baseline. ChangesScorecard observability
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant APIJobs
participant summarizeJobs
participant renderSummaryLines
participant SlackReport
APIJobs->>summarizeJobs: Provide job timestamps and labels
summarizeJobs->>summarizeJobs: Calculate and classify timing rows
summarizeJobs-->>renderSummaryLines: Return timingRows
renderSummaryLines->>SlackReport: Render timing markdown table
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 922d020 in the TypeScript / code-coverage/cliThe overall coverage in commit 922d020 in the Show a code coverage summary of the most impacted files.
Updated |
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 `@scripts/scorecard/summarize-jobs.mts`:
- Around line 103-110: Update normalizeRunnerClass to check normalized labels
for "self-hosted" before checking "ubuntu-latest", so jobs with both labels
return "unknown"; preserve the existing standard, larger, and empty-label
classifications otherwise.
🪄 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: CHILL
Plan: Enterprise
Run ID: 02d24688-9a22-4453-a84b-f6412937961c
📒 Files selected for processing (5)
scripts/scorecard/coordinate-scorecard.mtsscripts/scorecard/summarize-jobs.mtstest/e2e/README.mdtest/e2e/support/e2e-scorecard-coordinator.test.tstest/e2e/support/e2e-scorecard.test.ts
PR Review Advisor — InformationalAdvisor assessment: Informational / high confidence Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
This reverts commit 436e5a3. Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/e2e/support/artifact-zip.test.ts (1)
102-116: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winCover a valid payload with a mismatched CRC.
This only corrupts compressed bytes; add a case that preserves valid deflate data but changes both local and central-directory CRC fields, then expects
null. That exercises the checksum-validation contract directly.Proposed test addition
+ const crcMismatchArchive = Buffer.from(archive); + const centralDirectoryOffset = crcMismatchArchive.readUInt32LE( + crcMismatchArchive.length - 6, + ); + crcMismatchArchive.writeUInt32LE( + crcMismatchArchive.readUInt32LE(14) ^ 0xffffffff, + 14, + ); + crcMismatchArchive.writeUInt32LE( + crcMismatchArchive.readUInt32LE(centralDirectoryOffset + 16) ^ 0xffffffff, + centralDirectoryOffset + 16, + ); + expect( + readValidatedArtifactZipEntry(crcMismatchArchive, "summary.json", { maxBytes: 1_024 }), + ).toBeNull();As per path instructions, review tests for “behavioral confidence rather than implementation lock-in”; the supplied stack identifies CRC validation as part of this reader’s contract.
🤖 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 `@test/e2e/support/artifact-zip.test.ts` around lines 102 - 116, Add a CRC-mismatch case to the test for readValidatedArtifactZipEntry using the same valid deflated archive, preserving its compressed payload while changing the CRC fields in both the local header and central directory. Assert that reading summary.json with the existing maxBytes option returns null, directly covering checksum validation without depending on implementation details.Source: Path instructions
🤖 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 `@scripts/scorecard/analyze-runtime-history.mts`:
- Around line 335-336: Guard the current-summary creation and file write in the
runtime-history flow around createRuntimeSummary and fs.writeFileSync so invalid
input or IO failures are caught without propagating out of buildRuntimeHistory
or aborting the scorecard. Preserve the upload step’s expected output-file
contract: keep the successful write on the happy path, and ensure the existing
fallback/error handling does not leave a required upload file unexpectedly
missing.
---
Nitpick comments:
In `@test/e2e/support/artifact-zip.test.ts`:
- Around line 102-116: Add a CRC-mismatch case to the test for
readValidatedArtifactZipEntry using the same valid deflated archive, preserving
its compressed payload while changing the CRC fields in both the local header
and central directory. Assert that reading summary.json with the existing
maxBytes option returns null, directly covering checksum validation without
depending on implementation details.
🪄 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: CHILL
Plan: Enterprise
Run ID: c5469a9b-02e0-4c3c-be7f-d77ee60dcb2d
📒 Files selected for processing (12)
.github/workflows/e2e.yamlscripts/audit-test-runtime.mtsscripts/scorecard/analyze-runtime-history.mtsscripts/scorecard/analyze-trace-timing.mtsscripts/scorecard/read-artifact-zip.mtstest/e2e/README.mdtest/e2e/support/artifact-zip.test.tstest/e2e/support/e2e-operations-workflow-boundary.test.tstest/e2e/support/e2e-runtime-history.test.tstest/e2e/support/test-runtime-audit.test.tstest/e2e/support/upload-e2e-artifacts-workflow-boundary.test.tstools/e2e/upload-e2e-artifacts-workflow-boundary.mts
🚧 Files skipped from review as they are similar to previous changes (1)
- test/e2e/README.md
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com> # Conflicts: # test/e2e/README.md
Signed-off-by: Charan Jagwani <cjagwani@nvidia.com>
Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
<!-- markdownlint-disable MD041 --> ## Summary Adds the canonical pre-tag `## v0.0.95` release entry to `docs/changelog/2026-07-24.mdx`, before the existing v0.0.94 entry. The entry summarizes approved user-visible changes merged since v0.0.94 and excludes internal-only prerequisites. ## Changes - Adds the v0.0.95 summary and detailed bullets for gateway lifecycle, recovery, state transfer, inference compatibility, sandbox security, Discord policy, and E2E evidence. - Links each user-facing theme to the most specific published documentation. - Records the release entry in the shared native changelog used by the OpenClaw, Hermes, and Deep Agents guides. Source summary: - [#7246](#7246), [#7228](#7228), [#7267](#7267), [#7489](#7489), [#7509](#7509), [#7351](#7351), and [#7290](#7290) -> `docs/changelog/2026-07-24.mdx`: Gateway authority, forward teardown and retry, managed recovery, Hermes restart recovery, scoped uninstall, and orphan-aware backup behavior. - [#7344](#7344) and [#7416](#7416) -> `docs/changelog/2026-07-24.mdx`: Atomic SQLite restore and host download verification. - [#7476](#7476), [#7347](#7347), [#7281](#7281), [#7485](#7485), [#7491](#7491), and [#7422](#7422) -> `docs/changelog/2026-07-24.mdx`: Windows Ollama reuse, CDI fallback, bounded OpenRouter connection setup, Nemotron-3 request compatibility, and managed Deep Agents retry and provider-error behavior. - [#6884](#6884), [#7481](#7481), [#6878](#6878), [#7467](#7467), [#7502](#7502), [#7503](#7503), [#7504](#7504), and [#7486](#7486) -> `docs/changelog/2026-07-24.mdx`: Trusted base-image overrides, local rebuild images, runtime validation, config preservation, reviewed package updates, and fewer final-image payload layers. - [#7303](#7303) -> `docs/changelog/2026-07-24.mdx`: Scoped Discord application-command management. - [#7488](#7488), [#7465](#7465), [#7497](#7497), [#7464](#7464), [#7501](#7501), [#7494](#7494), and [#7493](#7493) -> `docs/changelog/2026-07-24.mdx`: Selected-test risk signals, retry cleanup, full root-image validation, direct-main Hermes setup, executed PR-gate evidence, nightly history, and runner wait reporting. - [#7447](#7447) is an internal pinned-runtime prerequisite and is intentionally excluded from canonical supported-integration documentation. - [#7370](#7370) adds maintainer-only advisory reconciliation tooling and does not change supported user behavior. - [#7495](#7495) updates existing documentation and does not add a new v0.0.95 behavior claim. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [x] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [ ] Tests added or updated for changed behavior - [x] Existing tests cover changed behavior — justification: `test/changelog-docs.test.ts` validates the dated changelog structure, heading uniqueness, and published links. - [ ] Tests not applicable — justification: - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [ ] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `docs-updated` - Evidence: `docs/changelog/2026-07-24.mdx`; writing rules, documentation style, factual release meaning, and published links reviewed at exact head `58b02f2bf`. - Agent: Codex documentation writer reviewer <!-- docs-review-head-sha: 58b02f2 --> <!-- docs-review-agents-blob-sha: 9c9b36d --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: - Station profile/scenario: - Result: - Supporting evidence: ## Verification - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: `npx vitest run test/changelog-docs.test.ts` passed 6 tests. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) — the build passed with 0 errors and 2 Fern warnings. - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added a new v0.0.95 changelog entry above v0.0.94. * Documented improved externally supervised gateway lifecycle ownership. * Improved snapshot restore reliability and SQLite state handling. * Tightened CLI `backup-all` behavior and host artifact verification. * Updated Windows onboarding guidance (including Ollama service reuse and CDI directory fallback). * Noted inference compatibility fixes, deeper agent failure classification, stricter base-image validation, updated Discord bot command permissions, and refined E2E release automation evidence handling. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Summary
The E2E scorecard now separates each job's runner queue time from its execution time. The report identifies whether a slow run waited for capacity or spent time executing tests without exposing raw runner labels.
Related Issue
Part of #7145.
Changes
standard,larger, orunknownrunner class.unknownand never expose raw runner labels.needsfallback when GitHub's jobs API is unavailable.623c87394fd461be3ebcac100031be312663b588with signed merge commit922d02022ab360436aa08f8a3ae5010daa8becdc. The refreshed patch hashcf1d06b1321eaa171765499ea1def712706bb9da2398c9b2ebbcfd68b69d5eeamatches the pre-refresh patch.Type of Change
Quality Gates
922d02022ab360436aa08f8a3ae5010daa8becdcagainst base SHA623c87394fd461be3ebcac100031be312663b588. All categories passed: the change emits normalized runner classes, does not emit raw labels, and does not change runner selection or test execution.Documentation Writer Review
docs-updatedtest/e2e/README.mdand all changed scorecard text and test titles at PR SHA922d02022ab360436aa08f8a3ae5010daa8becdcagainst base SHA623c87394fd461be3ebcac100031be312663b588. The README documents queue and execution timing, normalized runner classes, and runner-label redaction. No additional user-facing docs are needed. Focused E2E-support tests passed 29/29.npm run docspassed with 0 errors and 2 warnings.DGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablenpx vitest run --project e2e-support test/e2e/support/e2e-scorecard.test.ts test/e2e/support/e2e-scorecard-coordinator.test.ts: 2 files and 29 tests passed.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result:npm run docsbuilds without warnings (doc changes only)Signed-off-by: Charan Jagwani cjagwani@nvidia.com