Skip to content

ci(test): drop stale advisor timing hint - #8327

Closed
apurvvkumaria wants to merge 1 commit into
mainfrom
codex/fix-stale-cli-timing-hint
Closed

ci(test): drop stale advisor timing hint#8327
apurvvkumaria wants to merge 1 commit into
mainfrom
codex/fix-stale-cli-timing-hint

Conversation

@apurvvkumaria

@apurvvkumaria apurvvkumaria commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

Summary

Current main split and deleted test/pr-review-advisor.test.ts, but the CLI timing manifest still required that path to exist. CLI shard 8 therefore failed deterministically before unrelated pull requests could complete their merge gates. This removes the obsolete timing entry so the sequencer uses its conservative fallback for the new split files.

Changes

  • Remove the deleted advisor test path from ci/cli-test-timing-hints.json.
  • Preserve measured timing hints for every test file that still exists.

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with doc updates
  • Doc only (prose changes, no code sample modifications)
  • Doc only (includes code sample changes)

Quality Gates

  • Tests added or updated for changed behavior
  • Existing tests cover changed behavior — justification: test/cli-coverage-sequencer.test.ts validates that every timing-hint path exists and that the fallback remains conservative.
  • Tests not applicable — justification:
  • Docs updated for user-facing behavior changes
  • Docs not applicable — justification: This changes CI test-scheduling metadata only; production and user-visible behavior are unchanged.
  • 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

  • Documentation writer subagent reviewed the completed changes
  • Result: no-docs-needed
  • Evidence: The change removes stale CI test-timing metadata for a test file that current main deleted and split; production and user-visible behavior are unchanged.
  • Agent: Codex Desktop

DGX Station Hardware Evidence

  • Tested on DGX Station
  • Tested commit: Not applicable.
  • Station profile/scenario: Not applicable.
  • Result: Not applicable.
  • Supporting evidence: Not applicable.

Verification

  • PR description includes a Signed-off-by: line and every commit appears as Verified in GitHub
  • Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run validate:pr passed after refreshing origin/main when hooks were skipped or unavailable
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — npx vitest run --project integration test/cli-coverage-sequencer.test.ts: 8/8 passed.
  • Applicable broad gate passed — not applicable; the one-line manifest correction is fully exercised by the focused validator.
  • Quality Gates section completed with required justifications or waivers
  • No secrets, API keys, or credentials committed
  • npm run docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: Apurv Kumaria akumaria@nvidia.com

Summary by CodeRabbit

  • Chores
    • Updated CLI test timing configuration by removing an outdated timing hint.

Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
@apurvvkumaria apurvvkumaria added bug-fix PR fixes a bug or regression v0.0.103 Release target labels Aug 5, 2026
@apurvvkumaria apurvvkumaria self-assigned this Aug 5, 2026
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 616fe2c3-5f15-4cfe-912f-71df09231bf1

📥 Commits

Reviewing files that changed from the base of the PR and between 28fdd78 and 1bef44d.

📒 Files selected for processing (1)
  • ci/cli-test-timing-hints.json
💤 Files with no reviewable changes (1)
  • ci/cli-test-timing-hints.json

📝 Walkthrough

Walkthrough

This PR removes the timing hint entry for test/pr-review-advisor.test.ts from the CLI test timing configuration.

Changes

CLI timing hints

Layer / File(s) Summary
Remove obsolete timing entry
ci/cli-test-timing-hints.json
The files timing map no longer includes the recorded duration for test/pr-review-advisor.test.ts.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Possibly related PRs

  • NVIDIA/NemoClaw#8320: This PR appears to remove the timing hint that matched the deleted or split pr-review-advisor test file from that change.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the removal of a stale test timing hint.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/fix-stale-cli-timing-hint

Comment @coderabbitai help to get the list of available commands.

@github-code-quality

github-code-quality Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in commit 1bef44d in the codex/fix-stale-cli-... branch remains at 96%, unchanged from commit 28fdd78 in the main branch.

TypeScript / code-coverage/cli

The overall coverage in commit 1bef44d in the codex/fix-stale-cli-... branch remains at 81%, unchanged from commit 28fdd78 in the main branch.

Show a code coverage summary of the most impacted files.
File main 28fdd78 codex/fix-stale-cli-... 1bef44d +/-
src/lib/onboard...ider-cleanup.ts 100% 88% -12%
src/lib/shields/index.ts 74% 73% -1%
src/lib/inferen...lama-version.ts 100% 100% 0%
src/lib/onboard...-transaction.ts 94% 94% 0%
src/lib/sandbox...rce-identity.ts 87% 87% 0%
src/lib/credentials/store.ts 55% 56% +1%
src/lib/inferen...er-discovery.ts 87% 88% +1%
src/lib/inferen...time-context.ts 92% 93% +1%
src/lib/domain/.../connect-env.ts 89% 97% +8%

Updated August 05, 2026 10:43 UTC

@apurvvkumaria

Copy link
Copy Markdown
Collaborator Author

Closing in favor of merged #8325.

The merged PR removes the same obsolete test/pr-review-advisor.test.ts timing entry from ci/cli-test-timing-hints.json. The two changes have the matching stable patch ID 0d1db29269c728b4834b8ddbe8a5ebec4ff63200, and the resulting timing-manifest files are identical.

Current main therefore contains the complete fix that #8327 proposed. No work from this PR remains to land.

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — Informational

Advisor assessment: Informational / low confidence
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions
Status: PR review advisor failed: PR review advisor SDK execution failed: session: omitted required tool result(s): pr_review_controlled_words, pr_review_terminology_pr_context; terminology-review-analysis omitted required analysis; turn: terminology-review-analysis: omitted required tool result(s): pr_review_controlled_words, pr_review_terminology_pr_context; terminology-review-analysis omitted required analysis

Model lanes

  • GPT-5.6 Terra (primary): Failed
  • Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions

Second-opinion terminology and E2E selections are advisory. They do not change the primary assessment or E2E / PR Gate.

E2E guidance

Advisory only. E2E / PR Gate selects and runs jobs independently.

Recommended E2E: None

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug-fix PR fixes a bug or regression v0.0.103 Release target

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant