Skip to content

Add a validation step for the specified models. - #39338

Closed
AndriySvyryd wants to merge 2 commits into
mainfrom
ModelCheck
Closed

AndriySvyryd wants to merge 2 commits into
mainfrom
ModelCheck

Conversation

@AndriySvyryd

Copy link
Copy Markdown
Member

No description provided.

@AndriySvyryd
AndriySvyryd requested a review from wtgodbe as a code owner October 19, 2026 21:08
Copilot AI balanced review requested due to automatic review settings October 19, 2026 21:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The new integration test does not propagate its SDK mock to the grading subprocess, introducing authentication and network dependencies.

1 open issue
What changed in this PR

Adds model validation to the evaluation harness’s judge SDK adapter, rejecting unavailable models before session creation.

Changes:

  • Validates model IDs while preserving automatic routing and custom providers.
  • Extends tests for model rejection, timeout behavior, and CLI parameter forwarding.
File Description
eng/​harness-evaluation/​test/​harness.test.mjs Adds model-validation and launcher regression coverage.
eng/​harness-evaluation/​src/​judge-timeout-loader.mjs Validates judge models against the SDK catalog.

Review effort: Balanced

Update spawnSync options to include environment variables.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 19, 2026 21:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

No concrete defects were identified, but native Vally and Copilot SDK integration remains unverified.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

@github-code-quality

Copy link
Copy Markdown

Code coverage overview

Languages: C#

C# / code-coverage

main 3f03bb5 ModelCheck 3770cef +/-
76% 76% 0%
View coverage of the 10 most impacted files.
File main 3f03bb5 ModelCheck 3770cef +/-
src/EFCore.Rela...CommandCache.cs 100% 98% -2%
src/EFCore.Abst...erExtensions.cs 100% 100% 0%
src/EFCore.Cosm...tionStrategy.cs 40% 40% 0%
src/EFCore.Desi...onExtensions.cs 100% 100% 0%
src/EFCore.Desi...CSharpHelper.cs 96% 96% 0%
src/EFCore.Desi...elExtensions.cs 98% 98% 0%
src/EFCore.Desi...hotGenerator.cs 99% 99% 0%
src/EFCore.Desi...odeGenerator.cs 97% 97% 0%
src/Shared/NonC...yInitializer.cs 11% 12% +1%
src/EFCore/Stor.../ValueBuffer.cs 80% 85% +5%

ℹ️ Calculations represent line coverage. Results may change as calculations are updated.

@AndriySvyryd

Copy link
Copy Markdown
Member Author

This would be better handled at the Vally/Copilot SDK level

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants