Skip to content

test(inference): delete duplicate vLLM contract suite - #8451

Open
laitingsheng wants to merge 1 commit into
mainfrom
test/remove-duplicate-vllm-contract-suite
Open

test(inference): delete duplicate vLLM contract suite#8451
laitingsheng wants to merge 1 commit into
mainfrom
test/remove-duplicate-vllm-contract-suite

Conversation

@laitingsheng

@laitingsheng laitingsheng commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Summary

src/lib/inference/vllm-contracts.test.ts held five tests that src/lib/inference/vllm.test.ts already owns, so the same five behaviors were collected and executed twice on every run. This deletes the duplicate suite. The retained tests, the vLLM production code, and Vitest project membership are unchanged.

Related Issue

Resolves #8350

Changes

  • Deletes src/lib/inference/vllm-contracts.test.ts (97 lines, five it declarations across the vLLM served route identity and managed vLLM image distribution boundary suites).
  • Nothing else changes. No production code, no configuration, no fixture, no shared helper.

The retained owner is src/lib/inference/vllm.test.ts:236-308, which carries both suite names, all five test titles, and the same assertions. Comparing the two ranges shows one difference: the retained copy adds mocks.dockerPullWithProgressWatchdog.mockClear() inside refuses a local image ID before invoking Docker pull, where the deleted copy relied on a file-level beforeEach(() => vi.clearAllMocks()). The retained copy is therefore the stricter of the two, and no assertion is lost.

Nothing references the deleted path. git grep vllm-contracts returns no hits across the repository, and scripts/checks/vitest-project-overlap.mts discovers candidates from the filesystem rather than a hardcoded roster, so project membership stays exact after the deletion.

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: the five deleted tests keep their owner in src/lib/inference/vllm.test.ts:236-308, which passes unchanged
  • Tests not applicable — justification:
  • Docs updated for user-facing behavior changes
  • Docs not applicable — justification: a test-only deletion with no user-visible surface; no command, flag, environment variable, default, or error string changes
  • 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: no documentation path changed. The review ran against the completed diff and the writing rules in docs/AGENTS.md, WRITING.md, docs/CONTRIBUTING.md, and CONTRIBUTING.md. It searched for the surfaces a test deletion can strand: vllm-contracts references anywhere in the repository, test paths cited with line numbers in docs/ (the pattern exists, for example docs/reference/platform-support.mdx:172), regression-test citations in the security review pages, hardcoded test lists in scripts/checks/vitest-project-overlap.mts, and per-file entries in ci/test-file-size-budget.json and ci/cli-test-timing-hints.json. The deleted file appears in none of them. Its one finding was about wording in this description, which is applied above.
  • Agent: Claude Code

DGX Station Hardware Evidence

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

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 — command/result or justification: npx vitest run --project cli src/lib/inference/vllm.test.ts 67/67 pass; npm run test:projects:check reports Vitest project membership is exact (2204 candidate files across 7 projects); npm run typecheck:cli exits clean. All three rerun on the rebased head.
  • Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — command/result:
  • 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: Tinson Lai tinsonl@nvidia.com

Summary by CodeRabbit

  • Tests
    • Removed automated coverage for vLLM model identity resolution, registry digest validation, managed image references, and local image handling.

Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
@laitingsheng laitingsheng added refactor PR restructures code without intended behavior change provider: vllm vLLM local or hosted provider behavior area: inference Inference routing, serving, model selection, or outputs labels Aug 6, 2026
@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

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: 379dcdb4-1a26-4fa4-8d52-514c1a364143

📥 Commits

Reviewing files that changed from the base of the PR and between 2f29784 and 8cfd682.

📒 Files selected for processing (1)
  • src/lib/inference/vllm-contracts.test.ts
💤 Files with no reviewable changes (1)
  • src/lib/inference/vllm-contracts.test.ts

📝 Walkthrough

Walkthrough

The duplicate vllm-contracts.test.ts suite was deleted. No production code or public declarations changed.

Changes

Cohort / File(s) Summary
vLLM test cleanup
src/lib/inference/vllm-contracts.test.ts
Deletes duplicate tests for vLLM model identity, registry digest validation, managed image references, and local image handling.

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

Possibly related issues

Possibly related PRs

🚥 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 identifies deletion of the duplicate vLLM contract test suite.
Linked Issues check ✅ Passed The PR deletes the duplicate suite, preserves retained tests and production behavior, and satisfies issue #8350 acceptance criteria.
Out of Scope Changes check ✅ Passed The PR only deletes the duplicate test file and introduces no unrelated code, configuration, fixture, or production changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch test/remove-duplicate-vllm-contract-suite

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

@github-code-quality

github-code-quality Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in commit 8cfd682 in the test/remove-duplicat... branch remains at 96%, unchanged from commit 2f29784 in the main branch.

TypeScript / code-coverage/cli

The overall coverage in commit 8cfd682 in the test/remove-duplicat... branch remains at 82%, unchanged from commit 2f29784 in the main branch.

Show a code coverage summary of the most impacted files.
File main 2f29784 test/remove-duplicat... 8cfd682 +/-
src/lib/domain/.../connect-env.ts 97% 89% -8%
src/lib/state/m...-acquisition.ts 86% 85% -1%
src/lib/sandbox...rce-identity.ts 87% 87% 0%
src/lib/state/m...ock-identity.ts 94% 94% 0%
src/lib/state/m...lock-storage.ts 95% 95% 0%
src/lib/actions...air-approval.ts 89% 90% +1%

Updated August 06, 2026 07:08 UTC

@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — No blocking findings reported

Advisor assessment: No blocking advisor findings reported
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions

Model lanes

  • GPT-5.6 Terra (primary): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Model comparison: normalized findings match; normalized terminology decisions match; normalized E2E selections match; severity counts match.

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.

@laitingsheng laitingsheng added the v0.0.104 Release target label Aug 6, 2026
@apurvvkumaria apurvvkumaria self-assigned this Aug 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: inference Inference routing, serving, model selection, or outputs provider: vllm vLLM local or hosted provider behavior refactor PR restructures code without intended behavior change release-target v0.0.104 Release target

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Delete the duplicate vLLM contract test suite

2 participants