Skip to content

FIX: GUI target config shows wrong model name due to env var override#1590

Open
romanlutz wants to merge 1 commit intomicrosoft:mainfrom
romanlutz:fix/gui-custom-target-name
Open

FIX: GUI target config shows wrong model name due to env var override#1590
romanlutz wants to merge 1 commit intomicrosoft:mainfrom
romanlutz:fix/gui-custom-target-name

Conversation

@romanlutz
Copy link
Copy Markdown
Contributor

@romanlutz romanlutz commented Apr 11, 2026

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 display gpt-4o instead — the value from the OPENAI_CHAT_UNDERLYING_MODEL environment variable.

This affected:

  • GUI-created targets: Any model name entered in the Create Target dialog was overridden
  • Initializer-registered targets: Targets like DeepSeek, Mistral, Gemini, Llama, and GPT-4.1 all incorrectly showed gpt-4o as their model name

Root Cause

OpenAITarget.__init__ unconditionally fell back to the underlying_model env var (e.g., OPENAI_CHAT_UNDERLYING_MODEL) even when model_name was explicitly passed. The identifier then stored this env var value as model_name, which the frontend displayed.

# Before (bug): always falls back to env var
underlying_model_value = default_values.get_non_required_value(
    env_var_name=self.underlying_model_environment_variable, passed_value=underlying_model
)

Fix

Backend

  • openai_target.py: Only fall back to the underlying_model env var when model_name was also resolved from env vars. When model_name is explicitly passed but underlying_model is not, the env var no longer overrides it.
  • prompt_target.py: Store deployment_name (the user's actual model_name input) as a separate identifier param alongside model_name (underlying model), so both are always available.
  • target_mappers.py / targets.py: Surface deployment_name in the API response.

Frontend

  • CreateTargetDialog: Added a toggle for specifying a different underlying model (hidden by default), with an explanation about Azure deployment names vs actual model names.
  • TargetTable: Redesigned with:
    • Collapsible sections grouped alphabetically by target type
    • Expand All / Collapse All controls
    • Active target's section auto-expands
    • Model name tooltip when underlying model differs from deployment name
    • Line-wrapped parameters for readability
    • Wider endpoint column
  • ChatWindow: Shows deployment name in the target badge.

Tests

Backend (new)

  • test_get_identifier_ignores_underlying_model_env_var_when_model_name_explicit — chat target
  • test_get_identifier_ignores_underlying_model_env_var_when_model_name_explicit — response target
  • test_create_target_model_name_not_overridden_by_env_var — service layer (GUI create path)
  • Updated existing tests to verify deployment_name is preserved in identifiers

Frontend (18 TargetTable tests)

  • Alphabetical section ordering
  • Expand/collapse individual sections
  • Active target section auto-expand
  • Expand All / Collapse All
  • Tooltip for differing underlying model
  • Parameters rendering
  • Empty state, null fields, callback behavior

Screenshot

collapsed view

image

expanded section with tooltip for target where the underlying model differs from the model name

image

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>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

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_name to 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.

Comment on lines +101 to +107
// When active target changes, ensure its section is expanded
useMemo(() => {
if (activeGroup && !expandedSections.has(activeGroup)) {
setExpandedSections(prev => new Set([...prev, activeGroup]))
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [activeGroup])
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

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

useMemo is being used to perform a side effect (calling setExpandedSections). This runs during render and can trigger React warnings or render loops. Replace this block with a useEffect that expands the active section using a functional state update (so you don’t need to disable the exhaustive-deps rule).

Copilot uses AI. Check for mistakes.
Comment on lines +1294 to +1299
with patch.dict(os.environ, {"OPENAI_CHAT_UNDERLYING_MODEL": "gpt-4o"}):
target = OpenAIResponseTarget(
model_name="gpt-4.1",
endpoint="https://mock.azure.com/",
api_key="mock-api-key",
)
Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

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

This test patches OPENAI_CHAT_UNDERLYING_MODEL, but OpenAIResponseTarget reads OPENAI_RESPONSES_UNDERLYING_MODEL (see OpenAIResponseTarget._set_openai_env_configuration_vars). As written, the env var override path isn’t exercised for the response target. Patch the correct env var name so the test actually validates the intended behavior.

Copilot uses AI. Check for mistakes.
Comment on lines +283 to +285
import os
from unittest.mock import patch

Copy link

Copilot AI Apr 11, 2026

Choose a reason for hiding this comment

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

Imports were added inside the test method. This repo’s Python style guide requires imports at the top of the file (inline imports are only for documented circular-dependency breaks). Please move import os / from unittest.mock import patch to the module import section.

Copilot generated this review using guidance from repository custom instructions.
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