From 666b8bc08f04d54746dfbc34afea20fb3899d415 Mon Sep 17 00:00:00 2001 From: Paul Lizer Date: Wed, 2 Sep 2026 17:55:55 -0400 Subject: [PATCH] Make agent selection exclusive with model and reasoning in V2 chat Selecting an agent in the V2 composer left the Model picker showing a selected model and the Reasoning picker offering a level, though an agent can act on neither: it answers with its own azure_openai_gpt_deployment, and reasoning_effort only reaches the direct-model path. The invisible half mattered more. chatStore.sendMessage assigned the model identity unconditionally and then appended agent_info and reasoning_effort, so V2 posted all three. The route only lets an agent request pick its own model when no model identity was sent (should_use_default_model), so that branch never fired and an agent could answer through the wrong model. The rule now lives in one place, buildSelectionFields, read by both the toolbar and the request builder -- the original bug arose because each decided separately. With an agent selected the request carries agent_info alone; the model picker is shown as overridden rather than removed, keeping its selection and staying clickable because using it is how the user switches back; and the reasoning picker is hidden. This diverges from V1 deliberately: its getCurrentModelSelection reads the model select without checking that agent mode hid it, so it posts a model alongside an agent too. The test pins that asymmetry. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- application/single_app/config.py | 2 +- .../v2_ui/src/components/chat/Composer.tsx | 56 ++- .../v2_ui/src/components/ui/Dropdown.tsx | 24 +- .../v2_ui/src/lib/chatRequestSelection.ts | 91 ++++ application/v2_ui/src/lib/composerGating.ts | 32 +- application/v2_ui/src/stores/chatStore.ts | 63 ++- .../fixes/V2_AGENT_MODEL_EXCLUSIVITY_FIX.md | 153 +++++++ docs/explanation/release_notes.md | 13 + .../test_v2_agent_model_exclusivity.py | 395 ++++++++++++++++++ .../test_v2_agent_model_exclusivity_logic.ts | 255 +++++++++++ functional_tests/test_v2_chat_phase1_fixes.py | 9 +- .../test_v2_model_identity_and_scope.py | 15 +- 12 files changed, 1058 insertions(+), 50 deletions(-) create mode 100644 application/v2_ui/src/lib/chatRequestSelection.ts create mode 100644 docs/explanation/fixes/V2_AGENT_MODEL_EXCLUSIVITY_FIX.md create mode 100644 functional_tests/test_v2_agent_model_exclusivity.py create mode 100644 functional_tests/test_v2_agent_model_exclusivity_logic.ts diff --git a/application/single_app/config.py b/application/single_app/config.py index faa8bdf1a..c2fbdfcba 100644 --- a/application/single_app/config.py +++ b/application/single_app/config.py @@ -97,7 +97,7 @@ EXECUTOR_TYPE = 'thread' EXECUTOR_MAX_WORKERS = 30 SESSION_TYPE = 'filesystem' -VERSION = "0.261.033" +VERSION = "0.261.034" IS_DEVELOPMENT = is_development_env_enabled() SESSION_COOKIE_SAMESITE = os.getenv('SESSION_COOKIE_SAMESITE', 'Lax') diff --git a/application/v2_ui/src/components/chat/Composer.tsx b/application/v2_ui/src/components/chat/Composer.tsx index 21af639b7..e8f0b8ff7 100644 --- a/application/v2_ui/src/components/chat/Composer.tsx +++ b/application/v2_ui/src/components/chat/Composer.tsx @@ -22,6 +22,7 @@ import { useChatStore, type ComposerOptions } from '../../stores/chatStore'; import { useBootstrapStore } from '../../stores/bootstrapStore'; import { uploadDocument } from '../../lib/endpoints'; import { agentSelectionKey } from '../../lib/agents'; +import { hasResolvableAgent } from '../../lib/chatRequestSelection'; import { modelSelectionKey, findModel, type ModelCatalogEntry } from '../../lib/models'; import { resolveGating } from '../../lib/composerGating'; import { useUiStore } from '../../stores/uiStore'; @@ -93,6 +94,19 @@ export function Composer() { selectedDocumentIds: [], }); + // An agent supplies its own deployment and never receives a reasoning level, so a + // selection the server can actually resolve is what puts the model picker into its + // overridden state. Resolved against the catalog, not the raw key, so a stale selection + // does not silently deactivate a control that is still in force. + const agentActive = useMemo( + () => + hasResolvableAgent( + bootstrap?.catalogs?.agents as Record[] | undefined, + options.agentSelection, + ), + [bootstrap, options.agentSelection], + ); + // Which controls are relevant right now. Deep research and Read URLs depend on what is // currently typed, not only on what is enabled. const gating = useMemo( @@ -103,8 +117,16 @@ export function Composer() { webSearchActive: options.webSearch, urlAccessActive: options.urlAccess, imageGenerationActive: options.imageGeneration, + agentActive, }), - [text, features, options.webSearch, options.urlAccess, options.imageGeneration], + [ + text, + features, + options.webSearch, + options.urlAccess, + options.imageGeneration, + agentActive, + ], ); // A control that stops being relevant must not leave its option set behind it, or the @@ -184,6 +206,13 @@ export function Composer() { }), ); + // Names the agent in the model picker's tooltip. Saying which one is holding the model + // back is the difference between an explanation and a control that has simply gone dim. + const activeAgentLabel = agentActive + ? (agentOptions.find((option) => option.value === options.agentSelection)?.label ?? + 'the selected agent') + : null; + // Reasoning support is per-model, so the control appears only when the current model // actually offers a choice. Resolved from the catalog record rather than the label, // since the display name can be anything an administrator typed. @@ -289,16 +318,29 @@ export function Composer() {
{/* Hidden while generating an image: the request goes to an image - endpoint that does not take a chat model. */} + endpoint that does not take a chat model. Shown but overridden + while an agent is selected, since the agent brings its own + deployment — picking a model here is how the user gets back to + using one, so it stays usable rather than disabled. */} {gating.showModelPicker && ( setOptions((current) => ({ ...current, modelDeployment: value, + // Choosing a model is the way out of agent mode. The + // two cannot both apply, and the server reads a model + // sent alongside an agent as an override of it. + agentSelection: undefined, })) } /> @@ -314,6 +356,9 @@ export function Composer() { onChange={(value) => setOptions((current) => ({ ...current, + // The model selection is kept, not cleared: it is + // simply not in force, and it comes back the moment + // the agent is cleared. agentSelection: value, })) } @@ -410,9 +455,10 @@ export function Composer() { /> )} - {/* Only shown when the selected model offers a real choice; the - endpoint strips the parameter for models that reject it. */} - {reasoningLevels.length > 0 && ( + {/* Only shown when a reasoning level is a real choice: the selected + model has to offer one, and neither an agent nor image generation + can be in play, because neither carries the parameter. */} + {gating.showReasoning && reasoningLevels.length > 0 && ( (null); const selected = options.find((option) => option.value === value); + // What the trigger reads as. An overridden selection is still `selected` for the menu's + // check mark, but the trigger falls back to the placeholder so the row does not claim a + // choice that is not in force. + const triggerLabel = inactive ? placeholder : (selected?.label ?? placeholder); const measure = useCallback(() => { const element = containerRef.current; @@ -122,19 +139,20 @@ export function Dropdown({ }} aria-haspopup="listbox" aria-expanded={open} - title={compact ? (selected?.label ?? placeholder) : undefined} + title={title ?? (compact ? (selected?.label ?? placeholder) : undefined)} className={clsx( 'inline-flex h-9 items-center gap-2 rounded-xl border border-edge', 'bg-surface-1 text-sm font-medium transition-colors', 'hover:bg-surface-2 disabled:cursor-not-allowed disabled:opacity-50', compact ? 'w-9 justify-center' : 'max-w-[14rem] px-3', - selected ? 'text-text-1' : 'text-text-3', + selected && !inactive ? 'text-text-1' : 'text-text-3', + inactive && 'opacity-70', )} > {icon} {!compact && ( <> - {selected?.label ?? placeholder} + {triggerLabel} )} diff --git a/application/v2_ui/src/lib/chatRequestSelection.ts b/application/v2_ui/src/lib/chatRequestSelection.ts new file mode 100644 index 000000000..22776d4ab --- /dev/null +++ b/application/v2_ui/src/lib/chatRequestSelection.ts @@ -0,0 +1,91 @@ +// chatRequestSelection.ts +// Turning the composer's model / agent / reasoning selections into request fields. +// +// The rule this file exists for: AN AGENT WINS. When one is selected it answers with its own +// deployment (`azure_openai_gpt_deployment`, read by semantic_kernel_loader.py), and +// `reasoning_effort` only ever reaches the direct-model path +// (`_resolve_reasoning_effort_for_model` in route_backend_chats.py). So neither the model +// identity nor the reasoning level can be acted on, and sending them is not merely redundant: +// +// should_use_default_model = ( +// _has_chat_agent_selection(request_agent_info) +// and settings.get('enable_multi_model_endpoints', False) +// and not data.get('model_id') +// and not data.get('model_endpoint_id') +// ) +// (route_backend_chats.py) +// +// A model identity sent alongside `agent_info` reads to the server as a deliberate override, +// so the agent's own default-model handling never runs. The server has that handling for +// every configuration -- the multi-endpoint default, the first APIM deployment, and the +// configured default model -- and a request that always names a model never reaches any of it. +// +// The classic client does send a model alongside an agent, because `getCurrentModelSelection` +// reads the model select without checking that agent mode has hidden it. That is the bug +// being fixed here rather than the behaviour being matched. + +import { agentInfoForSelection } from './agents'; +import { modelIdentityForSelection, type ModelCatalogEntry } from './models'; +import type { Json } from './types'; + +export interface SelectionInput { + /** Agent catalog from bootstrap. */ + agents?: Record[]; + /** Model catalog from bootstrap. */ + models?: ModelCatalogEntry[]; + /** Picker selection key for the agent, if one is chosen. */ + agentSelection?: string; + /** Picker selection key for the model. Not a deployment name. */ + modelDeployment?: string; + reasoningEffort?: string; +} + +/** The mutually exclusive halves of a chat request's routing. */ +export interface SelectionFields { + agent_info?: Json; + model_deployment?: string; + model_id?: string; + model_endpoint_id?: string; + model_provider?: string; + reasoning_effort?: string; +} + +/** + * Build the routing fields for a chat request. + * + * Resolution is against the catalog, not the raw selection key, so a selection left over from + * a catalog that no longer contains it degrades to model mode rather than producing a request + * that claims an agent the server cannot find. + */ +export function buildSelectionFields(input: SelectionInput): SelectionFields { + const agentInfo = input.agentSelection + ? agentInfoForSelection(input.agents, input.agentSelection) + : null; + + if (agentInfo) { + return { agent_info: agentInfo as Json }; + } + + const fields: SelectionFields = { + ...modelIdentityForSelection(input.models, input.modelDeployment), + }; + + if (input.reasoningEffort) { + fields.reasoning_effort = input.reasoningEffort; + } + + return fields; +} + +/** + * Whether a selection resolves to an agent the server can act on. + * + * The composer greys out the model picker on this answer rather than on the raw selection + * key, so an unresolvable agent leaves the model picker looking exactly as live as it is. + */ +export function hasResolvableAgent( + agents: Record[] | undefined, + agentSelection: string | undefined, +): boolean { + return Boolean(agentSelection && agentInfoForSelection(agents, agentSelection)); +} diff --git a/application/v2_ui/src/lib/composerGating.ts b/application/v2_ui/src/lib/composerGating.ts index ccc448d64..02c6dc1b4 100644 --- a/application/v2_ui/src/lib/composerGating.ts +++ b/application/v2_ui/src/lib/composerGating.ts @@ -10,6 +10,12 @@ // // Showing every capability at all times is what makes the row crowded, and offering // "Read URLs" when there is no URL to read is an invitation to a confusing result. +// +// The model and reasoning controls are gated on the agent selection for the same reason. An +// agent answers with its own deployment (`azure_openai_gpt_deployment`, read by +// semantic_kernel_loader.py), and `reasoning_effort` only ever reaches the direct-model path +// (`_resolve_reasoning_effort_for_model` in route_backend_chats.py). Leaving either control +// looking live under an agent advertises a choice the request cannot act on. /** Matches the URL detection in the classic client. */ const URL_PATTERN = /https?:\/\/[^\s<>'"]+/gi; @@ -26,6 +32,8 @@ export interface GatingInput { webSearchActive: boolean; urlAccessActive: boolean; imageGenerationActive: boolean; + /** True while an agent is selected in the composer. */ + agentActive: boolean; } export interface ControlGating { @@ -42,6 +50,19 @@ export interface ControlGating { */ disabledByImageGeneration: boolean; showModelPicker: boolean; + /** + * The model picker is retained but overridden: an agent is selected, so it supplies the + * deployment. The picker stays usable, because choosing a model is how the user gets + * back out of agent mode. + */ + modelPickerInactive: boolean; + /** + * Whether a reasoning level is a real choice right now. Mirrors + * `updateReasoningButtonVisibility` in static/js/chat/chat-reasoning.js, which hides the + * control for image generation and for agents alike. Model support is a separate + * question, resolved from the catalog by the caller. + */ + showReasoning: boolean; } function enabled(features: Record, key: string): boolean { @@ -49,7 +70,14 @@ function enabled(features: Record, key: string): boolean { } export function resolveGating(input: GatingInput): ControlGating { - const { prompt, features, webSearchActive, urlAccessActive, imageGenerationActive } = input; + const { + prompt, + features, + webSearchActive, + urlAccessActive, + imageGenerationActive, + agentActive, + } = input; const urls = promptUrls(prompt); const hasUrls = urls.length > 0; @@ -70,5 +98,7 @@ export function resolveGating(input: GatingInput): ControlGating { showFileUpload: enabled(features, 'enable_chat_file_uploads'), disabledByImageGeneration: imageGenerationActive, showModelPicker: !imageGenerationActive, + modelPickerInactive: agentActive, + showReasoning: !agentActive && !imageGenerationActive, }; } diff --git a/application/v2_ui/src/stores/chatStore.ts b/application/v2_ui/src/stores/chatStore.ts index c48551248..c87b97ef1 100644 --- a/application/v2_ui/src/stores/chatStore.ts +++ b/application/v2_ui/src/stores/chatStore.ts @@ -28,8 +28,8 @@ import { streamChat, type ChatStreamHandlers, } from '../lib/sse'; -import { agentInfoForSelection } from '../lib/agents'; -import { modelIdentityForSelection, type ModelCatalogEntry } from '../lib/models'; +import { buildSelectionFields } from '../lib/chatRequestSelection'; +import type { ModelCatalogEntry } from '../lib/models'; import { resolveDocumentScope } from '../lib/documentScope'; import { messageThreadId } from '../lib/threads'; import { proposalSourceMessageId, type ImageProposalSpec } from '../lib/imageProposalSpec'; @@ -856,32 +856,20 @@ export const useChatStore = create((set, get) => ({ url_access_enabled: options.urlAccess, }; - // A model is identified by four fields together, not by its deployment name alone. - // Sending only the name makes the multi-endpoint resolver give up and fall back to - // the legacy endpoint, silently using a different model than the one chosen. + // Model identity, agent and reasoning level are mutually exclusive halves of the same + // decision, resolved in one place. An agent answers with its own deployment, and a + // model identity sent alongside `agent_info` reads to the server as an override of it. Object.assign( requestBody, - modelIdentityForSelection( - bootstrap?.catalogs?.models as ModelCatalogEntry[] | undefined, - options.modelDeployment, - ), + buildSelectionFields({ + agents: bootstrap?.catalogs?.agents as Record[] | undefined, + models: bootstrap?.catalogs?.models as ModelCatalogEntry[] | undefined, + agentSelection: options.agentSelection, + modelDeployment: options.modelDeployment, + reasoningEffort: options.reasoningEffort, + }), ); - if (options.agentSelection) { - // The server reads `agent_info` and requires a dict; a bare string is silently - // ignored, so the picker would appear to do nothing. - const agentInfo = agentInfoForSelection( - bootstrap?.catalogs?.agents as Record[] | undefined, - options.agentSelection, - ); - if (agentInfo) { - requestBody.agent_info = agentInfo; - } - } - if (options.reasoningEffort) { - requestBody.reasoning_effort = options.reasoningEffort; - } - await runChatStream(requestBody, conversationId, { isNewConversation }); }, @@ -1001,20 +989,21 @@ export const useChatStore = create((set, get) => ({ }); try { const bootstrap = useBootstrapStore.getState().data; - // `modelDeployment` holds the picker's selection key, so the deployment name - // is resolved from the catalog rather than sent as-is. - const identity = modelIdentityForSelection( - bootstrap?.catalogs?.models as ModelCatalogEntry[] | undefined, - options?.modelDeployment, - ); + // Same exclusive rule as a fresh send, so a retry cannot reintroduce the + // combination the server reads as a model override of the agent. The retry + // endpoint takes a flat deployment name, which `buildSelectionFields` has already + // resolved from the catalog — the option value is a selection key, not a name. + const selection = buildSelectionFields({ + agents: bootstrap?.catalogs?.agents as Record[] | undefined, + models: bootstrap?.catalogs?.models as ModelCatalogEntry[] | undefined, + agentSelection: options?.agentSelection, + modelDeployment: options?.modelDeployment, + reasoningEffort: options?.reasoningEffort, + }); const result = await retryMessageApi(messageId, { - model: identity.model_deployment, - reasoning_effort: options?.reasoningEffort, - agent_info: - agentInfoForSelection( - bootstrap?.catalogs?.agents as Record[] | undefined, - options?.agentSelection, - ) ?? undefined, + model: selection.model_deployment, + reasoning_effort: selection.reasoning_effort, + agent_info: selection.agent_info, }); if (!result?.chat_request) { throw new Error('The server did not return a retry request.'); diff --git a/docs/explanation/fixes/V2_AGENT_MODEL_EXCLUSIVITY_FIX.md b/docs/explanation/fixes/V2_AGENT_MODEL_EXCLUSIVITY_FIX.md new file mode 100644 index 000000000..96e19c0b8 --- /dev/null +++ b/docs/explanation/fixes/V2_AGENT_MODEL_EXCLUSIVITY_FIX.md @@ -0,0 +1,153 @@ +# V2 Agent / Model Exclusivity Fix + +**Fixed in version: 0.261.034** + +## Issue + +In the V2 chat composer the **Model**, **Agent** and **Reasoning** pickers were all +independently live. Selecting an agent left a model showing as selected and left a reasoning +level selectable, even though an agent can act on neither. The interface offered two choices +that had no effect, with nothing to indicate which one was actually in force. + +## Root cause + +Two halves, one visible and one not. + +### The controls were never told about each other + +`Composer.tsx` rendered the model picker on `gating.showModelPicker` (false only during image +generation) and the reasoning picker on model support alone. Neither consulted the agent +selection, so all three stayed live together. + +They are not independent. An agent answers with its own deployment: + +```python +# semantic_kernel_loader.py +deployment = agent.get("azure_openai_gpt_deployment") +``` + +and `reasoning_effort` only ever reaches the direct-model call parameters, through +`_resolve_reasoning_effort_for_model` into `api_params` / `stream_params` in +`route_backend_chats.py`. Under an agent, both controls are inert. + +### The request sent both halves, which the server reads as an override + +`chatStore.sendMessage` assigned the model identity unconditionally and then appended +`agent_info` and `reasoning_effort` after it, so V2 posted all three at once. The route only +lets an agent request choose its own model when no model identity was sent: + +```python +# route_backend_chats.py +should_use_default_model = ( + _has_chat_agent_selection(request_agent_info) + and settings.get('enable_multi_model_endpoints', False) + and not data.get('model_id') + and not data.get('model_endpoint_id') +) +``` + +Because V2 always sent `model_id` and `model_endpoint_id`, that branch never fired. The route +has agent-without-a-model handling for every configuration — the multi-endpoint default, the +first APIM deployment, and the configured default model — and V2 reached none of it. + +### Note on the classic client + +V1 has the same asymmetry and posts a model alongside an agent too: `getCurrentAgentSelection` +checks that agent mode is active, but `getCurrentModelSelection` reads the model select +without checking that agent mode has hidden it. Suppressing the fields is therefore a +deliberate divergence from V1, not a parity break, and `test_v2_agent_model_exclusivity.py` +pins that asymmetry so the decision is revisited if V1 changes. + +## Behaviour after the fix + +- Selecting an agent renders the model picker as its plain `Model` placeholder in muted + styling, with a tooltip naming the agent that supplies the model. The picker stays + **clickable**, and the menu still marks the retained model so it is visible what returns. +- Choosing a model clears the agent, because the two cannot both apply. +- Choosing an agent **retains** the model selection rather than clearing it; it is simply not + in force, and it comes back the moment the agent is cleared. +- The reasoning picker is hidden while an agent is selected. It is also hidden during image + generation, matching `updateReasoningButtonVisibility` in `static/js/chat/chat-reasoning.js`. +- With an agent selected, the request carries `agent_info` and nothing else — no + `model_deployment`, `model_id`, `model_endpoint_id`, `model_provider` or `reasoning_effort`. + +## Files modified + +| File | Change | +|------|--------| +| `application/v2_ui/src/lib/chatRequestSelection.ts` | **New.** `buildSelectionFields()` and `hasResolvableAgent()` — the single source of the agent-wins rule. | +| `application/v2_ui/src/lib/composerGating.ts` | Added `agentActive` input; added `modelPickerInactive` and `showReasoning` outputs. | +| `application/v2_ui/src/components/ui/Dropdown.tsx` | Added `inactive` (retained-but-overridden trigger state) and an explicit `title` tooltip. | +| `application/v2_ui/src/components/chat/Composer.tsx` | Wires the rule into the toolbar; choosing a model clears the agent; reasoning picker gated on `showReasoning`. | +| `application/v2_ui/src/stores/chatStore.ts` | `sendMessage` and `retryMessage` both build routing fields through `buildSelectionFields`. | +| `application/single_app/config.py` | `VERSION` `0.261.033` → `0.261.034`. | + +### Why `inactive` is not `disabled` + +The overridden model picker has to stay usable, because clicking it is how the user leaves +agent mode. It keeps its real `aria-haspopup` / `aria-expanded` semantics and stays keyboard +reachable; only its label and opacity change. A `disabled` control would have been a dead end. + +### Why the rule is a module rather than JSX + +The original bug arose precisely because "an agent wins" was written nowhere: the toolbar and +the request builder each made their own decision and drifted apart. `buildSelectionFields` is +what both now read, so the payload cannot disagree with what the user is being shown. + +## Testing + +| Test | Covers | +|------|--------| +| `functional_tests/test_v2_agent_model_exclusivity.py` | Establishes the server's contract first (the `should_use_default_model` condition, the APIM agent fallback, where `reasoning_effort` lands), then asserts the client honours it. Also runs the logic checks below. | +| `functional_tests/test_v2_agent_model_exclusivity_logic.ts` | 27 behavioural checks of `buildSelectionFields` and `resolveGating`, bundled with esbuild and run under node. | + +Validation performed: + +```powershell +cd application\v2_ui; npm run typecheck # clean +cd functional_tests +python test_v2_agent_model_exclusivity.py # 9/9, 27 logic checks +python test_v2_model_identity_and_scope.py # 9/9 +python test_v2_chat_phase1_fixes.py # 10/10 +python test_v2_conversation_details_and_gating.py # 9/9 +python test_v2_api_payload_shapes.py # 6/6 +python test_v2_dropdown_placement.py # 6/6 +``` + +The exclusivity test was verified against a deliberately reintroduced defect: making +`buildSelectionFields` emit `agent_info` alongside the model identity fails three checks +(`an agent selection sends no model identity at all`, `an agent selection sends no reasoning +level`, `nothing but agent_info is sent for an agent`). + +### Existing tests updated + +Three assertions in existing tests checked for a literal call site that has moved into the +shared rule. They were updated to follow the indirection rather than relaxed — each still +asserts the same guarantee, now in the module that owns it: + +- `test_v2_model_identity_and_scope.py` — `test_client_sends_the_whole_model_identity` and + `test_retry_resolves_the_model_the_same_way`. +- `test_v2_chat_phase1_fixes.py` — `test_agent_selection_is_sent_as_agent_info`. + +## Before / after + +| Situation | Before | After | +|-----------|--------|-------| +| Agent selected, model picker | Shows a model as selected and in force | Shows `Model`, muted, tooltip names the agent; selection retained | +| Agent selected, reasoning picker | Shown and selectable | Hidden | +| Agent selected, request body | `agent_info` + four model fields + `reasoning_effort` | `agent_info` only | +| Agent selected, server model resolution | Agent default-model handling skipped | Agent default-model handling runs | +| Picking a model while an agent is selected | Both remain selected | Agent is cleared | +| Clearing the agent | — | Previous model and reasoning level return | + +## Known limitations + +Image generation hides the model picker but leaves the agent picker visible, and the classic +client forces `image_generation = false` when an agent is explicitly tagged +(`chat-messages.js`). That is the same family of inconsistency but a separate decision, and it +is deliberately left unchanged here. + +`retryMessage` is still invoked with no options from `MessageActions.tsx`, so a retry uses +server defaults rather than the composer's current selection. That gap predates this change; +routing retry through `buildSelectionFields` means it cannot reintroduce the conflict when it +is eventually wired up. diff --git a/docs/explanation/release_notes.md b/docs/explanation/release_notes.md index f12df87e7..63b3dc7fd 100644 --- a/docs/explanation/release_notes.md +++ b/docs/explanation/release_notes.md @@ -2,6 +2,19 @@ For feature-focused and fix-focused drill-downs by version, see [Features by Version](https://github.com/microsoft/simplechat/tree/main/docs/explanation/features) and [Fixes by Version](https://github.com/microsoft/simplechat/tree/main/docs/explanation/fixes). +### **(v0.261.034)** + +#### Bug Fixes + +* **Picking An Agent No Longer Leaves A Model And Reasoning Level Pretending To Apply** + * In the new interface, choosing an agent left the **Model** picker still showing a selected model and the **Reasoning** picker still offering a level, even though an agent can act on neither — an agent answers with its own model, and reasoning levels only reach a directly chosen model. + * Worse, the request sent all three together, and the server reads a model sent alongside an agent as a deliberate instruction to override it. So an agent could quietly answer through the wrong model rather than the one it is configured with. + * Selecting an agent now dims the **Model** picker back to the plain word "Model" and hides **Reasoning**. The model you had chosen is remembered, not thrown away, and comes straight back when you clear the agent. + * The model picker stays clickable while it is dimmed: choosing a model is how you switch back, and doing so clears the agent for you. Its menu still shows a tick beside the model you had, so you can see what returns. + * Hovering the dimmed picker names the agent that is supplying the model. + * Reasoning is now also hidden while generating an image, matching the classic interface. + * (Ref: V2 chat, agent picker, model picker, reasoning effort, `chatRequestSelection.ts`) + ### **(v0.261.033)** #### New Features diff --git a/functional_tests/test_v2_agent_model_exclusivity.py b/functional_tests/test_v2_agent_model_exclusivity.py new file mode 100644 index 000000000..20510a805 --- /dev/null +++ b/functional_tests/test_v2_agent_model_exclusivity.py @@ -0,0 +1,395 @@ +#!/usr/bin/env python3 +""" +Functional test for V2 agent / model / reasoning exclusivity. + +Version: 0.261.034 +Implemented in: 0.261.034 + +In the V2 chat composer the Model, Agent and Reasoning pickers were all independently live. +Selecting an agent left a model showing as selected and left a reasoning level selectable, +but neither applies: an agent answers with its own ``azure_openai_gpt_deployment``, and +``reasoning_effort`` only ever reaches the direct-model path. + +The visible part was the smaller half. ``chatStore.sendMessage`` assigned the model identity +unconditionally and then appended ``agent_info`` and ``reasoning_effort``, so V2 posted all +three together -- and the server reads a model identity sent alongside ``agent_info`` as a +deliberate override, which meant V2 never reached any of the agent default-model handling the +route already has. + +This test first establishes the server's contract rather than assuming it, then asserts the +client honours it: an agent selection sends ``agent_info`` and nothing else, the model picker +is shown as overridden rather than removed or disabled, and the reasoning picker is hidden. +""" + +import os +import subprocess +import sys +from pathlib import Path + +sys.path.append(os.path.dirname(os.path.abspath(__file__))) + +from test_support.versioning import assert_app_version_at_least # noqa: E402 + +REPO_ROOT = Path(__file__).resolve().parents[1] +APP = REPO_ROOT / "application" / "single_app" +V2_SRC = REPO_ROOT / "application" / "v2_ui" / "src" +V2_UI = REPO_ROOT / "application" / "v2_ui" + +IMPLEMENTED_IN = "0.261.034" + + +def read(*parts) -> str: + return Path(*parts).read_text(encoding="utf-8") + + +def slice_between(text: str, start: str, end: str) -> str: + """Take the body of a store action, so an assertion is scoped to it.""" + begin = text.index(start) + return text[begin : text.index(end, begin)] + + +def test_version_is_at_least_the_implementing_release(): + """The fix must be present in the running application.""" + print("Testing the application version...") + try: + assert_app_version_at_least(IMPLEMENTED_IN) + print(" ok version is at least the implementing release") + return True + except Exception as e: + print(f"Test failed: {e}") + import traceback + + traceback.print_exc() + return False + + +def test_the_server_treats_a_model_alongside_an_agent_as_an_override(): + """Establish the server's contract rather than assuming it. + + This is the assertion the whole change rests on. If the route ever stops keying its + agent default-model handling on the absence of a model identity, suppressing those + fields becomes the wrong thing to do and this test should be the first to say so. + """ + print("Testing the server's agent-versus-model contract...") + try: + route = read(APP, "route_backend_chats.py") + + assert "should_use_default_model = (" in route, ( + "the route must still decide whether an agent request picks its own model" + ) + for condition in ( + "_has_chat_agent_selection(request_agent_info)", + "and not data.get('model_id')", + "and not data.get('model_endpoint_id')", + ): + assert condition in route, ( + f"expected {condition!r} in the should_use_default_model condition -- " + "an agent request only falls back to the default model when no model " + "identity was sent" + ) + + # The condition is what gates the multi-endpoint default. + assert "allow_default_selection=should_use_default_model" in route, ( + "the multi-endpoint resolver must be told when it may choose a default" + ) + + # And the two non-multi-endpoint configurations have their own agent handling, so + # omitting the model is a supported request shape in every deployment. + assert ( + "[GPT_CLIENT] Agent request without model_deployment; defaulting to first APIM deployment." + in route + ), "the APIM path must have its own agent-without-a-model fallback" + assert 'raise ValueError("No GPT model selected or configured.")' in route, ( + "the legacy single-endpoint path falls back to the configured default model" + ) + + print(" ok a model identity sent with an agent suppresses the agent default") + return True + except Exception as e: + print(f"Test failed: {e}") + import traceback + + traceback.print_exc() + return False + + +def test_an_agent_supplies_its_own_model_and_takes_no_reasoning_level(): + """Why the two controls are inert under an agent, established from the source.""" + print("Testing that an agent brings its own deployment...") + try: + loader = read(APP, "semantic_kernel_loader.py") + assert 'deployment = agent.get("azure_openai_gpt_deployment")' in loader, ( + "an agent answers with its own deployment, which is why the model picker " + "cannot apply to it" + ) + + route = read(APP, "route_backend_chats.py") + assert "def _resolve_reasoning_effort_for_model(" in route, ( + "reasoning effort is resolved per model" + ) + # It only ever lands on the direct-model call parameters. + assert "api_params['reasoning_effort'] = request_reasoning_effort" in route + assert "stream_params['reasoning_effort'] = request_reasoning_effort" in route + + print(" ok the agent path takes neither the picked model nor a reasoning level") + return True + except Exception as e: + print(f"Test failed: {e}") + import traceback + + traceback.print_exc() + return False + + +def test_the_divergence_from_the_classic_client_is_deliberate(): + """V1 posts a model alongside an agent. That is the bug, not the behaviour to match.""" + print("Testing the classic client's behaviour...") + try: + classic = read(APP, "static", "js", "chat", "chat-messages.js") + + # The classic payload always carries a model, whatever mode the UI is in... + assert "model_deployment: modelDeployment," in classic + assert "agent_info: agentInfo," in classic + # ...because the model selection is read without checking that agent mode hid it. + model_selection = slice_between( + classic, "function getCurrentModelSelection()", "function getCurrentAgentSelection()" + ) + assert "agent-select-container" not in model_selection, ( + "if the classic client ever starts checking agent mode before reading the " + "model select, revisit whether V2 should still diverge" + ) + # Whereas its agent selection *is* mode-aware, which is the asymmetry behind the bug. + agent_selection = slice_between( + classic, "function getCurrentAgentSelection()", "\n}\n" + ) + assert "areAgentsEnabled()" in agent_selection + + print(" ok the classic client's asymmetry is confirmed, so the divergence is known") + return True + except Exception as e: + print(f"Test failed: {e}") + import traceback + + traceback.print_exc() + return False + + +def test_the_request_builder_owns_the_exclusivity(): + """The rule lives in one place, not in JSX and the store separately.""" + print("Testing the shared request-selection helper...") + try: + selection = read(V2_SRC, "lib", "chatRequestSelection.ts") + + assert "export function buildSelectionFields(" in selection + assert "export function hasResolvableAgent(" in selection + # The identity contracts established by the earlier fix are reused, not re-derived. + assert "agentInfoForSelection" in selection, ( + "agent_info must keep its dict shape, which the server requires" + ) + assert "modelIdentityForSelection" in selection, ( + "the four-field model identity must still be resolved from the catalog" + ) + + print(" ok buildSelectionFields is the single source of the rule") + return True + except Exception as e: + print(f"Test failed: {e}") + import traceback + + traceback.print_exc() + return False + + +def test_the_store_no_longer_sends_both_halves(): + """Neither send nor retry may reintroduce the combination.""" + print("Testing the chat store...") + try: + store = read(V2_SRC, "stores", "chatStore.ts") + + send = slice_between(store, "sendMessage: async", "\n },") + assert "buildSelectionFields({" in send, ( + "sendMessage must build its routing fields through the shared rule" + ) + assert "modelIdentityForSelection(" not in send, ( + "the model identity must no longer be assigned unconditionally" + ) + assert "requestBody.agent_info =" not in send, ( + "agent_info must no longer be appended after a model identity" + ) + assert "requestBody.reasoning_effort =" not in send, ( + "reasoning_effort must no longer be appended independently" + ) + + retry = slice_between(store, "retryMessage: async", "\n },") + assert "buildSelectionFields({" in retry, ( + "retry must go through the same rule, or it can reintroduce the conflict" + ) + assert "model: options?.modelDeployment" not in retry, ( + "sending the selection key as the model name would not resolve" + ) + assert "model: selection.model_deployment" in retry, ( + "the retry endpoint takes a flat deployment name, resolved from the catalog" + ) + + print(" ok both request paths route through the exclusive rule") + return True + except Exception as e: + print(f"Test failed: {e}") + import traceback + + traceback.print_exc() + return False + + +def test_the_model_picker_is_overridden_rather_than_disabled(): + """It has to stay usable: choosing a model is the way back out of agent mode.""" + print("Testing the Dropdown inactive state...") + try: + dropdown = read(V2_SRC, "components", "ui", "Dropdown.tsx") + + assert "inactive?: boolean;" in dropdown + assert "inactive = false," in dropdown + # The trigger falls back to the placeholder while the value is retained. + assert "const triggerLabel = inactive ? placeholder :" in dropdown, ( + "an overridden picker shows its placeholder rather than the retained label" + ) + # But the menu still marks the retained value, so what returns is visible. + assert "const isSelected = option.value === value;" in dropdown + + # Crucially, inactive must not feed the disabled attribute. + assert "disabled={disabled}" in dropdown, "disabled stays its own, separate prop" + assert "disabled={disabled || inactive}" not in dropdown, ( + "an overridden picker must stay clickable, since using it clears the agent" + ) + + print(" ok the overridden picker is muted but still usable") + return True + except Exception as e: + print(f"Test failed: {e}") + import traceback + + traceback.print_exc() + return False + + +def test_the_composer_wires_the_rule_into_the_toolbar(): + """The visible half: what the user sees must match what is sent.""" + print("Testing the composer toolbar...") + try: + gating = read(V2_SRC, "lib", "composerGating.ts") + assert "agentActive: boolean;" in gating + assert "modelPickerInactive: boolean;" in gating + assert "showReasoning: boolean;" in gating + assert "modelPickerInactive: agentActive," in gating + assert "showReasoning: !agentActive && !imageGenerationActive," in gating + + composer = read(V2_SRC, "components", "chat", "Composer.tsx") + + # The agent must be resolved against the catalog, not taken from the raw key. + assert "hasResolvableAgent(" in composer + assert "agentActive," in composer, "the gating rule must be told about the agent" + + assert "inactive={gating.modelPickerInactive}" in composer + # Picking a model is what takes the override off. + assert "agentSelection: undefined," in composer, ( + "choosing a model must clear the agent, since the two cannot both apply" + ) + # Picking an agent must not clear the model: it is retained, just not in force. + assert "modelDeployment: undefined" not in composer, ( + "the model selection is retained under an agent and comes back when it is cleared" + ) + + assert "{gating.showReasoning && reasoningLevels.length > 0 && (" in composer, ( + "the reasoning picker must be gated on the rule as well as on model support" + ) + + print(" ok the toolbar reflects the same rule as the request") + return True + except Exception as e: + print(f"Test failed: {e}") + import traceback + + traceback.print_exc() + return False + + +def test_the_typescript_logic_checks_pass(): + """Run the bundled behaviour checks, when the front-end toolchain is installed.""" + print("Testing the behaviour of the rule itself...") + try: + check = Path(__file__).with_name("test_v2_agent_model_exclusivity_logic.ts") + assert check.exists(), "the logic check file is missing" + + if not (V2_UI / "node_modules").exists(): + print(" -- skipped the TypeScript checks: run npm install in application/v2_ui") + return True + + # The check file lives in functional_tests/, which has no node_modules of its own, so + # bare imports are left for node to resolve from where the bundle is written. + bundle = V2_UI / "node_modules" / ".cache-agent-model-exclusivity-check.mjs" + try: + subprocess.run( + [ + "npx", + "esbuild", + str(check), + "--bundle", + "--platform=node", + "--format=esm", + "--packages=external", + f"--outfile={bundle}", + "--log-level=error", + ], + cwd=str(V2_UI), + check=True, + shell=(sys.platform == "win32"), + ) + result = subprocess.run( + ["node", str(bundle)], + cwd=str(V2_UI), + capture_output=True, + text=True, + shell=(sys.platform == "win32"), + ) + finally: + if bundle.exists(): + bundle.unlink() + + if result.returncode != 0: + print(result.stdout) + print(result.stderr) + raise AssertionError("the TypeScript logic checks failed") + + passed = result.stdout.count(" ok ") + assert passed >= 25, f"expected the full check suite, saw {passed} checks" + print(f" ok {passed} TypeScript logic checks passed") + return True + except Exception as e: + print(f"Test failed: {e}") + import traceback + + traceback.print_exc() + return False + + +TESTS = [ + test_version_is_at_least_the_implementing_release, + test_the_server_treats_a_model_alongside_an_agent_as_an_override, + test_an_agent_supplies_its_own_model_and_takes_no_reasoning_level, + test_the_divergence_from_the_classic_client_is_deliberate, + test_the_request_builder_owns_the_exclusivity, + test_the_store_no_longer_sends_both_halves, + test_the_model_picker_is_overridden_rather_than_disabled, + test_the_composer_wires_the_rule_into_the_toolbar, + test_the_typescript_logic_checks_pass, +] + + +if __name__ == "__main__": + results = [] + for test in TESTS: + print(f"\nRunning {test.__name__}...") + results.append(test()) + + print(f"\nResults: {sum(results)}/{len(results)} tests passed") + sys.exit(0 if all(results) else 1) diff --git a/functional_tests/test_v2_agent_model_exclusivity_logic.ts b/functional_tests/test_v2_agent_model_exclusivity_logic.ts new file mode 100644 index 000000000..20d71036c --- /dev/null +++ b/functional_tests/test_v2_agent_model_exclusivity_logic.ts @@ -0,0 +1,255 @@ +// test_v2_agent_model_exclusivity_logic.ts +// Behavioural checks for the V2 agent / model / reasoning exclusivity. +// +// Version: 0.261.034 +// Implemented in: 0.261.034 +// +// The V2 interface has no unit test runner, and adding one would pull in a test framework for +// a single file. This is bundled with the esbuild that Vite already brings in and run under +// node by test_v2_agent_model_exclusivity.py, which skips it when the front-end toolchain has +// not been installed. +// +// What it protects, in rough order of importance: +// +// - An agent selection must never travel with a model identity. The server reads +// `model_id` / `model_endpoint_id` alongside `agent_info` as a deliberate override, so its +// agent default-model handling never runs. That is a wrong-model bug, not a tidiness one. +// - An agent selection must never travel with a reasoning level, which only reaches the +// direct-model path. +// - With no agent, the full four-field model identity must still be sent, unchanged. That is +// the guarantee an earlier fix established and this change must not regress. +// - A selection key that no longer matches the catalog must degrade to model mode rather +// than suppressing the model on the strength of an agent the server cannot resolve. + +import { + buildSelectionFields, + hasResolvableAgent, +} from '../application/v2_ui/src/lib/chatRequestSelection'; +import { resolveGating } from '../application/v2_ui/src/lib/composerGating'; +import type { ModelCatalogEntry } from '../application/v2_ui/src/lib/models'; + +let failures = 0; +function check(name: string, condition: boolean, detail?: unknown) { + if (condition) { + console.log(` ok ${name}`); + } else { + failures += 1; + console.log(`FAIL ${name}`, detail ?? ''); + } +} + +/* ---- fixtures ---- */ + +/** Shaped like `_build_chat_model_catalog` output, including the per-endpoint selection key. */ +const MODELS: ModelCatalogEntry[] = [ + { + selection_key: 'personal::endpoint-a:gpt-5', + model_id: 'gpt-5', + deployment_name: 'gpt-5-deploy', + endpoint_id: 'endpoint-a', + provider: 'azure_openai', + display_name: 'GPT-5 (East)', + }, + { + // The same deployment name on a second endpoint: why the key is not the name. + selection_key: 'personal::endpoint-b:gpt-5', + model_id: 'gpt-5', + deployment_name: 'gpt-5-deploy', + endpoint_id: 'endpoint-b', + provider: 'azure_openai', + display_name: 'GPT-5 (West)', + }, +]; + +const AGENTS: Record[] = [ + { + id: 'agent-1', + name: 'researcher', + display_name: 'Researcher', + is_global: false, + is_group: true, + group_id: 'group-9', + group_name: 'Research', + }, +]; + +const BASE_GATING = { + prompt: '', + features: { + enable_web_search: true, + enable_image_generation: true, + enable_chat_file_uploads: true, + } as Record, + webSearchActive: false, + urlAccessActive: false, + imageGenerationActive: false, + agentActive: false, +}; + +const MODEL_FIELDS = [ + 'model_deployment', + 'model_id', + 'model_endpoint_id', + 'model_provider', +] as const; + +/* ---- an agent suppresses the model identity and the reasoning level ---- */ + +{ + const fields = buildSelectionFields({ + agents: AGENTS, + models: MODELS, + agentSelection: 'agent-1', + modelDeployment: 'personal::endpoint-a:gpt-5', + reasoningEffort: 'high', + }); + + check('an agent selection produces agent_info', Boolean(fields.agent_info)); + + const leaked = MODEL_FIELDS.filter((field) => field in fields); + check( + 'an agent selection sends no model identity at all', + leaked.length === 0, + leaked, + ); + check( + 'an agent selection sends no reasoning level', + !('reasoning_effort' in fields), + fields, + ); + check( + 'nothing but agent_info is sent for an agent', + Object.keys(fields).join(',') === 'agent_info', + Object.keys(fields), + ); + + // The seven fields the route resolves an agent against. + const info = fields.agent_info as Record; + check( + 'agent_info keeps the full identity the route reads', + info.id === 'agent-1' && + info.name === 'researcher' && + info.display_name === 'Researcher' && + info.is_global === false && + info.is_group === true && + info.group_id === 'group-9' && + info.group_name === 'Research', + info, + ); +} + +/* ---- without an agent, the model identity is unchanged ---- */ + +{ + const fields = buildSelectionFields({ + agents: AGENTS, + models: MODELS, + modelDeployment: 'personal::endpoint-b:gpt-5', + reasoningEffort: 'medium', + }); + + check('no agent means no agent_info', !('agent_info' in fields)); + check( + 'the endpoint the user actually picked is what is sent', + fields.model_endpoint_id === 'endpoint-b', + fields, + ); + check( + 'the deployment name is resolved from the catalog, not the selection key', + fields.model_deployment === 'gpt-5-deploy', + fields, + ); + check('the model id travels with its endpoint', fields.model_id === 'gpt-5', fields); + check('the provider travels too', fields.model_provider === 'azure_openai', fields); + check('the reasoning level is sent', fields.reasoning_effort === 'medium', fields); +} + +/* ---- an agent selection the catalog cannot resolve must not suppress the model ---- */ + +{ + const fields = buildSelectionFields({ + agents: AGENTS, + models: MODELS, + agentSelection: 'agent-that-was-deleted', + modelDeployment: 'personal::endpoint-a:gpt-5', + reasoningEffort: 'low', + }); + + check('an unresolvable agent produces no agent_info', !('agent_info' in fields)); + check( + 'an unresolvable agent falls back to the model, rather than to nothing', + fields.model_endpoint_id === 'endpoint-a' && + fields.model_deployment === 'gpt-5-deploy', + fields, + ); + check( + 'an unresolvable agent leaves the reasoning level in place', + fields.reasoning_effort === 'low', + fields, + ); + check( + 'the composer agrees the agent is not in force', + hasResolvableAgent(AGENTS, 'agent-that-was-deleted') === false, + ); + check( + 'a resolvable agent is reported as in force', + hasResolvableAgent(AGENTS, 'agent-1') === true, + ); + check('no selection is not an agent', hasResolvableAgent(AGENTS, undefined) === false); +} + +/* ---- an empty request stays empty ---- */ + +{ + const fields = buildSelectionFields({ agents: AGENTS, models: MODELS }); + check( + 'no selection at all sends no routing fields', + Object.keys(fields).length === 0, + fields, + ); +} + +/* ---- the toolbar reflects the same rule ---- */ + +{ + const withAgent = resolveGating({ ...BASE_GATING, agentActive: true }); + const withoutAgent = resolveGating({ ...BASE_GATING, agentActive: false }); + const withImage = resolveGating({ ...BASE_GATING, imageGenerationActive: true }); + + check('an agent marks the model picker overridden', withAgent.modelPickerInactive); + check( + 'the model picker is still shown, not removed', + withAgent.showModelPicker, + withAgent, + ); + check('an agent hides the reasoning picker', withAgent.showReasoning === false); + + check('no agent leaves the model picker live', withoutAgent.modelPickerInactive === false); + check('no agent leaves the reasoning picker available', withoutAgent.showReasoning); + + // Matches updateReasoningButtonVisibility in static/js/chat/chat-reasoning.js. + check('image generation hides the reasoning picker', withImage.showReasoning === false); + check( + 'image generation still hides the model picker outright', + withImage.showModelPicker === false, + ); + check( + 'image generation alone does not mark the model picker merely overridden', + withImage.modelPickerInactive === false, + ); + + // Nothing else in the gating rule may move because an agent was picked. + const unrelated = ( + ['showDocuments', 'showWeb', 'showImage', 'showUrlAccess', 'showDeepResearch', 'showFileUpload', 'disabledByImageGeneration'] as const + ).filter((key) => withAgent[key] !== withoutAgent[key]); + check( + 'selecting an agent changes nothing else in the toolbar', + unrelated.length === 0, + unrelated, + ); +} + +if (failures > 0) { + console.log(`\n${failures} check(s) failed`); + process.exit(1); +} diff --git a/functional_tests/test_v2_chat_phase1_fixes.py b/functional_tests/test_v2_chat_phase1_fixes.py index 08f0b89f6..6c374c539 100644 --- a/functional_tests/test_v2_chat_phase1_fixes.py +++ b/functional_tests/test_v2_chat_phase1_fixes.py @@ -265,9 +265,16 @@ def test_agent_selection_is_sent_as_agent_info(): assert field in agents, f"agent_info should carry {field}" store = read(V2_SRC, "stores", "chatStore.ts") - assert "requestBody.agent_info" in store, ( + # agent_info now reaches the request through the shared selection rule, which is + # also what stops a model identity travelling with it. Following the indirection + # keeps this about the guarantee rather than about a particular call site. + assert "buildSelectionFields" in store, ( "The chat request must send agent_info" ) + selection = read(V2_SRC, "lib", "chatRequestSelection.ts") + assert "agent_info" in selection, ( + "The selection rule must emit agent_info, which is the key the server reads" + ) # The original defect. assert "agent_selection =" not in store, ( "agent_selection is not a key the server reads" diff --git a/functional_tests/test_v2_model_identity_and_scope.py b/functional_tests/test_v2_model_identity_and_scope.py index f2e696289..ae4bef2f7 100644 --- a/functional_tests/test_v2_model_identity_and_scope.py +++ b/functional_tests/test_v2_model_identity_and_scope.py @@ -95,9 +95,16 @@ def test_client_sends_the_whole_model_identity(): ) store = read(V2_SRC, "stores", "chatStore.ts") - assert "modelIdentityForSelection" in store, ( + # The resolved identity now reaches the request through the shared selection rule, + # which is also what suppresses it when an agent is selected. Following the + # indirection keeps this assertion about the guarantee rather than about a call site. + assert "buildSelectionFields" in store, ( "The chat request must carry the resolved identity" ) + selection = read(V2_SRC, "lib", "chatRequestSelection.ts") + assert "modelIdentityForSelection" in selection, ( + "The selection rule must resolve the full identity from the catalog" + ) # The original defect: only the deployment name was sent. assert "requestBody.model_deployment = options.modelDeployment" not in store, ( @@ -216,10 +223,14 @@ def test_retry_resolves_the_model_the_same_way(): retry = store[store.index("retryMessage: async") :] retry = retry[: retry.index("\n },")] - assert "modelIdentityForSelection" in retry, ( + assert "buildSelectionFields" in retry, ( "Retry must resolve the deployment name from the catalog, since the option " "value is a selection key rather than a model name" ) + assert "model: selection.model_deployment" in retry, ( + "The retry endpoint takes a flat deployment name, which the selection rule " + "has already resolved" + ) assert "model: options?.modelDeployment" not in retry, ( "Sending the selection key as the model name would not resolve" )