fix(cli): resolve NameError in legacy create-eval-set route - #6681
Open
gaurav-gandhi-2411 wants to merge 1 commit into
Open
fix(cli): resolve NameError in legacy create-eval-set route#6681gaurav-gandhi-2411 wants to merge 1 commit into
gaurav-gandhi-2411 wants to merge 1 commit into
Conversation
create_eval_set_legacy in dev_server.py constructed an EvalSet via
UserEvalSet(...), a name that is never imported or defined anywhere in
the package. CreateEvalSetRequest.eval_set is typed EvalSet (already
imported in this file), which is the class that was clearly intended.
Every call to the deprecated POST /dev/apps/{app_name}/eval_sets/{eval_set_id}
route crashed unconditionally with NameError: name 'UserEvalSet' is not
defined.
A strict-xfail regression test for exactly this bug already existed
(test_create_eval_set_legacy_route_creates_eval_set, added 2026-08-06),
so this change removes the xfail marker instead of adding a new test.
Note: ruff's F821 (undefined-name) rule is not enabled in this repo's
lint config (pyproject.toml sets lint.select = ["F401"] only), which is
why this went uncaught by CI.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔴 Required Information
Describe the Bug:
create_eval_set_legacyindev_server.pyconstructs the eval set viaUserEvalSet(eval_set_id=eval_set_id, eval_cases=[]).UserEvalSetis never imported or defined anywhere in this file or anywhere else in the package.CreateEvalSetRequest.eval_setis typedEvalSet(from ..evaluation.eval_set import EvalSet, already imported at the top ofdev_server.py), which is clearly the class that was intended here.Steps to Reproduce:
web=True).POST /dev/apps/{app_name}/eval_sets/{eval_set_id}(the deprecated legacy create-eval-set route).Expected Behavior: An empty eval set is created, same as the non-legacy
POST /dev/apps/{app_name}/eval-setsroute.Observed Behavior: Every call raises unconditionally:
This is executable code (a constructor call), not a type annotation, so it isn't deferred by
from __future__ import annotations— the route is broken for every caller, unconditionally.Why this fix
Swap
UserEvalSet(...)forEvalSet(...)— the type the surroundingCreateEvalSetRequestmodel already declares and that's already imported in this file. One line.A strict-xfail regression test for exactly this bug already existed —
test_create_eval_set_legacy_route_creates_eval_setintests/unittests/cli/test_fast_api.py, added 2026-08-06 (commit 456524d, "test: add unit tests for public symbols that had no coverage") with reason "legacy create-eval-set route references an undefined name". Rather than add a duplicate test, this PR removes the xfail marker so the existing test now runs and passes for real.Testing Plan
Confirmed on
main(before this change): removing the xfail marker alone reproduces the exact NameError above. After the fix, the same test passes.Observation (not a proposal for this PR)
Ruff's F821 (undefined-name) rule is not enabled in this repo's lint config — pyproject.toml sets
[tool.ruff] lint.select = ["F401"]only — which is why this went uncaught by CI. Noting this as context for why the bug shipped, not proposing a CI/config change here.