FIX: GUI target config shows wrong model name due to env var override#1590
Open
romanlutz wants to merge 13 commits intomicrosoft:mainfrom
Open
FIX: GUI target config shows wrong model name due to env var override#1590romanlutz wants to merge 13 commits intomicrosoft:mainfrom
romanlutz wants to merge 13 commits intomicrosoft:mainfrom
Conversation
The target configuration view in the GUI displayed incorrect model names because the underlying_model environment variable (e.g., OPENAI_CHAT_UNDERLYING_MODEL) silently overrode user-provided model names. This affected both GUI-created targets and initializer-registered targets (deepseek, mistral, gemini, llama, etc.). Root cause: OpenAITarget.__init__ unconditionally fell back to the underlying_model env var even when a model_name was explicitly passed, causing the identifier to store the wrong value. The frontend then displayed this incorrect identifier value. ## Backend fixes - **openai_target.py**: Only fall back to underlying_model env var when model_name was also resolved from env vars. When model_name is explicitly passed (by the GUI or initializer) but underlying_model is not, the env var no longer overrides it. - **prompt_target.py**: Store deployment_name (the user's model_name input) as a separate identifier param alongside model_name (underlying model), so both are always available. - **target_mappers.py**: Extract deployment_name from identifier params for the API. - **targets.py model**: Add deployment_name field to TargetInstance DTO. ## Frontend fixes - **CreateTargetDialog**: Add toggle for specifying a different underlying model (hidden by default, with explanation about Azure deployment names). - **TargetTable**: Redesigned with collapsible sections grouped alphabetically by target type, Expand All / Collapse All, model tooltip when underlying model differs, line-wrapped parameters, and wider endpoint column. - **ChatWindow**: Show deployment_name (user's input) in the target badge. - **types**: Add deployment_name to TargetInstance interface. ## Tests - Backend: test that explicit model_name is not overridden by env var (service, chat target, response target), test deployment_name preserved in identifier. - Frontend: 18 TargetTable tests covering alphabetical ordering, expand/collapse, active target auto-expand, expand/collapse all, tooltip, and params rendering. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes incorrect model name display in the GUI when an OpenAI underlying-model environment variable was unintentionally overriding explicitly provided model/deployment names. This aligns target identifiers and API responses with what users actually configured, while still preserving the underlying model when it’s intentionally provided.
Changes:
- Update OpenAI target initialization to only use the underlying-model env var when the deployment/model name is also coming from env vars.
- Add
deployment_nameto target identifiers and surface it through backend DTOs/API and frontend types/UI. - Redesign the TargetTable UI (grouped/collapsible sections, expand/collapse controls, tooltip when underlying model differs) and adjust related tests/e2e.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/target/test_openai_response_target.py | Adds unit test coverage for env var override behavior (response target). |
| tests/unit/target/test_openai_chat_target.py | Refines env var override tests; asserts deployment_name preservation. |
| tests/unit/backend/test_target_service.py | Adds service-layer test ensuring GUI create path preserves explicit model/deployment name. |
| pyrit/prompt_target/openai/openai_target.py | Adjusts underlying-model env var fallback logic to avoid overriding explicit model_name. |
| pyrit/prompt_target/common/prompt_target.py | Adds deployment_name into target identifiers alongside model_name. |
| pyrit/backend/models/targets.py | Extends TargetInstance DTO with deployment_name and clarifies model_name semantics. |
| pyrit/backend/mappers/target_mappers.py | Maps deployment_name from identifier params into TargetInstance responses. |
| frontend/src/types/index.ts | Adds deployment_name to the frontend TargetInstance type. |
| frontend/src/components/Config/TargetTable.tsx | Implements grouped/collapsible TargetTable UI and tooltip behavior. |
| frontend/src/components/Config/TargetTable.test.tsx | Updates/adds tests for new TargetTable grouping and expand/collapse behaviors. |
| frontend/src/components/Config/TargetTable.styles.ts | Adds styling to support multi-line parameter display. |
| frontend/src/components/Config/CreateTargetDialog.tsx | Adds optional “underlying model differs” toggle and field; sends underlying_model when set. |
| frontend/src/components/Config/CreateTargetDialog.test.tsx | Updates placeholder text in tests to match dialog copy change. |
| frontend/src/components/Chat/ChatWindow.tsx | Displays deployment_name in the active target badge when available. |
| frontend/e2e/config.spec.ts | Updates placeholder text used by e2e test to match dialog copy change. |
- Replace useMemo side effect with useEffect for active section expansion - Fix response target test to use correct env var (OPENAI_RESPONSES_UNDERLYING_MODEL) - Move inline imports to top of file in test_target_service.py Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
TargetConfig tests and e2e tests now expand sections before asserting on row-level data (model names, endpoints, Set Active buttons), since sections are collapsed by default in the new grouped layout. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ctive All e2e test files that interact with the target config view now expand sections first (via Expand All or clicking the section heading), since target table sections are collapsed by default. Affected files: accessibility, chat, config, converters, errors, flows. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
rlundeen2
reviewed
Apr 13, 2026
rlundeen2
reviewed
Apr 13, 2026
…esolution Addresses rlundeen2's review comments: 1. **Revert to flat table layout** with type filter dropdown instead of grouped/collapsible sections. Adds active target indicator above the table. Keeps model tooltip, param wrapping, and wider endpoint. 2. **Centralize underlying_model resolution** into resolve_underlying_model() in default_values.py. The policy (explicit value > env var only if model_name wasn't explicit > None) is now reusable by any target, not just OpenAI targets. 3. **Revert e2e tests** to original patterns (no expand/collapse needed with flat table). Fix TargetConfig/TargetTable unit tests for filter dropdown causing multiple text matches. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…tTarget PromptTarget.__init__ now accepts optional underlying_model_env_var and model_name_was_explicit params. When provided, it calls resolve_underlying_model() to resolve the underlying model centrally. This means any target (AzureML, HuggingFace, HTTP, etc.) can now use the same underlying_model resolution by passing underlying_model_env_var to the PromptTarget constructor. OpenAITarget no longer does this itself. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Backend: test_create_target_with_different_underlying_model verifies deployment_name and model_name are distinct when underlying_model is explicitly provided - Frontend: test toggle sends underlying_model when enabled, and omits it when toggle is off - Fix endpoint column to wrap long URLs instead of overflowing Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove env var fallback for underlying_model entirely. The rule is now: - model_name is always the truth for display/identity - underlying_model only overrides if someone explicitly passes it This eliminates the confusing 'was model_name explicit?' heuristic. The initializer configs that need underlying_model already pass it explicitly via their underlying_model_var config. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Env var reading for underlying_model is now solely the initializer's responsibility, not the target class's. Removed: - underlying_model_environment_variable attribute from OpenAITarget - self.underlying_model_environment_variable = '...' from all 7 subclasses - underlying_model_env_var and model_name_was_explicit params from PromptTarget - resolve_underlying_model() from default_values.py Unchanged: - .env files: all underlying model env vars stay - TargetConfig: still reads env vars via os.getenv(config.underlying_model_var) and passes as explicit underlying_model= kwargs to constructors Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Align naming across backend, frontend, and identifier: - model_name = what the user enters (used in API calls) - underlying_model_name = actual model if different (e.g., Azure deployment) Renamed in: identifier params, TargetInstance DTO, mapper, frontend types, TargetTable (ModelCell tooltip), ChatWindow badge, all tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ator Scope getByText queries for target type names to table element to avoid matching filter dropdown options. Scope Active badge query to table to avoid matching the active target indicator above. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
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.
Problem
Users reported that the GUI's target configuration view showed incorrect model names. When creating a target with a specific model name (e.g.,
claude-sonnet-4-6), the target table would displaygpt-4oinstead — pulled from theOPENAI_CHAT_UNDERLYING_MODELenvironment variable.This affected:
gpt-4oRoot Cause
OpenAITarget.__init__readunderlying_modelfrom a class-level env var (e.g.,OPENAI_CHAT_UNDERLYING_MODEL) and used it to overridemodel_namein the identifier. Since allOpenAIChatTargetinstances shared the same env var, unrelated targets got the wrong model name.Fix
Simplify underlying_model — remove class-level env var logic
Env var reading for
underlying_modelis now the initializer's responsibility, not the target class's. The.envfile andTargetConfigregistry stay exactly as-is.What changed:
OpenAITarget: Removedunderlying_model_environment_variableattribute and env var reading in__init__. Just passesunderlying_modelthrough directly.self.underlying_model_environment_variable = "..."from each_set_openai_env_configuration_vars()PromptTarget: Storesunderlying_modeldirectly (removedunderlying_model_env_varandmodel_name_was_explicitparams)default_values.py: Removedresolve_underlying_model()What stayed the same:
.envfiles: All underlying model env vars stayTargetConfigin the initializer: Still reads env vars viaos.getenv(config.underlying_model_var)and passes as explicitunderlying_model=kwargsConsistent naming
Aligned naming across the entire stack:
model_name= what the user enters (deployment name used in API calls)underlying_model_name= the actual model behind it, if different (e.g., Azure deploymentmy-deploy→ underlyinggpt-4o)Identifier changes
_create_identifier()now stores bothmodel_name(user's input) andunderlying_model_name(if set) as separate params. The backend mapper surfaces both to the frontend.Frontend
model_namein the target badge.Tests
Backend
test_create_target_model_name_not_overridden_by_env_var— env var no longer overrides explicit model_nametest_create_target_with_different_underlying_model— explicit underlying_model preserved separatelytest_get_identifier_ignores_underlying_model_env_var_*— chat target, response targettest_underlying_model_param_takes_precedence_over_env_var— explicit param winsFrontend