From a7b8e04d65febe4be7d0979b73513ea704800672 Mon Sep 17 00:00:00 2001 From: Paul Lizer Date: Mon, 7 Sep 2026 13:01:42 -0400 Subject: [PATCH] Fix orchestration follow-ups and selected model routing Accept valid nullable resolver output, preserve authorized model selections through planning and execution, and report the model that actually answered. Normalize Anthropic completion flags and add regression coverage and documentation for version 0.261.101. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- application/single_app/config.py | 2 +- .../functions_orchestration_adapters.py | 17 +- .../functions_orchestration_context.py | 3 +- .../functions_orchestration_events.py | 14 +- .../functions_orchestration_executor.py | 10 +- .../functions_orchestration_models.py | 283 +++++++++++ .../functions_orchestration_planner.py | 257 +++++++--- .../single_app/model_endpoint_clients.py | 19 +- .../single_app/route_backend_orchestration.py | 179 +++++-- docs/admin/orchestration.md | 18 +- .../features/CHAT_ORCHESTRATION.md | 72 ++- .../ORCHESTRATION_CONVERSATION_CONTEXT_FIX.md | 96 +++- .../ORCHESTRATION_MODEL_SELECTION_FIX.md | 156 +++++++ .../test_model_endpoint_protocol_inference.py | 29 +- ...test_orchestration_conversation_context.py | 154 +++++- ...chestration_conversation_context_routes.py | 382 ++++++++++++++- .../test_orchestration_model_selection.py | 440 ++++++++++++++++++ ...t_v2_orchestration_conversation_context.py | 22 +- 18 files changed, 1993 insertions(+), 160 deletions(-) create mode 100644 application/single_app/functions_orchestration_models.py create mode 100644 docs/explanation/fixes/ORCHESTRATION_MODEL_SELECTION_FIX.md create mode 100644 functional_tests/test_orchestration_model_selection.py diff --git a/application/single_app/config.py b/application/single_app/config.py index 5f640a5ab..1f873d83d 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.099" +VERSION = "0.261.101" IS_DEVELOPMENT = is_development_env_enabled() SESSION_COOKIE_SAMESITE = os.getenv('SESSION_COOKIE_SAMESITE', 'Lax') diff --git a/application/single_app/functions_orchestration_adapters.py b/application/single_app/functions_orchestration_adapters.py index 45d407612..196773689 100644 --- a/application/single_app/functions_orchestration_adapters.py +++ b/application/single_app/functions_orchestration_adapters.py @@ -44,7 +44,7 @@ lives in ``route_backend_chats``, importing which at module load would be a circular import -- so the same lazy pattern is used uniformly rather than only where it is strictly forced. -Version: 0.261.099 +Version: 0.261.101 """ import json @@ -972,17 +972,14 @@ def _finalize_source_review( ) -def _resolve_source_review_planner(settings): +def _resolve_source_review_planner(settings, context=None): """The optional client for research query and link-selection planning. - perform_source_review takes a planner client/model so it can decide which discovered links - are worth reading. The context's ``invoke_prompt`` closure has already resolved a client, - but it is a ``call(prompt) -> text`` seam by design and does not expose the client object, - so we resolve one the same way the planner does. ``resolve_planner_client`` handles APIM, - managed identity and key auth and returns the planner deployment -- the right model for an - internal planning call rather than for writing the final answer. Expected configuration - failures leave the existing backup query/link planning available. + A running orchestration supplies its already-authorized, protocol-aware client. + Standalone callers retain the legacy optional planner and backup planning behavior. """ + if getattr(context, 'planner_client', None) is not None: + return context.planner_client, context.planner_deployment from functions_orchestration_planner import PlannerError, resolve_planner_client try: @@ -1110,7 +1107,7 @@ def run_deep_research(step, context, *, settings, user_id, emit, cancel_requeste 'Deep research is not enabled or permitted.', ) - planner_client, planner_model = _resolve_source_review_planner(settings) + planner_client, planner_model = _resolve_source_review_planner(settings, context) if _is_cancelled(cancel_requested): return _cancelled_result('Cancelled before deep research.') diff --git a/application/single_app/functions_orchestration_context.py b/application/single_app/functions_orchestration_context.py index d4a98dc34..79c4576ac 100644 --- a/application/single_app/functions_orchestration_context.py +++ b/application/single_app/functions_orchestration_context.py @@ -27,7 +27,7 @@ A user who picked a document and then watched the planner search their whole workspace would rightly conclude the control did nothing. -Version: 0.261.087 +Version: 0.261.101 """ import hashlib @@ -171,6 +171,7 @@ def resolve_seeds(request_data): ), 'agent': agent, 'model': model or None, + 'reasoning_effort': _text(request_data.get('reasoning_effort')), 'prompt': prompt, # A user who switched web search on has said something about intent even in # orchestration mode, so it is carried through as a constraint rather than dropped. diff --git a/application/single_app/functions_orchestration_events.py b/application/single_app/functions_orchestration_events.py index 5d4cde33c..0af8c53b5 100644 --- a/application/single_app/functions_orchestration_events.py +++ b/application/single_app/functions_orchestration_events.py @@ -30,7 +30,7 @@ plan card ticks specific steps by id, and reverse-engineering that from prose would be guesswork. -Version: 0.261.085 +Version: 0.261.101 """ import json @@ -283,6 +283,10 @@ def build_run_done_event( plan_summary=None, status='completed', agent_citations=None, + model_deployment_name=None, + model_provider=None, + model_endpoint_id=None, + model_id=None, ): """Terminal frame of the run endpoint. @@ -307,6 +311,14 @@ def build_run_done_event( 'generated_artifacts': list(artifacts or ()), 'orchestration': plan_summary or {}, 'status': status, + **{ + key: value for key, value in { + 'model_deployment_name': model_deployment_name, + 'model_provider': model_provider, + 'model_endpoint_id': model_endpoint_id, + 'model_id': model_id, + }.items() if value is not None + }, }) diff --git a/application/single_app/functions_orchestration_executor.py b/application/single_app/functions_orchestration_executor.py index 4f7c79f3a..15409e6ee 100644 --- a/application/single_app/functions_orchestration_executor.py +++ b/application/single_app/functions_orchestration_executor.py @@ -13,10 +13,10 @@ Two properties are worth stating because they are the reason this is an engine and not a loop: -**A plan always produces an answer.** A gather step can fail, be skipped because its +**A plan always attempts an answer.** A gather step can fail, be skipped because its dependency failed, or be cut off by a budget, and the run still reaches ``respond`` and answers with whatever evidence survived. The terminal step is therefore exempt from every -skip rule; the only thing that stops it is an explicit cancellation. +skip rule except an explicit cancellation. A failed answer completion still fails the run. **Access is re-checked at answer time, not trusted from plan time.** Between the planner naming a document and the executor answering from it, the user's access to that document can @@ -31,7 +31,7 @@ itself. The route owns that loop, because only the route can decide to spend another planner round trip. -Version: 0.261.087 +Version: 0.261.101 """ import logging @@ -164,6 +164,8 @@ def __init__( user_id=None, turn_index=0, invoke_prompt=None, + planner_client=None, + planner_deployment=None, user_message='', user_message_id=None, resolved_message=None, @@ -200,6 +202,8 @@ def __init__( self.turn_index = turn_index self.invoke_prompt = invoke_prompt + self.planner_client = planner_client + self.planner_deployment = planner_deployment self.user_message = user_message self.user_message_id = user_message_id self.resolved_message = resolved_message if resolved_message is not None else user_message diff --git a/application/single_app/functions_orchestration_models.py b/application/single_app/functions_orchestration_models.py new file mode 100644 index 000000000..218d0d990 --- /dev/null +++ b/application/single_app/functions_orchestration_models.py @@ -0,0 +1,283 @@ +# functions_orchestration_models.py +"""Authorized model bindings for orchestration planning and execution. + +Version: 0.261.101 +""" + +from dataclasses import dataclass, field +from types import SimpleNamespace +from typing import Any + +from model_endpoint_clients import ( + MODEL_ENDPOINT_PROTOCOL_AZURE_OPENAI, + ModelEndpointBehavior, + infer_model_endpoint_protocol, +) + + +REASONING_COMPLETION_BUDGET = 8192 +MODEL_IDENTITY_FIELDS = ('model_deployment', 'model_id', 'model_endpoint_id', 'model_provider') +PLANNER_MODEL_FIELDS = { + 'model_deployment': 'chat_orchestration_planner_deployment', + 'model_id': 'chat_orchestration_planner_model_id', + 'model_endpoint_id': 'chat_orchestration_planner_model_endpoint_id', + 'model_provider': 'chat_orchestration_planner_model_provider', +} + + +class OrchestrationModelError(ValueError): + """The requested model cannot be used without changing the user's selection.""" + + def __init__(self): + super().__init__('The selected model is unavailable. Choose an enabled model you can access.') + + +def _text(value): + return str(value or '').strip() + + +class _PlannerCompletions: + def __init__(self, model): + self.model = model + + def create(self, **kwargs): + return self.model.create_completion(**kwargs) + + +@dataclass +class OrchestrationModel: + client: Any = field(repr=False) + deployment: str + provider: str = 'aoai' + endpoint_id: str = '' + model_id: str = '' + behavior_name: str = '' + response_length: int | None = None + reasoning_effort: str = '' + source: str = 'legacy' + _answer_selection: dict[str, str] | None = field(default=None, repr=False) + _closed: bool = field(default=False, init=False, repr=False) + + def answer_model_selection(self): + """Pin the answer choice even when this binding is a separate planner.""" + return dict(self._answer_selection) if self._answer_selection is not None else { + key: value for key, value in { + 'model_deployment': self.deployment, + 'model_provider': self.provider, + 'model_endpoint_id': self.endpoint_id, + 'model_id': self.model_id, + }.items() if value + } + + def metadata(self): + return { + key: value for key, value in { + 'model_deployment_name': self.deployment, + 'model_provider': self.provider, + 'model_endpoint_id': self.endpoint_id, + 'model_id': self.model_id, + }.items() if value + } + + def as_planner_client(self): + """Keep the chat-completions interface used by planning and source review.""" + return SimpleNamespace(chat=SimpleNamespace(completions=_PlannerCompletions(self))) + + def create_completion(self, *, use_model_response_length=False, **kwargs): + parameters = dict(kwargs) + if parameters.get('model', self.deployment) != self.deployment: + raise OrchestrationModelError() + parameters['model'] = self.deployment + behavior = ModelEndpointBehavior(self.provider, self.behavior_name or self.deployment) + limit = parameters.pop('max_completion_tokens', None) + legacy_limit = parameters.pop('max_tokens', None) + limit = limit if limit is not None else legacy_limit + if use_model_response_length and self.response_length is not None: + limit = self.response_length + elif behavior.is_openai_reasoning_model and limit is not None: + # Reasoning tokens share the completion budget with the visible JSON or answer. + limit = max(limit, REASONING_COMPLETION_BUDGET) + if limit is not None: + parameters[behavior.response_length_parameter] = limit + if behavior.is_openai_reasoning_model: + parameters.pop('temperature', None) + effort = behavior.resolve_reasoning_effort( + parameters.pop('reasoning_effort', None) or self.reasoning_effort + ) + if effort: + parameters['reasoning_effort'] = effort + return self.client.chat.completions.create(**parameters) + + def close(self): + if not self._closed: + self._closed = True + close = getattr(self.client, 'close', None) + if callable(close): + close() + + +def _resolve_legacy_binding(settings, *, deployment='', reasoning_effort='', source='legacy', + answer_selection=None): + # Keep the legacy client seam lazy to avoid an import cycle with the planner. + from functions_orchestration_planner import resolve_planner_client + + client, resolved_deployment = resolve_planner_client(settings) + return OrchestrationModel( + client, deployment or resolved_deployment, reasoning_effort=reasoning_effort, + source=source, _answer_selection=answer_selection, + ) + + +def has_planner_model_override(settings): + return any(_text((settings or {}).get(key)) for key in PLANNER_MODEL_FIELDS.values()) + + +def _resolve_planner_binding(settings, *, user_id, seeds, identity_context, answer_selection): + selection = { + field: _text(settings.get(key)) for field, key in PLANNER_MODEL_FIELDS.items() + } + if selection['model_endpoint_id'] or selection['model_id']: + binding = resolve_orchestration_model( + settings, user_id=user_id, + seeds={**seeds, 'model': selection, 'reasoning_effort': ''}, + identity_context=identity_context, + ) + binding.source = 'planner_override' + binding._answer_selection = dict(answer_selection) + return binding + if not selection['model_deployment'] or selection['model_provider'].lower() not in ('', 'aoai'): + raise OrchestrationModelError() + return _resolve_legacy_binding( + settings, source='planner_override', answer_selection=answer_selection, + ) + + +def resolve_orchestration_model(settings, *, user_id, seeds=None, planner=False, identity_context=None): + """Resolve a request selection, then the admin default, without retargeting failures.""" + settings = settings or {} + seeds = seeds or {} + supplied = seeds.get('model') or {} + if not isinstance(supplied, dict): + raise OrchestrationModelError() + selection = {key: _text(supplied.get(key)) for key in MODEL_IDENTITY_FIELDS} + reasoning_effort = _text(seeds.get('reasoning_effort')) + override = planner and has_planner_model_override(settings) + + if selection['model_id'] and not selection['model_endpoint_id']: + raise OrchestrationModelError() + if selection['model_endpoint_id'] and not (selection['model_id'] or selection['model_deployment']): + raise OrchestrationModelError() + if any(selection.values()) and not (selection['model_endpoint_id'] or selection['model_deployment']): + raise OrchestrationModelError() + + multi_endpoint = bool(settings.get('enable_multi_model_endpoints')) + explicit = any(selection.values()) + source = 'request' if explicit else 'legacy' + if not explicit and multi_endpoint: + default = settings.get('default_model_selection') or {} + if not isinstance(default, dict): + raise OrchestrationModelError() + if any(default.get(key) for key in ('endpoint_id', 'model_id', 'provider')): + if not _text(default.get('endpoint_id')) or not _text(default.get('model_id')): + raise OrchestrationModelError() + selection.update({ + 'model_endpoint_id': _text(default['endpoint_id']), + 'model_id': _text(default['model_id']), + 'model_provider': _text(default.get('provider')), + }) + source = 'default' + + if selection['model_endpoint_id']: + if not multi_endpoint or not user_id: + raise OrchestrationModelError() + # Endpoint and credential dependencies are only loaded when that runtime is needed. + from functions_model_endpoint_runtime import ( + MODEL_ENDPOINT_PROVIDER_ALLOWLIST, + build_model_endpoint_sync_chat_client, + resolve_model_endpoint_from_context, + ) + + context = { + 'endpoint_id': selection['model_endpoint_id'], + 'model_id': selection['model_id'], + 'model_deployment': selection['model_deployment'], + 'provider': selection['model_provider'], + 'user_id': user_id, + 'active_group_ids': seeds.get('active_group_ids') or [], + } + endpoint = resolve_model_endpoint_from_context(settings, context, authorize=True) + if not endpoint or not endpoint.get('enabled', True): + raise OrchestrationModelError() + models = endpoint.get('models') or [] + model = next(( + item for item in models if isinstance(item, dict) and ( + _text(item.get('id')) == selection['model_id'] if selection['model_id'] + else _text(item.get('deploymentName') or item.get('deployment')) == selection['model_deployment'] + ) + ), None) + if not model or not model.get('enabled', True): + raise OrchestrationModelError() + deployment = _text(model.get('deploymentName') or model.get('deployment')) + provider = _text(endpoint.get('provider')).lower() + if ( + not deployment or provider not in MODEL_ENDPOINT_PROVIDER_ALLOWLIST + or _text(endpoint.get('id')) != selection['model_endpoint_id'] + or (selection['model_provider'] and selection['model_provider'].lower() != provider) + or (selection['model_deployment'] and selection['model_deployment'] != deployment) + ): + raise OrchestrationModelError() + connection = endpoint.get('connection') or {} + address = _text(connection.get('endpoint')) + api_version = _text(connection.get('openai_api_version') or connection.get('api_version')) + protocol = infer_model_endpoint_protocol(provider, address, deployment) + if not address or (protocol == MODEL_ENDPOINT_PROTOCOL_AZURE_OPENAI and not api_version): + raise OrchestrationModelError() + answer_selection = { + 'model_deployment': deployment, 'model_provider': provider, + 'model_endpoint_id': selection['model_endpoint_id'], 'model_id': _text(model.get('id')), + } + if override: + return _resolve_planner_binding( + settings, user_id=user_id, seeds=seeds, identity_context=identity_context, + answer_selection=answer_selection, + ) + client, _ = build_model_endpoint_sync_chat_client( + endpoint.get('auth') or {}, provider, address, api_version, deployment, + settings=settings, endpoint_config=endpoint, + identity_context={**(identity_context or {}), 'user_id': user_id}, + ) + response_length = model.get('responseLength') + if not isinstance(response_length, int) or isinstance(response_length, bool) or response_length <= 0: + response_length = None + return OrchestrationModel( + client, deployment, provider=provider, endpoint_id=selection['model_endpoint_id'], + model_id=_text(model.get('id')), behavior_name=_text(model.get('modelName')) or deployment, + response_length=response_length, reasoning_effort=reasoning_effort, source=source, + ) + + if selection['model_provider'] and selection['model_provider'].lower() != 'aoai': + raise OrchestrationModelError() + if settings.get('enable_gpt_apim'): + deployments = [ + value.strip() for value in (settings.get('azure_apim_gpt_deployment') or '').split(',') + if value.strip() + ] + else: + deployments = [ + _text(model.get('deploymentName')) + for model in (settings.get('gpt_model') or {}).get('selected') or [] + if isinstance(model, dict) and _text(model.get('deploymentName')) + ] + deployment = selection['model_deployment'] or next(iter(deployments), '') + if not deployment or deployment not in deployments: + raise OrchestrationModelError() + if override: + return _resolve_planner_binding( + settings, user_id=user_id, seeds=seeds, identity_context=identity_context, + answer_selection={'model_deployment': deployment, 'model_provider': 'aoai'}, + ) + # Legacy models share one connection; the answer must not inherit a planner override. + return _resolve_legacy_binding( + {**settings, 'chat_orchestration_planner_deployment': ''}, deployment=deployment, + reasoning_effort=reasoning_effort, source=source, + ) diff --git a/application/single_app/functions_orchestration_planner.py b/application/single_app/functions_orchestration_planner.py index 2e8dfd907..3408ea714 100644 --- a/application/single_app/functions_orchestration_planner.py +++ b/application/single_app/functions_orchestration_planner.py @@ -27,14 +27,14 @@ answering step rather than to an error -- a user who asked a question should get an answer even when the planning layer had a bad day. -Version: 0.261.099 +Version: 0.261.101 """ import json import logging import re -from openai import APIError, AzureOpenAI +from openai import APIError, AzureOpenAI, BadRequestError from azure.identity import DefaultAzureCredential, get_bearer_token_provider from config import cognitive_services_scope @@ -57,6 +57,7 @@ PLANNER_MAX_TOKENS = 2000 PLANNER_TEMPERATURE = 0.1 RESOLUTION_MAX_TOKENS = 1200 +RESOLUTION_MAX_ATTEMPTS = 2 RESOLVED_REQUEST_MAX_LENGTH = 6000 ACKNOWLEDGMENT_PATTERN = re.compile( r'(?:hi|hello|hey|thanks|thank you|ok|okay|got it|understood|great|sounds good)[.! ]*', @@ -90,12 +91,32 @@ class PlannerError(RuntimeError): """Raised when the planner could not be reached or configured.""" +class PlannerResponseError(PlannerError): + """A completion was refused, absent, or incomplete rather than malformed JSON.""" + + def __init__(self, reason): + super().__init__('The model did not return a complete response.') + self.reason = reason + + class ConversationResolutionError(PlannerError): """A follow-up could not be interpreted safely.""" + def __init__( + self, message='The conversation could not be interpreted. Please retry your request.', + *, reason='invalid_resolution', attempts=0, + ): + super().__init__(message) + self.reason = reason + self.attempts = attempts + def resolve_planner_client(settings): - """Create the chat client that writes plans, and return it with its deployment name. + """Create a legacy chat client and return it with its deployment name. + + Orchestration HTTP requests supply an authorized model binding instead when using + manual selections or configured model endpoints. This helper retains the classic + single-endpoint/APIM contract for those bindings and standalone planner callers. Falls back to the deployment's ordinary chat configuration when no planner deployment is configured, so orchestration works the moment it is switched on rather than @@ -421,10 +442,26 @@ def extract_planner_json(reply): return None +def _unsupported_json_format(error): + body = error.body if isinstance(error.body, dict) else {} + body = body.get('error') if isinstance(body.get('error'), dict) else body + parameter = str(body.get('param') or '') + message = str(body.get('message') or '').lower() + return ( + (parameter.startswith('response_format') or 'response_format' in message) + and ( + body.get('code') in ('unsupported_parameter', 'unsupported_value') + or 'not supported' in message + or 'unsupported' in message + ) + ) + + def _call_planner( - client, deployment, messages, *, max_tokens=PLANNER_MAX_TOKENS, temperature=PLANNER_TEMPERATURE + client, deployment, messages, *, max_tokens=PLANNER_MAX_TOKENS, + temperature=PLANNER_TEMPERATURE, require_complete_response=False, ): - """One planner completion, asking for JSON where the deployment supports it.""" + """Ask for JSON, with strict completion and retry handling for the resolver.""" try: response = client.chat.completions.create( model=deployment, @@ -434,12 +471,18 @@ def _call_planner( response_format={'type': 'json_object'}, ) except Exception as exc: + # Preserve the planner's fallback, but never repair a resolver's provider failure. + if require_complete_response and not ( + isinstance(exc, BadRequestError) and _unsupported_json_format(exc) + ): + raise # Not every deployment or API version accepts response_format, and a refusal here # is a configuration difference rather than a failure. The prompt already asks for # one JSON object, and the extractor copes with a reply that merely contains one. log_event( - f"[ORCHESTRATION_PLANNER] Retrying without a JSON response format: {exc}", + '[ORCHESTRATION_PLANNER] Retrying without a JSON response format.', level=logging.INFO, + extra={'reason': 'json_format_retry', 'error_type': type(exc).__name__}, ) response = client.chat.completions.create( model=deployment, @@ -449,10 +492,22 @@ def _call_planner( ) if not response or not response.choices: + if require_complete_response: + raise PlannerResponseError('empty_completion') return '', None + choice = response.choices[0] + if require_complete_response: + finish_reason = getattr(choice, 'finish_reason', None) + if finish_reason == 'content_filter' or getattr(choice.message, 'refusal', None): + raise PlannerResponseError('model_refusal') + if finish_reason not in (None, 'stop'): + raise PlannerResponseError('incomplete_completion') + if not choice.message.content: + raise PlannerResponseError('empty_completion') + usage = getattr(response, 'usage', None) - return (response.choices[0].message.content or ''), usage + return (choice.message.content or ''), usage RESOLUTION_SYSTEM_PROMPT = """Interpret the user's latest request within this conversation. @@ -465,6 +520,12 @@ def _call_planner( "clarification": "" } +Include all five fields. Use an empty string "" for clarification when no question is +needed; a nonempty clarification is required only for relationship "clarification". +requires_retrieval must be a JSON boolean, and message_ids must be an array of exact +supplied IDs. A follow_up needs at least one historical ID unless clarification answers +supply the missing context. + Resolve pronouns, "which", "those", omitted subjects, and follow-ups against both the user's earlier requests and the assistant's actual answers. Preserve relevant constraints such as place, travel route, date, opening day, and time. A new explicit constraint overrides an old @@ -489,8 +550,51 @@ def _call_planner( If history is truncated, do not pretend to know what was omitted.""" -def resolve_conversation_request(user_message, snapshot, settings=None, answered_questions=None): - """Resolve context before candidate retrieval, using the existing planner deployment.""" +def _normalize_request_resolution(parsed, valid_ids, *, has_answers=False): + """Validate model-owned fields before any can influence retrieval or execution.""" + if not isinstance(parsed, dict): + raise ConversationResolutionError(reason='invalid_json') + relationship = parsed.get('relationship') + if relationship not in ('follow_up', 'new_topic', 'clarification'): + raise ConversationResolutionError(reason='invalid_relationship') + resolved = parsed.get('resolved_message') + if not isinstance(resolved, str) or not resolved.strip() or len(resolved) > RESOLVED_REQUEST_MAX_LENGTH: + raise ConversationResolutionError(reason='invalid_resolved_message') + message_ids = parsed.get('message_ids') + if not isinstance(message_ids, list) or any(not isinstance(value, str) for value in message_ids): + raise ConversationResolutionError(reason='invalid_message_ids') + if any(value not in valid_ids for value in message_ids): + raise ConversationResolutionError(reason='unknown_message_ids') + if len(message_ids) != len(set(message_ids)): + raise ConversationResolutionError(reason='duplicate_message_ids') + if not isinstance(parsed.get('requires_retrieval'), bool): + raise ConversationResolutionError(reason='invalid_retrieval_flag') + + clarification = parsed.get('clarification', '') + # JSON null also means "no question", but cannot satisfy a requested clarification. + if clarification is None and relationship != 'clarification': + clarification = '' + if not isinstance(clarification, str) or len(clarification) > 1000: + raise ConversationResolutionError(reason='invalid_clarification') + if relationship == 'follow_up' and not message_ids and not has_answers: + raise ConversationResolutionError(reason='missing_follow_up_context') + if relationship == 'clarification' and not clarification.strip(): + raise ConversationResolutionError(reason='missing_clarification') + if relationship == 'new_topic' and message_ids: + raise ConversationResolutionError(reason='unexpected_new_topic_context') + return { + 'relationship': relationship, + 'resolved_message': resolved.strip(), + 'message_ids': message_ids, + 'requires_retrieval': parsed['requires_retrieval'], + 'clarification': clarification.strip(), + } + + +def resolve_conversation_request( + user_message, snapshot, settings=None, answered_questions=None, planner_model=None, +): + """Resolve context before retrieval, using the captured model binding when supplied.""" message = str(user_message or '').strip() history = conversation_reference_messages(snapshot) default = { @@ -513,77 +617,93 @@ def resolve_conversation_request(user_message, snapshot, settings=None, answered 'answered_questions': answered_questions or [], } try: - client, deployment = resolve_planner_client(settings) - reply, usage = _call_planner( - client, deployment, - [ - {'role': 'system', 'content': RESOLUTION_SYSTEM_PROMPT}, - {'role': 'user', 'content': json.dumps( - payload, ensure_ascii=False, separators=(',', ':') - )}, - ], - max_tokens=RESOLUTION_MAX_TOKENS, - temperature=0, - ) + if planner_model is not None: + client, deployment = planner_model.as_planner_client(), planner_model.deployment + else: + client, deployment = resolve_planner_client(settings) except (APIError, PlannerError) as exc: log_event( - '[ORCHESTRATION_PLANNER] Conversation request resolution failed.', + '[ORCHESTRATION_PLANNER] Conversation resolver could not be configured.', level=logging.ERROR, extra={'stage': 'request_resolution', 'error_type': type(exc).__name__}, ) - raise ConversationResolutionError( - 'The conversation could not be interpreted. Please retry your request.' - ) from exc + raise ConversationResolutionError(reason='planner_unavailable') from exc - parsed = extract_planner_json(reply) + messages = [ + {'role': 'system', 'content': RESOLUTION_SYSTEM_PROMPT}, + {'role': 'user', 'content': json.dumps( + payload, ensure_ascii=False, separators=(',', ':') + )}, + ] valid_ids = {entry['id'] for entry in history} - if not isinstance(parsed, dict): - raise ConversationResolutionError('The conversation could not be interpreted. Please retry.') - relationship = parsed.get('relationship') - resolved = parsed.get('resolved_message') - message_ids = parsed.get('message_ids') - clarification = parsed.get('clarification', '') - if ( - relationship not in ('follow_up', 'new_topic', 'clarification') - or not isinstance(resolved, str) - or not resolved.strip() - or len(resolved) > RESOLVED_REQUEST_MAX_LENGTH - or not isinstance(message_ids, list) - or any(not isinstance(value, str) or value not in valid_ids for value in message_ids) - or len(message_ids) != len(set(message_ids)) - or not isinstance(parsed.get('requires_retrieval'), bool) - or not isinstance(clarification, str) - or len(clarification) > 1000 - or (relationship == 'follow_up' and not message_ids and not answered_questions) - or (relationship == 'clarification' and not clarification.strip()) - or (relationship == 'new_topic' and message_ids) - ): - raise ConversationResolutionError('The conversation could not be interpreted. Please retry.') - token_usage = { - field: getattr(usage, field) - for field in ('prompt_tokens', 'completion_tokens', 'total_tokens') - if isinstance(getattr(usage, field, None), int) - } + token_usage = {} + for attempt in range(1, RESOLUTION_MAX_ATTEMPTS + 1): + try: + reply, usage = _call_planner( + client, deployment, messages, + max_tokens=RESOLUTION_MAX_TOKENS, temperature=0, + require_complete_response=True, + ) + except (APIError, PlannerError) as exc: + reason = exc.reason if isinstance(exc, PlannerResponseError) else 'model_request_failed' + log_event( + '[ORCHESTRATION_PLANNER] Conversation request resolution failed.', + level=logging.ERROR, + extra={ + 'stage': 'request_resolution', 'reason': reason, + 'attempt': attempt, 'error_type': type(exc).__name__, + }, + ) + raise ConversationResolutionError(reason=reason, attempts=attempt) from exc + + for field in ('prompt_tokens', 'completion_tokens', 'total_tokens'): + value = getattr(usage, field, None) + if isinstance(value, int): + token_usage[field] = token_usage.get(field, 0) + value + try: + resolution = _normalize_request_resolution( + extract_planner_json(reply), valid_ids, has_answers=bool(answered_questions), + ) + break + except ConversationResolutionError as exc: + log_event( + '[ORCHESTRATION_PLANNER] Rejected a conversation resolution response.', + level=logging.WARNING, + extra={ + 'stage': 'request_resolution', 'reason': exc.reason, 'attempt': attempt, + 'history_message_count': len(history), 'response_length': len(reply), + }, + ) + if attempt == RESOLUTION_MAX_ATTEMPTS: + raise ConversationResolutionError(reason=exc.reason, attempts=attempt) from exc + messages = [ + *messages, + { + 'role': 'user', + 'content': ( + 'The previous response did not satisfy the JSON contract. ' + f'Validation reason: {exc.reason}. Return a corrected JSON object ' + 'for the unchanged request and conversation above. Use only supplied ' + 'historical IDs and do not invent or discard context to satisfy the schema.' + ), + }, + ] + + if resolution['relationship'] == 'new_topic' and not answered_questions: + resolution['resolved_message'] = message + resolution['token_usage'] = token_usage log_event( '[ORCHESTRATION_PLANNER] Resolved the conversational request.', debug_only=True, extra={ 'stage': 'request_resolution', - 'relationship': relationship, + 'relationship': resolution['relationship'], + 'attempt': attempt, 'history_message_count': len(history), - 'selected_message_count': len(message_ids), + 'selected_message_count': len(resolution['message_ids']), }, ) - return { - 'relationship': relationship, - 'resolved_message': ( - message if relationship == 'new_topic' and not answered_questions else resolved.strip() - ), - 'message_ids': message_ids, - 'requires_retrieval': parsed['requires_retrieval'], - 'clarification': clarification.strip(), - 'token_usage': token_usage, - } + return resolution # -------------------------------------------------------------------------------------- @@ -605,6 +725,7 @@ def plan_request( seeds=None, document_labels=None, request_context=None, + planner_model=None, ): """Produce a validated plan, or a question set, for one request. @@ -663,7 +784,10 @@ def _fallback(reason): return 'plan', plan try: - client, deployment = resolve_planner_client(settings) + if planner_model is not None: + client, deployment = planner_model.as_planner_client(), planner_model.deployment + else: + client, deployment = resolve_planner_client(settings) except PlannerError as exc: return _fallback(str(exc)) @@ -713,6 +837,7 @@ def _fallback(reason): seeds=seeds, document_labels=document_labels, request_context=request_context, + planner_model=planner_model, ) if kind == 'elicitation': diff --git a/application/single_app/model_endpoint_clients.py b/application/single_app/model_endpoint_clients.py index c40f8b9ee..41b7ec4ce 100644 --- a/application/single_app/model_endpoint_clients.py +++ b/application/single_app/model_endpoint_clients.py @@ -36,6 +36,19 @@ MODEL_CONTEXT_MODE_FOLD_LATEST_USER = "fold_latest_user" +def normalize_anthropic_finish_reason(finish_reason: Any) -> str | None: + """Preserve Anthropic completion semantics in the chat-completions interface.""" + normalized_reason = str(finish_reason or "").strip().lower() + return { + "end_turn": "stop", + "stop_sequence": "stop", + "max_tokens": "length", + "model_context_window_exceeded": "length", + "tool_use": "tool_calls", + "refusal": "content_filter", + }.get(normalized_reason, normalized_reason or None) + + class ModelEndpointBehavior: """Provider/model behavior policy shared by Simple Chat model endpoint callers.""" @@ -485,7 +498,7 @@ def _build_completion_response(self, response_payload: Dict[str, Any]): return SimpleNamespace( choices=[SimpleNamespace( message=SimpleNamespace(content=text, tool_calls=tool_calls), - finish_reason=response_payload.get("stop_reason"), + finish_reason=normalize_anthropic_finish_reason(response_payload.get("stop_reason")), )], usage=SimpleNamespace( prompt_tokens=prompt_tokens, @@ -697,11 +710,9 @@ def _create_chat_message_contents_from_response(self, response) -> List[ChatMess ] def _normalize_finish_reason(self, finish_reason: Any) -> FinishReason | None: - normalized_reason = str(finish_reason or "").strip().lower() + normalized_reason = normalize_anthropic_finish_reason(finish_reason) if not normalized_reason: return None - if normalized_reason == "tool_use": - normalized_reason = "tool_calls" try: return FinishReason(normalized_reason) except ValueError: diff --git a/application/single_app/route_backend_orchestration.py b/application/single_app/route_backend_orchestration.py index 81287f103..dcc1fb076 100644 --- a/application/single_app/route_backend_orchestration.py +++ b/application/single_app/route_backend_orchestration.py @@ -18,9 +18,10 @@ raises rather than returning the value it would have had -- a failure that only appears once streaming is actually exercised. -Version: 0.261.099 +Version: 0.261.101 """ +import hashlib import logging import queue import threading @@ -31,6 +32,7 @@ from azure.core.exceptions import AzureError from azure.cosmos.exceptions import CosmosResourceNotFoundError from flask import Response, jsonify, request, session +from openai import OpenAIError from config import cosmos_conversations_container, cosmos_messages_container from functions_appinsights import log_event @@ -78,12 +80,20 @@ from functions_orchestration_executor import RunContext, execute_plan from functions_orchestration_planner import ( ConversationResolutionError, + PlannerError, + PlannerResponseError, build_trivial_plan, plan_request, resolve_conversation_request, resolve_planner_client, triage_request, ) +from functions_orchestration_models import ( + OrchestrationModel, + OrchestrationModelError, + has_planner_model_override, + resolve_orchestration_model, +) from functions_orchestration_runs import ( clear_pending_turn_context, create_orchestration_run, @@ -113,6 +123,7 @@ ) from functions_settings import get_settings, get_user_settings from functions_prompt_metadata import build_prompt_selection_metadata +from model_endpoint_clients import extract_chat_completion_response_text from swagger_wrapper import get_auth_security, swagger_route # SSE responses must not be buffered by an intermediary, or progress arrives all at once at @@ -167,7 +178,7 @@ def _orchestration_enabled(settings): return bool((settings or {}).get('enable_chat_orchestration')) -def _build_invoke_prompt(settings, token_usage=None): +def _build_invoke_prompt(settings, token_usage=None, model=None): """A closure the adapters call to ask the model something. The signature is not ours to choose. ``run_document_analysis``, @@ -183,18 +194,17 @@ def _build_invoke_prompt(settings, token_usage=None): model call either way. They are named rather than swallowed by ``**kwargs`` so this file states the contract it is honouring. - Shares the planner's client resolution, which already handles APIM, managed identity - and key auth, but deliberately not its deployment: planning may be pointed at a small - model, while the answer should come from the deployment the administrator chose for - chat. + The run supplies an authorized model binding resolved from its saved selection or + the administrator's default. The legacy branch remains for callers without a binding. """ - client, planner_deployment = resolve_planner_client(settings) - - deployment = None - gpt_model = (settings or {}).get('gpt_model') or {} - if gpt_model.get('selected'): - deployment = (gpt_model['selected'][0] or {}).get('deploymentName') - deployment = deployment or planner_deployment + if model is None: + client, planner_deployment = resolve_planner_client(settings) + gpt_model = (settings or {}).get('gpt_model') or {} + deployment = ( + (gpt_model['selected'][0] or {}).get('deploymentName') + if gpt_model.get('selected') else None + ) or planner_deployment + model = OrchestrationModel(client, deployment) def invoke_prompt(prompt_text, stage='window_analysis', metadata=None): messages = ( @@ -202,12 +212,19 @@ def invoke_prompt(prompt_text, stage='window_analysis', metadata=None): if isinstance(prompt_text, list) else [{'role': 'user', 'content': str(prompt_text or '')}] ) - response = client.chat.completions.create( - model=deployment, - messages=messages, - temperature=ANSWER_TEMPERATURE, - max_tokens=ANSWER_MAX_TOKENS, - ) + try: + response = model.create_completion( + messages=messages, + temperature=ANSWER_TEMPERATURE, + max_tokens=ANSWER_MAX_TOKENS, + use_model_response_length=True, + ) + except (OpenAIError, AzureError) as exc: + log_event( + '[ORCHESTRATION] The answer model request failed.', + level=logging.WARNING, extra={'stage': stage, 'error_type': type(exc).__name__}, + ) + raise PlannerResponseError('model_request_failed') from exc # Accumulated here because this is the only place every model call an orchestration # run makes passes through. A run's cost was previously reported as zero for that @@ -224,8 +241,14 @@ def invoke_prompt(prompt_text, stage='window_analysis', metadata=None): f"[ORCHESTRATION] The model returned no choices at stage '{stage}'.", level=logging.WARNING, ) - return '' - return response.choices[0].message.content or '' + raise PlannerResponseError('empty_completion') + choice = response.choices[0] + if getattr(choice, 'finish_reason', None) == 'content_filter' or getattr(choice.message, 'refusal', None): + raise PlannerResponseError('model_refusal') + text = extract_chat_completion_response_text(response) + if not text.strip(): + raise PlannerResponseError('empty_completion') + return text return invoke_prompt @@ -929,6 +952,20 @@ def orchestration_plan(): user_id, seeds=seeds, settings=settings, user_groups=seeds.get('active_group_ids') or None, ) + try: + planner_model = resolve_orchestration_model( + settings, user_id=user_id, seeds=seeds, planner=True, identity_context=identity, + ) + seeds['model'] = planner_model.answer_model_selection() + except (ValueError, PermissionError, PlannerError, AzureError) as exc: + log_event( + '[ORCHESTRATION] The planner model could not be selected.', + level=logging.WARNING, extra={'error_type': type(exc).__name__}, + ) + return _sse(iter([build_error_event( + 'The selected model is unavailable. Choose an enabled model you can access.', + conversation_id, + )])) def generate(): try: @@ -987,6 +1024,7 @@ def generate(): validate_clarification_answers(answered_record) resolution = resolve_conversation_request( message, snapshot, settings=settings, answered_questions=answered_record, + planner_model=planner_model, ) planning_usage = _sum_token_usage( (pending or {}).get('planning_token_usage'), resolution.get('token_usage') @@ -1101,6 +1139,7 @@ def generate(): action_catalog, allowed_user_urls=allowed_user_urls, ), + planner_model=planner_model, ) planning_usage = _sum_token_usage(planning_usage, plan.get('token_usage')) @@ -1161,7 +1200,21 @@ def generate(): yield build_planning_thought('Plan ready.', status='completed') yield build_plan_event(plan) - except (ConversationContextError, ConversationResolutionError) as exc: + except ConversationResolutionError as exc: + log_event( + '[ORCHESTRATION] The conversational request could not be interpreted.', + level=logging.WARNING, + extra={ + 'stage': 'request_resolution', 'reason': exc.reason, + 'attempt': exc.attempts, 'error_type': type(exc).__name__, + 'resource': f"conversation:{hashlib.sha256(resolved_conversation_id.encode('utf-8')).hexdigest()}", + }, + ) + yield build_error_event( + 'The conversation could not be interpreted. Please retry your request.', + resolved_conversation_id, + ) + except ConversationContextError as exc: log_event( '[ORCHESTRATION] Conversation context could not be used for planning.', level=logging.WARNING, extra={'error_type': type(exc).__name__}, @@ -1176,8 +1229,12 @@ def generate(): level=logging.ERROR, exceptionTraceback=True, ) yield build_error_event('The request could not be planned.', conversation_id) + finally: + planner_model.close() - return _sse(generate()) + response = _sse(generate()) + response.call_on_close(planner_model.close) + return response @bp.route("/api/v2/orchestration/run", methods=["POST"]) @swagger_route(security=get_auth_security()) @@ -1234,12 +1291,6 @@ def orchestration_run(): # One accumulator for the whole run, filled by every model call the closure makes. run_token_usage = _sum_token_usage(record.get('planning_token_usage')) - try: - invoke_prompt = _build_invoke_prompt(settings, token_usage=run_token_usage) - except Exception as exc: - log_event(f"[ORCHESTRATION] No usable chat model: {exc}", level=logging.ERROR) - return jsonify({'error': 'No chat model is configured.'}), 503 - seeds = record.get('seeds') if isinstance(record.get('seeds'), dict) else {} user_message = _text(record.get('user_message')) or _text( (plan.get('intent') or {}).get('summary') @@ -1268,17 +1319,51 @@ def orchestration_run(): user_id, seeds=seeds, settings=settings, user_groups=seeds.get('active_group_ids') or None, ) - selected_model = seeds.get('model') or {} + answer_model = None + research_model = None + + def close_models(): + try: + if research_model is not None: + research_model.close() + finally: + if answer_model is not None: + answer_model.close() + + try: + answer_model = resolve_orchestration_model( + settings, user_id=user_id, seeds=seeds, identity_context=identity, + ) + research_model = ( + resolve_orchestration_model( + settings, user_id=user_id, seeds=seeds, planner=True, identity_context=identity, + ) if has_planner_model_override(settings) else answer_model + ) + invoke_prompt = _build_invoke_prompt( + settings, token_usage=run_token_usage, model=answer_model, + ) + except (ValueError, PermissionError, PlannerError, AzureError) as exc: + close_models() + log_event( + '[ORCHESTRATION] The execution model could not be selected.', + level=logging.WARNING, extra={'error_type': type(exc).__name__}, + ) + return jsonify({ + 'error': 'The selected model is unavailable. Choose an enabled model you can access.', + }), 403 if isinstance(exc, (PermissionError, OrchestrationModelError)) else 503 + action_model_context = { - 'model_id': selected_model.get('model_id'), - 'endpoint_id': selected_model.get('model_endpoint_id'), - 'provider': selected_model.get('model_provider'), - 'model_deployment': selected_model.get('model_deployment'), + 'model_id': answer_model.model_id, + 'endpoint_id': answer_model.endpoint_id, + 'provider': answer_model.provider, + 'model_deployment': answer_model.deployment, 'user_id': user_id, 'active_group_ids': seeds.get('active_group_ids') or [], } + worker_started = False def generate(): + nonlocal worker_started approved_at = _now_iso() try: update_orchestration_run(run_id, user_id, { @@ -1353,6 +1438,8 @@ def persist(record_type, payload): user_id=user_id, turn_index=record.get('turn_index') or 0, invoke_prompt=invoke_prompt, + planner_client=research_model.as_planner_client(), + planner_deployment=research_model.deployment, user_message=user_message, user_message_id=record.get('user_message_id'), resolved_message=_text(record.get('resolved_message')) or user_message, @@ -1377,7 +1464,7 @@ def persist(record_type, payload): active_group_id=(seeds.get('active_group_ids') or [None])[0], agent_catalog=agent_catalog, action_catalog=action_catalog, - gpt_model=selected_model.get('model_deployment'), + gpt_model=answer_model.deployment, model_context=action_model_context, agent_execution_identity=agent_execution_identity, ) @@ -1405,12 +1492,16 @@ def worker(): # The sentinel is what ends the drain loop. Sent from `finally` so a # thrown worker cannot leave the response waiting on a queue nothing # will ever write to again. - frames.put(None) + try: + close_models() + finally: + frames.put(None) thread = threading.Thread( target=worker, name=f'orchestration-run-{run_id}', daemon=True ) thread.start() + worker_started = True while True: try: @@ -1427,7 +1518,11 @@ def worker(): thread.join(timeout=RUN_JOIN_TIMEOUT_SECONDS) - if 'error' in outcome or 'result' not in outcome: + failed_result = outcome.get('result') or {} + if ( + 'error' in outcome or 'result' not in outcome + or failed_result.get('status') == PLAN_STATUS_FAILED + ): error_message = ( 'Conversation context changed. Create a new plan.' if isinstance(outcome.get('error'), ConversationContextError) @@ -1438,7 +1533,9 @@ def worker(): 'status': PLAN_STATUS_FAILED, 'error': error_message, 'completed_at': _now_iso(), - 'token_usage': run_token_usage, + 'token_usage': _combined_token_usage( + run_token_usage, failed_result.get('token_usage'), + ), }, conversation_id=conversation_id) except AzureError as exc: log_event( @@ -1475,6 +1572,7 @@ def worker(): 'token_usage': combined_usage, }, extra={ + **answer_model.metadata(), 'hybrid_citations': document_citations, 'web_search_citations': web_citations, 'agent_citations': tool_citations, @@ -1512,9 +1610,12 @@ def worker(): artifacts=result.get('artifacts'), plan_summary=summary, status=result.get('status') or PLAN_STATUS_COMPLETED, + **answer_model.metadata(), ) - return _sse(generate()) + response = _sse(generate()) + response.call_on_close(lambda: close_models() if not worker_started else None) + return response @bp.route("/api/v2/orchestration/cancel/", methods=["POST"]) @swagger_route(security=get_auth_security()) diff --git a/docs/admin/orchestration.md b/docs/admin/orchestration.md index d71c6bd3f..2b1b6e9a7 100644 --- a/docs/admin/orchestration.md +++ b/docs/admin/orchestration.md @@ -246,8 +246,20 @@ Selects the model that writes plans. Planning is a short, structured task rather than a conversational one, so a smaller and faster deployment usually does it well and costs less per message than the model that -writes the answer. Leaving the deployment blank plans with the deployment's default chat -model, which means orchestration works as soon as it is switched on. +writes the answer. Since **0.261.101**, leaving all planner fields blank uses the model +chosen in **Manual controls** first, then the administrator's default model connection, +rather than an unrelated legacy GPT deployment. Classic chat/APIM settings remain the +fallback when no model-connection selection or default applies. + +A dedicated planner remains independent of the model that writes the answer. For a +configured model connection, supply its endpoint and model IDs; the deployment and +provider, when supplied, must agree with that selection. Model access is checked for +the requesting user. A deployment-only override uses the classic chat/APIM connection. + +The answer choice is saved with the plan and checked again when it runs. Changing the +admin default during approval does not switch that answer to a different model. +If the saved model is no longer available to the user, execution stops with a model +availability error rather than silently falling back to GPT-4o. The same deployment resolves substantive follow-ups before retrieval. This adds a small completion when usable conversation history or clarification answers are present. @@ -257,7 +269,7 @@ First turns without history and simple acknowledgments skip that call. | Setting | What it does | Default | Notes | | --- | --- | --- | --- | -| Planner deployment name | Names the deployment used for planning. Blank uses the default chat model. | Empty | `chat_orchestration_planner_deployment` | +| Planner deployment name | Names a separate planning deployment without changing the answer model. When all planner fields are blank, planning uses the manual selection or admin default. | Empty | `chat_orchestration_planner_deployment` | | Planner model id | Identifies the model when planning through a configured model endpoint. | Empty | `chat_orchestration_planner_model_id` | | Planner model endpoint id | Identifies the endpoint when planning through a configured model endpoint rather than the default deployment. | Empty | `chat_orchestration_planner_model_endpoint_id` | | Planner model provider | Identifies the provider when planning through a configured model endpoint. | Empty | `chat_orchestration_planner_model_provider` | diff --git a/docs/explanation/features/CHAT_ORCHESTRATION.md b/docs/explanation/features/CHAT_ORCHESTRATION.md index 97e4688c8..3aef6ca71 100644 --- a/docs/explanation/features/CHAT_ORCHESTRATION.md +++ b/docs/explanation/features/CHAT_ORCHESTRATION.md @@ -1,12 +1,14 @@ # Chat Orchestration -**Version: 0.261.099** (tracked in `application/single_app/config.py`) +**Version: 0.261.101** (tracked in `application/single_app/config.py`) **Implemented in version: 0.261.086** **Knowledge phase added in version: 0.261.089** **Research selection and multi-query execution updated in version: 0.261.099** **Direct action access implemented in version: 0.261.098** **Conversation continuity implemented in version: 0.261.096** +**Follow-up resolver compatibility fixed in version: 0.261.100** +**Selected/default model routing fixed in version: 0.261.101** ## Overview @@ -103,6 +105,58 @@ These rules apply to Auto, countdown, and manual approval. They do not introduce summaries or cross-conversation memory. First turns without history and simple acknowledgments do not require a resolution completion. +Since **0.261.100**, an unused `clarification: null` in the resolver's JSON is +accepted as "no clarification needed", just like an empty string. It does not +discard the rest of a valid follow-up or require another model call. A request +whose relationship is `clarification` still needs a nonempty question. + +Other malformed resolver output gets at most one corrective completion using +the same bounded history, original request, and clarification answers. Both +completions count toward the successful turn's token usage. Unknown message IDs, +invalid required field types, and inconsistent relationships remain invalid: +the application never drops history or starts a new topic merely to make a +response pass validation. Persistent failure produces an interpretation error, +distinct from an inaccessible or changed conversation. + +Refused, filtered, absent, or incomplete completions are not retried as malformed +JSON. The resolver does not mistake provider failures for unsupported JSON formatting; only +an explicit unsupported-response-format error uses the existing no-format +compatibility fallback. The separate plan generator retains its existing retry +behavior. No new model setting or API version is required. + +#### Model selection + +The answer model comes from **Manual controls** when one is selected, otherwise from +the administrator's default model connection. Classic single-endpoint or APIM settings +remain the fallback only when no connection-based selection or default applies. +The selected deployment, provider, endpoint ID and model ID are resolved together: +changing only the deployment on a legacy client could send it to the wrong endpoint. + +The authorized answer choice is saved with the plan, including a resolved admin default. +Changing the default while approval is pending does not retarget that run. The model is +authorized again at execution, so a disabled, removed or inaccessible selection produces +an error instead of silently switching to another model. + +Without a dedicated planner override, the same selection handles conversational +resolution, plan generation and research review as well as the answer. A configured +planner deployment or endpoint remains separate and does not override the answer model. +Planner endpoint selections use the existing planner model/endpoint/provider settings and +the caller's normal model access checks. A deployment-only planner override retains the +classic single-endpoint/APIM connection. + +Clients are prepared on the request thread with captured caller identity. The worker +uses those bindings, and direct actions receive the actual answer model identity. +GPT-5-family completions use `max_completion_tokens`, omit unsupported temperature, +and retain compatible manual reasoning effort. A positive configured model response +length governs answer calls; otherwise reasoning completions have an 8192-token budget +floor so reasoning does not consume the entire smaller visible-output allowance. +Anthropic completion flags are normalized at the protocol boundary, so successful +Claude follow-ups pass the same strict checks while truncation and refusal remain failures. + +The saved assistant message and terminal stream identify the model that actually answered. +The existing V2 renderer displays that name. Empty, refused, filtered or failed answer +completions produce an error rather than a success-shaped empty turn. + #### What the context picker contributes The composer's context picker lets a user name documents, tags and whole workspaces before @@ -449,6 +503,7 @@ See [the Orchestration settings page](../../admin/orchestration.md) for the full | `functions_orchestration_executor.py` | Step engine, budgets, cancellation, re-authorization | | `functions_orchestration_runs.py` | Run and step persistence | | `functions_orchestration_events.py` | Stream event builders | +| `functions_orchestration_models.py` | Authorized model selection, endpoint clients, completion parameters and safe model metadata | | `route_backend_orchestration.py` | The V2 endpoints, conversation and message persistence | | `route_backend_chats.py` | Shared ordinary and multi-query web-search helpers | | `functions_source_review.py` | Shared bounded query generation, backup planning, and source review | @@ -481,6 +536,7 @@ to the front. | `functional_tests/test_orchestration_elicitation_schema.py` | The MCP flat-object restriction, paging staying outside the schema, response validation | | `functional_tests/test_orchestration_run_ledger.py` | Run and byte bounds, oldest-first compaction, honest truncation, answered questions carrying forward | | `functional_tests/test_orchestration_invoke_prompt_contract.py` | The model-call convention: the route's closure must accept what the adapters and the document functions actually pass, and must count token usage | +| `functional_tests/test_orchestration_model_selection.py` | Manual/default precedence, independent planner connections, authorization, unavailable models, legacy/APIM compatibility, protocol parameters and client ownership | | `functional_tests/test_orchestration_executor.py` | Step ordering, dependency skipping, cancellation, budget caps, re-authorization | | `functional_tests/test_orchestration_phase_ordering.py` | Knowledge sorts before reasoning, a plan gathering after answering is repaired, a backwards dependency is dropped with a note | | `functional_tests/test_orchestration_adapter_contract.py` | Every capability resolves to an adapter, every adapter matches the executor's call signature, no adapter touches Flask state, and identity is captured on the request thread | @@ -491,9 +547,9 @@ to the front. | `functional_tests/test_orchestration_action_planning.py` | Default-off action gating, short requests, validated action inputs, and retained agent selections | | `functional_tests/test_orchestration_action_runtime.py` | One-action loading, bounded function calls, model authorization, cancellation, usage and resource cleanup | | `functional_tests/test_orchestration_context_picker.py` | Picked tags reach the seeds and both search paths under the parameter `hybrid_search` really takes; a tag scopes the probe rather than replacing it; a picked document reaches the planner and the approval card by name; a browser-supplied name cannot widen access; search citations carry the workspace a document came from; a step can read what an earlier step found, an unusable reference is repaired or dropped, and a run-time document still respects the configured ceiling | -| `functional_tests/test_orchestration_conversation_context.py` | Message eligibility, bounds, snapshot validation, follow-up resolution, contextualized adapters, synthesis roles, and URL provenance | -| `functional_tests/test_orchestration_conversation_context_routes.py` | Owned server history across HTTP/SSE planning and execution, all approval modes, clarification, retries, stale sources, and legacy cutoffs | -| `ui_tests/test_v2_orchestration_conversation_context.py` | Matching clarification transport, cancellation, original-turn continuity, all approval modes, and navigation | +| `functional_tests/test_orchestration_conversation_context.py` | Message eligibility, bounds, snapshot validation, nullable unused clarifications, strict response validation, bounded repair, token accounting, provider/refusal handling, contextualized adapters, synthesis roles, and URL provenance | +| `functional_tests/test_orchestration_conversation_context_routes.py` | New and existing conversations across HTTP/SSE planning and execution, all approval modes, null clarifications, bounded recovery, model selection and attribution, revocation, completion failures, stream cleanup, stale sources and legacy cutoffs | +| `ui_tests/test_v2_orchestration_conversation_context.py` | Matching clarification/model transport, cancellation, original-turn continuity, all approval modes, visible answer model names and navigation | Research-selection evaluation distinguishes contract coverage from model behaviour. A mocked plan proves that the application preserves an allowed choice; it does not prove @@ -506,9 +562,10 @@ research-selection rate is not itself a quality improvement. - **Recent context only.** There is no orchestration rolling summary or cross-chat memory. A reference outside the retained window may need clarification. -- **Automatic per-step model routing is not implemented.** Planning uses its configured - model. Direct action execution honors an explicitly selected, available chat model or - the deployment defaults; it does not select models by task capability or cost. +- **Automatic per-step model routing is not implemented.** Planning and research use the + selected/default answer model unless a dedicated planner override is configured. + Direct action execution receives the answer selection. Models are not selected + dynamically by task capability or cost; configured agents retain their own model behavior. - **No output-phase workflow.** Existing MCP, OpenAPI and other action types can now gather knowledge directly, but the `output` phase remains empty. There are no dedicated output scheduling, workspace placement or delivery steps. Actions retain their existing @@ -530,5 +587,6 @@ research-selection rate is not itself a quality improvement. - [Orchestration settings](../../admin/orchestration.md) - [Chat Orchestration Action Access](CHAT_ORCHESTRATION_ACTIONS.md) - [Conversation context fix](../fixes/ORCHESTRATION_CONVERSATION_CONTEXT_FIX.md) +- [Model selection fix](../fixes/ORCHESTRATION_MODEL_SELECTION_FIX.md) - `docs/explanation/release_notes.md` - [Deep research selection and execution fix](../fixes/ORCHESTRATION_DEEP_RESEARCH_SELECTION_FIX.md) diff --git a/docs/explanation/fixes/ORCHESTRATION_CONVERSATION_CONTEXT_FIX.md b/docs/explanation/fixes/ORCHESTRATION_CONVERSATION_CONTEXT_FIX.md index 704052371..8bae1cf90 100644 --- a/docs/explanation/fixes/ORCHESTRATION_CONVERSATION_CONTEXT_FIX.md +++ b/docs/explanation/fixes/ORCHESTRATION_CONVERSATION_CONTEXT_FIX.md @@ -1,6 +1,9 @@ # Orchestration Conversation Context Fix -**Fixed in version: 0.261.096** +**Version: 0.261.101** (tracked in `application/single_app/config.py`) + +**Conversation continuity fixed in version: 0.261.096** +**Follow-up resolver compatibility fixed in version: 0.261.100** ## Issue @@ -23,11 +26,62 @@ that history nor a durable contextual interpretation. A related clarification handoff sent the user's answer without the matching elicitation schema, so the server could not validate and incorporate it. -## Changes +## Second-question resolver compatibility + +The additional resolver completion exposed a separate response-contract mismatch. +A first message could succeed, but a subsequent substantive message failed with +"Conversation context could not be used" in both new and existing conversations. +The model request itself succeeded; its JSON response was then rejected. + +A non-persisting replay against the configured GPT-4o deployment reproduced an +otherwise valid `new_topic` response containing `clarification: null`. The +validator required a string even though no question was needed. First turns +without history bypass this completion, which explains the second-question +pattern. + +### Changes in 0.261.100 + +`functions_orchestration_planner.py` now explicitly requests an empty clarification +string when no question is needed and canonicalizes an unused JSON `null` to that +same empty value. This succeeds without a repair call. A `clarification` +relationship still requires an actual nonempty question; required field types, +historical IDs, duplicate checks, and relationship constraints are not relaxed. + +Other malformed responses receive one corrective completion with the original +history, request, and clarification answers intact. Feedback contains a fixed +validation reason, not the rejected model text. Both completions contribute to +the successful turn's token usage. Persistent invalid output fails explicitly +rather than continuing without history or creating duplicate turn records. + +Filtered, refused, empty, or incomplete completions are not repaired as JSON. +Resolver provider failures do not trigger a formatting retry; an explicit unsupported +`response_format` error can still retry without that option. This retains +compatibility with the observed `2024-05-01-preview` API rather than requiring +newer structured-output support. The separate plan generator's established +retry behavior is unchanged. + +`route_backend_orchestration.py` distinguishes interpretation failures from +inaccessible or stale conversation context while retaining the existing SSE +event contract. It logs safe stage/reason codes and attempt counts through +`log_event`. A `sc_resource` value of `conversation:` +allows correlation after the streaming request context has ended. The existing +logger preserves that allowlisted diagnostic field; raw IDs, prompts, model +responses, and credentials are not included in these diagnostics. + +The corresponding configuration version update is +`application/single_app/config.py`: **0.261.099 -> 0.261.100**. +No deployment-configuration change, UI change, new capability setting, or +conversation-data migration is needed. + +### Related model-routing correction in 0.261.101 + +The resolver failure and the unexpected GPT-4o choice had separate causes. The +model-routing correction now honors the manual selection or admin default throughout +orchestration, while retaining the nullable-clarification fix described above. +See [Orchestration Model Selection Fix](ORCHESTRATION_MODEL_SELECTION_FIX.md) for +model precedence, endpoint authorization, completion parameters and validation. -| Component | Change | -| --- | --- | -| `functions_orchestration_context.py` | Normalizes eligible stored messages, respects masking and current block revisions, bounds snapshots, and verifies source fingerprints before reuse. | +## Changes Before a runnable plan exists, the validated clarification chain and original snapshot are retained in a private `pending_turn` record in the existing run @@ -38,7 +92,11 @@ a newer answer. The pending state is removed after a runnable plan is saved. Clarification chains are limited to 12 answers and 32 KiB of answer data; a complete pending record is limited to 64 KiB. Exceeding a limit produces an error rather than silently discarding earlier answers. Subsequent answers are validated against -the server's saved question, not a replacement schema from the browser.py` | Normalizes eligible stored messages, respects masking and current block revisions, bounds snapshots, and verifies source fingerprints before reuse. | +the server's saved question, not a replacement schema from the browser. + +| Component | Change | +| --- | --- | +| `functions_orchestration_context.py` | Normalizes eligible stored messages, respects masking and current block revisions, bounds snapshots, and verifies source fingerprints before reuse. | | `functions_orchestration_planner.py` | Resolves follow-ups before retrieval, distinguishes new topics and transformations, and requires self-contained queries and tasks. | | `route_backend_orchestration.py` | Loads history from an owned conversation partition, persists the interpreted turn, reuses its user-message ID, and reloads the same context for execution. | | `functions_orchestration_runs.py` | Saves run context and bounded private pending-turn state, preserving answers across successive clarifications without creating phantom runs. | @@ -87,20 +145,35 @@ are omitted from the run-list response. ## Validation +For **0.261.100**, the focused conversation-context, HTTP/SSE route, action-planning, +and research-selection suites passed **105 tests and 72 subtests**. The required +route policies, documentation coverage and quality checks, and plan-schema and +invoke-prompt contracts also passed. + +A non-persisting replay of the affected two-message history succeeded with one +resolver completion and preserved the original new-topic request. No conversation +records were changed. The null-value regression is also exercised independently +of the revised prompt, so correctness does not depend on the model always +returning an empty string. + The following suites exercise the fix without accessing production data: - `functional_tests\test_orchestration_conversation_context.py`: bounds, Unicode, - masking, source changes, request resolution, URL provenance, analysis adapters, - and synthesis prompt roles. + masking, source changes, nullable unused clarifications, strict resolution + validation, bounded correction, cumulative usage, provider/refusal handling, + URL provenance, analysis adapters, and synthesis prompt roles. - `functional_tests\test_orchestration_conversation_context_routes.py`: real Flask HTTP/SSE planning and execution with controlled external boundaries, - all approval modes, clarified requests, retries, source cutoffs, stale plans, - ownership failures, successive clarifications, pending-state isolation, and older - pending records. + first-then-second messages, existing conversations in all approval modes, + clarified requests, correction without duplicate turns, searchable privacy-safe + diagnostics, source cutoffs, stale plans, ownership failures, successive + clarifications, pending-state isolation, and older pending records. - `ui_tests\test_v2_orchestration_conversation_context.py`: the shipped controller and cards, accepting/declining/cancelling clarifications, approval modes, and navigation without retargeting a pending run. +### Historical validation notes (0.261.096) + The deterministic functional and browser scenarios pass, along with the affected orchestration contracts, route-policy coverage, and V2 TypeScript check. These results establish context transport and lifecycle behavior; controlled completions @@ -126,5 +199,6 @@ must create a new plan. Completed historical runs remain readable. ## Related - [Chat orchestration](../features/CHAT_ORCHESTRATION.md) +- [Model selection fix](ORCHESTRATION_MODEL_SELECTION_FIX.md) - [Orchestration settings](../../admin/orchestration.md) - [Chat settings](../../admin/chat.md) diff --git a/docs/explanation/fixes/ORCHESTRATION_MODEL_SELECTION_FIX.md b/docs/explanation/fixes/ORCHESTRATION_MODEL_SELECTION_FIX.md new file mode 100644 index 000000000..fe2f55fd4 --- /dev/null +++ b/docs/explanation/fixes/ORCHESTRATION_MODEL_SELECTION_FIX.md @@ -0,0 +1,156 @@ +# Orchestration Model Selection Fix + +**Version: 0.261.101** (tracked in `application/single_app/config.py`) + +**Fixed in version: 0.261.101** + +## Issue + +V2 Orchestrate could use GPT-4o even when the user selected GPT-5.6 Terra in +**Manual controls** and the admin default also pointed to Terra. The selected model +identity reached the backend and was saved with the run, but planning and answer +generation still used the legacy chat configuration. + +This was separate from the second-question resolver validation failure fixed in +**0.261.100**. That compatibility fix remains in place. + +## Root cause + +The orchestration answer closure constructed its client without the saved selection. +The planner's legacy resolver likewise preferred the classic `gpt_model.selected` +list when no dedicated deployment override was present, ignoring the modern +`default_model_selection` reference. Selecting a different deployment string alone +would not fix this: a model connection can have a different endpoint, credential, +provider protocol and caller-identity header policy. + +The answer stream also omitted model attribution, so the existing V2 message +renderer could not show the model that actually produced an orchestrated answer. + +## Resolution + +`functions_orchestration_models.py` provides an authorized binding used by the +orchestration HTTP routes. Answer selection follows this order: + +1. The model selected in Manual controls. +2. The admin default model connection, when no manual selection was supplied. +3. The configured classic single-endpoint/APIM model, only when neither applies. + +Endpoint-backed choices use `resolve_model_endpoint_from_context(..., authorize=True)` +and the existing protocol-aware client factory. Provider, endpoint, model ID and +deployment must agree. Disabled, deleted, inconsistent or inaccessible selections +fail explicitly; they do not become a request to GPT-4o. + +The resolved answer identity, including an admin default, is pinned in the saved +plan's seeds. Execution reauthorizes that selection rather than applying a newly +changed default or accepting replacement model fields on the run request. + +### Planning and research + +Without a dedicated planner override, conversational resolution, plan generation, +research query/review completions and answer synthesis use the same selection. +Trivial plans still avoid a planning completion. + +A dedicated planner remains independent of the answer. Deployment-only overrides +retain the classic/APIM connection. The existing planner endpoint/model/provider +fields can instead identify a separate authorized model connection. An unavailable +answer or planner selection fails rather than falling back to another connection. +Standalone legacy planner callers retain their existing interface. + +Clients and caller identity are captured before streaming leaves Flask's request +context. Research receives the captured planner binding, and direct actions receive +the actual answer model identity. Configured agents retain their own model behavior. + +### Completion compatibility and lifecycle + +GPT-5-family calls use `max_completion_tokens` rather than `max_tokens` and omit +unsupported temperature. Supported manual reasoning effort is retained for the +selected model; a separate planner keeps its independent defaults. Behavior uses +the underlying model name when available, so a custom deployment alias is supported. + +A positive configured model-connection `responseLength` is honored for answer calls. Otherwise, +reasoning completion budgets have an 8192-token floor to leave room for reasoning +and visible output. Planning uses that bounded allowance rather than treating its +smaller visible JSON budget as the entire reasoning budget. + +The Anthropic adapter normalizes successful `end_turn` and `stop_sequence` +reasons to the chat-completions `stop` value. Truncation, tool use and refusal +retain their distinct meanings. This lets a valid Claude follow-up pass the +same strict completion checks without accepting a truncated or refused JSON response. + +Client bindings close idempotently after completion and failures. A stream closed +before starting releases its clients; disconnecting from an active run does not +close a client still in use by its worker. + +Empty, refused, filtered and provider-failed answer completions fail the run +explicitly. The stream no longer presents these as completed empty answers. +Provider response text is not included in the new answer-error messages. + +### Attribution + +The saved assistant message and terminal SSE carry the actual +`model_deployment_name`, provider and available endpoint/model IDs. Only model +identity is exposed, not endpoint addresses or credentials. The existing V2 message +renderer displays the returned deployment name without new browser runtime code. + +## Files and impact + +| Component | Change | +| --- | --- | +| `functions_orchestration_models.py` | Shared authorized selection, client binding, parameter compatibility and safe identity metadata. | +| `functions_orchestration_context.py` | Retains manual reasoning effort with the model seeds. | +| `functions_orchestration_planner.py` | Accepts the captured binding for resolution, planning and clarification replanning without changing standalone callers. | +| `route_backend_orchestration.py` | Pins answer selection, reauthorizes execution, binds synthesis, closes clients and surfaces failed answers. | +| `functions_orchestration_executor.py`, `functions_orchestration_adapters.py` | Carry the selected planner into research instead of resolving an unrelated legacy client. | +| `functions_orchestration_events.py` | Includes actual answer-model metadata in the terminal stream. | +| `model_endpoint_clients.py` | Normalizes Anthropic completion reasons for chat-completions and Semantic Kernel consumers. | +| `application/single_app/config.py` | Advances the patch version from `0.261.100` to `0.261.101`. | + +No new setting, dependency, conversation migration or deployment-configuration +change is required. Previously saved explicit selections are honored when their +pending plans run. Older plans without a model selection resolve the current +configured default. New plans pin the resolved default before approval. + +## Validation + +The focused orchestration, endpoint authorization, protocol and standalone +planner suites passed **255 tests and 208 subtests**. Coverage includes manual and +default selection, separate planner connections, all approval modes, first and +second questions, unused null clarifications, stale defaults, access revocation, +protocol parameters, native Anthropic completion flags, action/research identity, +token usage and client cleanup. + +The local Playwright harness passed **5 tests and 3 subtests** using the shipped +controller, stores and components. It exercises model/clarification transport and +the visible answer-model label in Auto, countdown and manual approval. + +A two-turn synthetic replay used the real configured Terra SDK endpoint with the +Flask route and in-memory conversation/run containers. The follow-up resolved the +prior answer successfully; all three model completions used Terra and finished +normally with the configured `2024-05-01-preview` API. No production conversation +or settings records were written, and no deployment was performed. + +Primary regressions: + +- `functional_tests/test_orchestration_model_selection.py` +- `functional_tests/test_orchestration_conversation_context_routes.py` +- `functional_tests/test_orchestration_conversation_context.py` +- `functional_tests/test_model_endpoint_protocol_inference.py` +- `ui_tests/test_v2_orchestration_conversation_context.py` + +These results establish routing and compatibility, not factual accuracy or an +availability guarantee for every configured provider. + +### Existing adjacent-test failures + +Additional tabular/export-summary checks exposed four failures that also reproduce +against the unchanged **6a34c8a6** baseline. The tabular test requires an exact historical +`0.241.186` application version. Three export-summary tests omit +`build_model_endpoint_identity_headers` from their extracted helper namespace. +These unrelated tests and the export route are unchanged by this fix. + +## Related + +- [Conversation context fix](ORCHESTRATION_CONVERSATION_CONTEXT_FIX.md) +- [Chat orchestration](../features/CHAT_ORCHESTRATION.md) +- [Orchestration settings](../../admin/orchestration.md) +- [AI Models settings](../../admin/ai-models.md) diff --git a/functional_tests/test_model_endpoint_protocol_inference.py b/functional_tests/test_model_endpoint_protocol_inference.py index c2d9ce34c..f69f82160 100644 --- a/functional_tests/test_model_endpoint_protocol_inference.py +++ b/functional_tests/test_model_endpoint_protocol_inference.py @@ -2,8 +2,9 @@ #!/usr/bin/env python3 """ Functional test for model endpoint protocol inference. -Version: 0.250.109 +Version: 0.261.101 Implemented in: 0.241.179; updated in 0.250.109 +Anthropic completion reason normalization: 0.261.101 This test ensures that Foundry model endpoint runtime calls infer Claude as Anthropic messages, OpenAI-compatible Foundry endpoints as /openai/v1, and @@ -39,6 +40,7 @@ from semantic_kernel.contents.chat_history import ChatHistory # noqa: E402 from semantic_kernel.contents.chat_message_content import ChatMessageContent # noqa: E402 from semantic_kernel.contents.utils.author_role import AuthorRole # noqa: E402 +from semantic_kernel.contents.utils.finish_reason import FinishReason # noqa: E402 def assert_equal(actual, expected, description): @@ -214,6 +216,31 @@ def test_model_endpoint_protocol_inference(): assert_equal(sk_payload["temperature"], 0.2, "SK Claude service should copy temperature") assert_equal(sk_payload["stream"], True, "SK Claude service should support streaming") + for native_reason, expected_reason, expected_sk_reason in ( + ("end_turn", "stop", FinishReason.STOP), + ("stop_sequence", "stop", FinishReason.STOP), + ("max_tokens", "length", FinishReason.LENGTH), + ("model_context_window_exceeded", "length", FinishReason.LENGTH), + ("tool_use", "tool_calls", FinishReason.TOOL_CALLS), + ("refusal", "content_filter", FinishReason.CONTENT_FILTER), + ("pause_turn", "pause_turn", None), + (None, None, None), + ): + response = client._build_completion_response({ + "stop_reason": native_reason, + "content": [{"type": "text", "text": "Response text"}], + "usage": {"input_tokens": 10, "output_tokens": 5}, + }) + assert_equal( + response.choices[0].finish_reason, expected_reason, + f"Anthropic {native_reason} should keep its completion meaning", + ) + message = sk_service._create_chat_message_contents_from_response(response)[0] + assert_equal( + message.finish_reason, expected_sk_reason, + f"SK should receive the normalized {native_reason} completion reason", + ) + loader_content = (APP_DIR / "semantic_kernel_loader.py").read_text(encoding="utf-8") if "create_model_endpoint_chat_completion_service" not in loader_content: raise AssertionError("Semantic Kernel loader should centralize endpoint chat service creation.") diff --git a/functional_tests/test_orchestration_conversation_context.py b/functional_tests/test_orchestration_conversation_context.py index 228b827db..954293873 100644 --- a/functional_tests/test_orchestration_conversation_context.py +++ b/functional_tests/test_orchestration_conversation_context.py @@ -1,8 +1,9 @@ # test_orchestration_conversation_context.py """ Functional regressions for bounded, conversation-aware orchestration. -Version: 0.261.099 +Version: 0.261.101 Implemented in: 0.261.096 +Resolver response compatibility and bounded recovery: 0.261.100 Exercises the real history, resolution, triage, and adapter code with external model/search/analysis boundaries replaced. No Azure resources or credentials are used. @@ -15,7 +16,10 @@ import unittest from copy import deepcopy from types import SimpleNamespace -from unittest.mock import patch +from unittest.mock import Mock, patch + +from httpx import Request, Response +from openai import AuthenticationError, BadRequestError, RateLimitError from test_support.app_stubs import stubbed_app_imports, stubbed_config from test_support.versioning import assert_app_version_at_least @@ -209,13 +213,15 @@ def setUp(self): 'clarification': '', } - def resolve(self, payload=None, text=LATEST, answers=None): + def resolve(self, payload=None, text=LATEST, answers=None, replies=None): planner = self.modules.planner usage = SimpleNamespace(prompt_tokens=30, completion_tokens=20, total_tokens=50) with patch.object(planner, 'resolve_planner_client', return_value=(object(), 'planner')), \ patch.object(planner, '_call_planner', return_value=( json.dumps(payload if payload is not None else self.payload), usage )) as call: + if replies is not None: + call.side_effect = [(json.dumps(reply), usage) for reply in replies] result = planner.resolve_conversation_request( text, self.snapshot, settings={}, answered_questions=answers ) @@ -267,6 +273,139 @@ def test_clarification_answers_are_available_to_resolution(self): self.assertEqual(supplied['answered_questions'], answers) self.assertEqual(result['resolved_message'], RESOLVED) + def test_unused_null_clarification_is_normalized_without_a_repair_call(self): + original_snapshot = deepcopy(self.snapshot) + for relationship in ('follow_up', 'new_topic'): + with self.subTest(relationship=relationship): + payload = {**self.payload, 'relationship': relationship, 'clarification': None} + if relationship == 'new_topic': + payload['message_ids'] = [] + result, call = self.resolve(payload) + self.assertEqual(result['clarification'], '') + self.assertEqual(result['relationship'], relationship) + self.assertEqual(result['message_ids'], payload['message_ids']) + self.assertEqual(result['resolved_message'], RESOLVED if relationship == 'follow_up' else LATEST) + self.assertEqual(result['token_usage']['total_tokens'], 50) + self.assertEqual(call.call_count, 1) + self.assertIsNone(payload['clarification']) + self.assertEqual(self.snapshot, original_snapshot) + + def test_repair_preserves_input_and_accounts_for_both_completions(self): + planner = self.modules.planner + answers = [{'question': 'Which day?', 'answer': {'day': 'Wednesday'}}] + invalid = {**self.payload, 'message_ids': ['PRIVATE_FORGED_MESSAGE_ID']} + with patch.object(planner, 'log_event') as log: + result, call = self.resolve(answers=answers, replies=[invalid, self.payload]) + self.assertEqual(call.call_count, 2) + original_messages = call.call_args_list[0].args[2] + repaired_messages = call.call_args_list[1].args[2] + self.assertEqual(repaired_messages[:2], original_messages) + original_payload = json.loads(repaired_messages[1]['content']) + self.assertEqual(original_payload['answered_questions'], answers) + self.assertEqual(original_payload['original_message'], LATEST) + self.assertIn('Schmidt', json.dumps(original_payload['conversation'])) + self.assertIn('unknown_message_ids', repaired_messages[-1]['content']) + self.assertNotIn('PRIVATE_FORGED_MESSAGE_ID', json.dumps(repaired_messages)) + self.assertNotIn('PRIVATE_FORGED_MESSAGE_ID', repr(log.call_args_list)) + self.assertEqual(result['message_ids'], self.payload['message_ids']) + self.assertEqual(result['token_usage'], { + 'prompt_tokens': 60, 'completion_tokens': 40, 'total_tokens': 100, + }) + + def test_unparseable_output_can_be_repaired_without_replaying_it(self): + result, call = self.resolve(replies=['PRIVATE_INVALID_OUTPUT', self.payload]) + self.assertEqual(call.call_count, 2) + self.assertEqual(result['resolved_message'], RESOLVED) + self.assertIn('invalid_json', call.call_args.args[2][-1]['content']) + self.assertNotIn('PRIVATE_INVALID_OUTPUT', json.dumps(call.call_args.args[2])) + + def test_persistent_invalid_output_has_an_exact_attempt_limit_and_safe_reason(self): + planner = self.modules.planner + invalid = {**self.payload, 'resolved_message': 'PRIVATE_MODEL_TEXT', 'requires_retrieval': 'false'} + with patch.object(planner, 'resolve_planner_client', return_value=(object(), 'planner')), \ + patch.object(planner, '_call_planner', return_value=(json.dumps(invalid), None)) as call, \ + patch.object(planner, 'log_event') as log: + with self.assertRaises(planner.ConversationResolutionError) as failure: + planner.resolve_conversation_request(LATEST, self.snapshot) + self.assertEqual(call.call_count, 2) + self.assertEqual(failure.exception.attempts, 2) + self.assertEqual(failure.exception.reason, 'invalid_retrieval_flag') + self.assertNotIn('PRIVATE_MODEL_TEXT', str(failure.exception)) + self.assertNotIn('PRIVATE_MODEL_TEXT', repr(log.call_args_list)) + + def model_response(self, *, finish_reason='stop', refusal=None, content=None): + return SimpleNamespace( + choices=[SimpleNamespace( + finish_reason=finish_reason, + message=SimpleNamespace( + content=json.dumps(self.payload) if content is None else content, + refusal=refusal, + ), + )], + usage=SimpleNamespace(prompt_tokens=30, completion_tokens=20, total_tokens=50), + ) + + def test_refused_incomplete_and_absent_completions_are_not_repaired(self): + planner = self.modules.planner + cases = [ + (self.model_response(finish_reason='content_filter'), 'model_refusal'), + (self.model_response(refusal='PRIVATE_REFUSAL'), 'model_refusal'), + (self.model_response(finish_reason='length'), 'incomplete_completion'), + (self.model_response(content=''), 'empty_completion'), + (SimpleNamespace(choices=[]), 'empty_completion'), + ] + for response, reason in cases: + with self.subTest(reason=reason): + client = Mock() + client.chat.completions.create.return_value = response + with patch.object(planner, 'resolve_planner_client', return_value=(client, 'planner')), \ + patch.object(planner, 'log_event') as log: + with self.assertRaises(planner.ConversationResolutionError) as failure: + planner.resolve_conversation_request(LATEST, self.snapshot) + self.assertEqual(client.chat.completions.create.call_count, 1) + self.assertEqual(failure.exception.attempts, 1) + self.assertEqual(failure.exception.reason, reason) + self.assertNotIn('PRIVATE_REFUSAL', repr(log.call_args_list)) + + def test_provider_failures_are_not_retried_as_json_format_or_schema_failures(self): + planner = self.modules.planner + for error_type, status in ( + (AuthenticationError, 401), (RateLimitError, 429), (BadRequestError, 400), + ): + with self.subTest(status=status): + client = Mock() + response = Response(status, request=Request('POST', 'https://model.example.test/completions')) + client.chat.completions.create.side_effect = error_type( + 'PRIVATE_PROVIDER_DETAIL', response=response, + body={'error': {'code': 'content_filter', 'message': 'PRIVATE_PROVIDER_DETAIL'}}, + ) + with patch.object(planner, 'resolve_planner_client', return_value=(client, 'planner')), \ + patch.object(planner, 'log_event') as log: + with self.assertRaises(planner.ConversationResolutionError) as failure: + planner.resolve_conversation_request(LATEST, self.snapshot) + self.assertEqual(client.chat.completions.create.call_count, 1) + self.assertEqual(failure.exception.reason, 'model_request_failed') + self.assertNotIn('PRIVATE_PROVIDER_DETAIL', repr(log.call_args_list)) + self.assertNotIn('PRIVATE_PROVIDER_DETAIL', str(failure.exception)) + + def test_unsupported_json_format_keeps_the_compatible_fallback(self): + planner = self.modules.planner + response = Response(400, request=Request('POST', 'https://model.example.test/completions')) + error = BadRequestError('Unsupported format', response=response, body={'error': { + 'message': "'response_format' of type 'json_object' is not supported with this model.", + 'param': None, 'code': None, + }}) + client = Mock() + client.chat.completions.create.side_effect = [error, self.model_response()] + with patch.object(planner, 'resolve_planner_client', return_value=(client, 'planner')): + result = planner.resolve_conversation_request(LATEST, self.snapshot) + calls = client.chat.completions.create.call_args_list + self.assertEqual(len(calls), 2) + self.assertIn('response_format', calls[0].kwargs) + self.assertNotIn('response_format', calls[1].kwargs) + self.assertEqual(result['message_ids'], self.payload['message_ids']) + self.assertEqual(result['token_usage']['total_tokens'], 50) + def test_malformed_and_forged_resolution_do_not_become_silent_fallbacks(self): invalid = [ {}, @@ -274,6 +413,13 @@ def test_malformed_and_forged_resolution_do_not_become_silent_fallbacks(self): {**self.payload, 'message_ids': ['u1', 'u1']}, {**self.payload, 'requires_retrieval': 'false'}, {**self.payload, 'relationship': 'clarification', 'clarification': ''}, + {**self.payload, 'relationship': 'clarification', 'clarification': None}, + {**self.payload, 'clarification': False}, + {**self.payload, 'message_ids': []}, + {**self.payload, 'message_ids': None}, + {**self.payload, 'message_ids': [{}]}, + {**self.payload, 'resolved_message': None}, + {**self.payload, 'relationship': 'new_topic'}, {**self.payload, 'resolved_message': 'x' * 6001}, ] for payload in invalid: @@ -455,5 +601,5 @@ def stale(): if __name__ == '__main__': - assert_app_version_at_least('0.261.096') + assert_app_version_at_least('0.261.100') unittest.main() diff --git a/functional_tests/test_orchestration_conversation_context_routes.py b/functional_tests/test_orchestration_conversation_context_routes.py index 50d00e5a6..7a91233a3 100644 --- a/functional_tests/test_orchestration_conversation_context_routes.py +++ b/functional_tests/test_orchestration_conversation_context_routes.py @@ -1,16 +1,19 @@ # test_orchestration_conversation_context_routes.py """ Functional tests for conversation context across real orchestration HTTP/SSE routes. -Version: 0.261.098 +Version: 0.261.101 Implemented in: 0.261.096 Prompt attachment integration: 0.261.097 Direct action integration: 0.261.098 +Resolver response compatibility and bounded recovery: 0.261.100 +Authorized model routing and completion metadata: 0.261.101 Uses Flask, the real planner/executor/adapters/run store, an in-memory Cosmos boundary, and deterministic model completions. Authentication is a signed-in test user; actual conversation ownership checks remain active. No external service is contacted. """ +import hashlib import importlib import importlib.util import json @@ -18,22 +21,27 @@ import sys import unittest from copy import deepcopy +from threading import Event from types import SimpleNamespace -from unittest.mock import patch +from unittest.mock import Mock, patch from azure.core.exceptions import AzureError from azure.cosmos.exceptions import ( CosmosAccessConditionFailedError, CosmosResourceExistsError, CosmosResourceNotFoundError, ) -from flask import Blueprint, Flask +from flask import Blueprint, Flask, has_request_context +from httpx import Request as HttpRequest, Response as HttpResponse +from openai import BadRequestError from werkzeug.test import Client from werkzeug.wrappers import Response from test_orchestration_conversation_context import ( LATEST, RESOLVED, fake_module, load_modules, message, winery_history, ) +from test_orchestration_model_selection import TERRA_SELECTION, endpoint_runtime, model_endpoint from test_support.app_stubs import APP_ROOT, stubbed_config from test_support.versioning import assert_app_version_at_least +from test_unified_logging_entrypoint import _install_logging_stubs, _restore_modules class MemoryContainer: @@ -119,14 +127,18 @@ def __init__(self, modules): self.modules = modules self.calls = [] self.resolution_override = None + self.resolution_responses = [] self.plan_override = None + self.answer_response = None + self.answer_error = None self.chat = SimpleNamespace(completions=SimpleNamespace(create=self.create)) def create(self, **kwargs): self.calls.append(deepcopy(kwargs)) system = kwargs['messages'][0]['content'] if system == self.modules.planner.RESOLUTION_SYSTEM_PROMPT: - text = json.dumps(self.resolution_override or { + payload = self.resolution_responses.pop(0) if self.resolution_responses else self.resolution_override + text = json.dumps(payload if payload is not None else { 'relationship': 'follow_up', 'resolved_message': RESOLVED, 'message_ids': ['u1', 'u2', 'a2'], @@ -151,9 +163,13 @@ def create(self, **kwargs): ], }) else: + if self.answer_error is not None: + raise self.answer_error + if self.answer_response is not None: + return self.answer_response text = 'You mean the wineries near Grants Pass. I do not have verified Wednesday hours.' return SimpleNamespace( - choices=[SimpleNamespace(message=SimpleNamespace(content=text))], + choices=[SimpleNamespace(finish_reason='stop', message=SimpleNamespace(content=text))], usage=SimpleNamespace(prompt_tokens=10, completion_tokens=5, total_tokens=15), ) @@ -266,6 +282,264 @@ def run_plan(self, plan): 'run_id': plan['run_id'], 'conversation_id': 'conv1', }, buffered=True) + def use_modern_models(self): + self.endpoint = model_endpoint() + self.model_clients = [] + self.model_runtime = endpoint_runtime(self.endpoint, None) + + def build_client(*args, **kwargs): + self.assertTrue(has_request_context(), 'Model authorization must stay on the request thread.') + client = SimpleNamespace(chat=self.model.chat, close=Mock()) + self.model_clients.append(client) + return client, 'azure_openai' + + self.model_runtime.build_model_endpoint_sync_chat_client.side_effect = build_client + self.settings.update({ + 'enable_multi_model_endpoints': True, + 'default_model_selection': { + 'endpoint_id': 'selected-endpoint', 'model_id': 'terra-model', 'provider': 'aoai', + }, + 'gpt_model': {'selected': [{'deploymentName': 'gpt-4o'}]}, + }) + patcher = patch.dict(sys.modules, {'functions_model_endpoint_runtime': self.model_runtime}) + patcher.start() + self.addCleanup(patcher.stop) + return dict(TERRA_SELECTION) + + def test_manual_model_is_used_for_resolution_planning_and_answer_in_all_approval_modes(self): + selection = self.use_modern_models() + self.settings['default_model_selection']['model_id'] = 'luna-model' + for mode in ('auto', 'timed', 'manual'): + with self.subTest(mode=mode): + self.messages.items.clear() + self.runs.items.clear() + self.model.calls.clear() + for row in winery_history(): + self.messages.upsert_item(row) + plan = self.planned(**selection, reasoning_effort='high', approval_mode=mode) + stored = self.runs.read_item(plan['run_id'], 'conv1') + self.assertEqual(stored['seeds']['model'], selection) + self.assertEqual(stored['seeds']['reasoning_effort'], 'high') + self.assertEqual(stored['plan']['planner_model'], 'gpt-5.6-terra') + events = frames(self.run_plan(plan)) + self.assertFalse(any(event.get('error') for event in events), events) + self.assertEqual(len(self.model.calls), 3) + for call in self.model.calls: + self.assertEqual(call['model'], 'gpt-5.6-terra') + self.assertEqual(call['reasoning_effort'], 'high') + self.assertIn('max_completion_tokens', call) + self.assertNotIn('max_tokens', call) + self.assertNotIn('temperature', call) + updated = self.runs.read_item(plan['run_id'], 'conv1') + answer = self.messages.read_item(updated['assistant_message_id'], 'conv1') + terminal = next(event for event in events if event.get('type') == 'orchestration_done') + for key, value in { + 'model_deployment_name': 'gpt-5.6-terra', 'model_provider': 'aoai', + 'model_endpoint_id': 'selected-endpoint', 'model_id': 'terra-model', + }.items(): + self.assertEqual(answer[key], value) + self.assertEqual(terminal[key], value) + self.assertEqual(updated['token_usage']['total_tokens'], 45) + for client in self.model_clients: + client.close.assert_called_once_with() + + def test_admin_default_is_pinned_to_the_plan_and_cannot_be_retargeted_at_run_time(self): + self.use_modern_models() + plan = self.planned() + self.settings['default_model_selection']['model_id'] = 'luna-model' + response = self.client.post('/api/v2/orchestration/run', json={ + 'run_id': plan['run_id'], 'conversation_id': 'conv1', + 'model_id': 'luna-model', 'model_deployment': 'gpt-5.6-luna', + }, buffered=True) + self.assertFalse(any(event.get('error') for event in frames(response))) + self.assertEqual({call['model'] for call in self.model.calls}, {'gpt-5.6-terra'}) + self.assertEqual(self.runs.read_item(plan['run_id'], 'conv1')['seeds']['model'], TERRA_SELECTION) + + def test_planner_override_does_not_change_the_selected_answer_or_research_binding(self): + selection = self.use_modern_models() + self.settings['chat_orchestration_planner_deployment'] = 'small-planner' + contexts = [] + run_context = self.route.RunContext + + def capture_context(**kwargs): + context = run_context(**kwargs) + contexts.append(context) + return context + + with patch.object( + self.modules.planner, 'resolve_planner_client', + side_effect=lambda settings: (self.model, settings['chat_orchestration_planner_deployment']), + ), patch.object(self.route, 'RunContext', side_effect=capture_context): + plan = self.planned(**selection) + events = frames(self.run_plan(plan)) + self.assertFalse(any(event.get('error') for event in events), events) + self.assertEqual([call['model'] for call in self.model.calls], [ + 'small-planner', 'small-planner', 'gpt-5.6-terra', + ]) + self.assertEqual(contexts[0].gpt_model, 'gpt-5.6-terra') + self.assertEqual(contexts[0].planner_deployment, 'small-planner') + client, deployment = self.modules.adapters._resolve_source_review_planner(self.settings, contexts[0]) + self.assertIs(client, contexts[0].planner_client) + self.assertEqual(deployment, 'small-planner') + self.assertEqual(self.runs.read_item(plan['run_id'], 'conv1')['seeds']['model'], selection) + + def test_revoked_model_access_stops_an_approved_run_instead_of_falling_back(self): + selection = self.use_modern_models() + plan = self.planned(**selection) + self.model_runtime.resolve_model_endpoint_from_context.return_value = None + self.settings['default_model_selection']['model_id'] = 'luna-model' + response = self.run_plan(plan) + self.assertEqual(response.status_code, 403) + self.assertIn('selected model is unavailable', response.get_json()['error']) + self.assertEqual(len(self.model.calls), 2) + self.assertTrue(all(call['model'] == 'gpt-5.6-terra' for call in self.model.calls)) + self.assertEqual(self.model_runtime.resolve_model_endpoint_from_context.call_count, 2) + self.assertTrue(self.model_runtime.resolve_model_endpoint_from_context.call_args.kwargs['authorize']) + self.assertEqual(self.model_runtime.build_model_endpoint_sync_chat_client.call_count, 1) + + def test_model_id_only_planner_override_also_reaches_research_execution(self): + self.use_modern_models() + self.settings.update({ + 'chat_orchestration_planner_model_endpoint_id': 'selected-endpoint', + 'chat_orchestration_planner_model_id': 'luna-model', + }) + contexts = [] + run_context = self.route.RunContext + + def capture_context(**kwargs): + context = run_context(**kwargs) + contexts.append(context) + return context + + with patch.object(self.route, 'RunContext', side_effect=capture_context): + plan = self.planned() + events = frames(self.run_plan(plan)) + self.assertFalse(any(event.get('error') for event in events), events) + self.assertEqual([call['model'] for call in self.model.calls], [ + 'gpt-5.6-luna', 'gpt-5.6-luna', 'gpt-5.6-terra', + ]) + self.assertEqual(contexts[0].planner_deployment, 'gpt-5.6-luna') + self.assertEqual(contexts[0].gpt_model, 'gpt-5.6-terra') + self.assertEqual(self.runs.read_item(plan['run_id'], 'conv1')['seeds']['model'], TERRA_SELECTION) + for client in self.model_clients: + client.close.assert_called_once_with() + + def test_research_model_initialization_failure_closes_the_created_answer_client(self): + self.use_modern_models() + plan = self.planned() + self.settings['chat_orchestration_planner_deployment'] = 'small-planner' + self.model_runtime.resolve_model_endpoint_from_context.side_effect = [self.endpoint, None] + response = self.run_plan(plan) + self.assertEqual(response.status_code, 403) + self.assertEqual(len(self.model_clients), 2) + for client in self.model_clients: + client.close.assert_called_once_with() + + def test_unstarted_streams_close_their_model_clients(self): + self.use_modern_models() + with self.app.test_request_context('/api/v2/orchestration/plan', method='POST', json={ + 'message': LATEST, 'conversation_id': 'conv1', 'turn_id': 'abandoned', + }): + response = self.app.view_functions['context_test.orchestration_plan']() + response.close() + self.assertEqual(self.model.calls, []) + self.assertEqual(self.runs.items, {}) + self.model_clients[-1].close.assert_called_once_with() + + plan = self.planned() + with self.app.test_request_context('/api/v2/orchestration/run', method='POST', json={ + 'run_id': plan['run_id'], 'conversation_id': 'conv1', + }): + response = self.app.view_functions['context_test.orchestration_run']() + response.close() + self.assertEqual(len(self.model.calls), 2) + for client in self.model_clients: + client.close.assert_called_once_with() + + def test_disconnecting_does_not_close_a_model_still_used_by_the_worker(self): + self.use_modern_models() + plan = self.planned() + searching, resume, closed = Event(), Event(), Event() + + def pause_search(): + searching.set() + self.assertTrue(resume.wait(timeout=10)) + + self.after_search = pause_search + response = self.client.post('/api/v2/orchestration/run', json={ + 'run_id': plan['run_id'], 'conversation_id': 'conv1', + }, buffered=False) + self.model_clients[-1].close.side_effect = closed.set + try: + self.assertTrue(searching.wait(timeout=10)) + response.close() + self.model_clients[-1].close.assert_not_called() + finally: + resume.set() + self.assertTrue(closed.wait(timeout=10)) + self.model_clients[-1].close.assert_called_once_with() + + def test_unavailable_default_fails_planning_without_creating_a_run(self): + self.use_modern_models() + self.model_runtime.resolve_model_endpoint_from_context.return_value = None + _, events = self.plan() + self.assertTrue(any('selected model is unavailable' in event.get('error', '') for event in events)) + self.assertEqual(self.model.calls, []) + self.assertEqual(self.runs.items, {}) + self.assertEqual(len(self.messages.items), 4) + + def test_failed_or_empty_answer_is_not_reported_as_a_completed_turn(self): + self.use_modern_models() + usage = SimpleNamespace(prompt_tokens=10, completion_tokens=5, total_tokens=15) + for response in ( + SimpleNamespace(choices=[], usage=usage), + SimpleNamespace(choices=[SimpleNamespace( + finish_reason='length', message=SimpleNamespace(content='', refusal=None), + )], usage=usage), + SimpleNamespace(choices=[SimpleNamespace( + finish_reason='content_filter', + message=SimpleNamespace(content='PRIVATE_PROVIDER_RESPONSE', refusal=None), + )], usage=usage), + SimpleNamespace(choices=[SimpleNamespace( + finish_reason='stop', + message=SimpleNamespace(content='', refusal='PRIVATE_PROVIDER_RESPONSE'), + )], usage=usage), + ): + with self.subTest(response=response): + self.messages.items.clear() + self.runs.items.clear() + for row in winery_history(): + self.messages.upsert_item(row) + self.model.answer_response = response + plan = self.planned() + events = frames(self.run_plan(plan)) + self.assertTrue(any(event.get('error') for event in events), events) + self.assertFalse(any(event.get('type') == 'orchestration_done' for event in events)) + self.assertNotIn('PRIVATE_PROVIDER_RESPONSE', json.dumps(events)) + stored = self.runs.read_item(plan['run_id'], 'conv1') + self.assertEqual(stored['status'], 'failed') + self.assertFalse(stored.get('assistant_message_id')) + self.assertEqual(stored['token_usage']['total_tokens'], 45) + for client in self.model_clients: + client.close.assert_called_once_with() + + def test_provider_answer_error_is_safe_and_closes_the_bound_client(self): + self.use_modern_models() + self.model.answer_error = BadRequestError( + 'PRIVATE_PROVIDER_RESPONSE', + response=HttpResponse(400, request=HttpRequest('POST', 'https://selected.example.test')), + body={'error': 'PRIVATE_PROVIDER_RESPONSE'}, + ) + plan = self.planned() + with patch.object(self.route, 'log_event') as log: + events = frames(self.run_plan(plan)) + self.assertTrue(any(event.get('error') for event in events)) + self.assertNotIn('PRIVATE_PROVIDER_RESPONSE', json.dumps(events)) + self.assertNotIn('PRIVATE_PROVIDER_RESPONSE', repr(log.call_args_list)) + self.assertEqual(self.runs.read_item(plan['run_id'], 'conv1')['status'], 'failed') + for client in self.model_clients: + client.close.assert_called_once_with() + def test_prompt_snapshot_and_fingerprint_survive_replanning_without_duplicate_messages(self): prompt_text = "Use the winery context." content = f"{prompt_text}\n\n{LATEST}" @@ -298,6 +572,10 @@ def test_prompt_snapshot_and_fingerprint_survive_replanning_without_duplicate_me ) def test_multiturn_plan_and_run_share_context_in_every_approval_mode(self): + self.model.resolution_override = { + 'relationship': 'follow_up', 'resolved_message': RESOLVED, + 'message_ids': ['u1', 'u2', 'a2'], 'requires_retrieval': True, 'clarification': None, + } for mode in ('auto', 'timed', 'manual'): with self.subTest(mode=mode): self.messages.items.clear() @@ -311,6 +589,7 @@ def test_multiturn_plan_and_run_share_context_in_every_approval_mode(self): stored = self.runs.read_item(plan['run_id'], 'conv1') self.assertEqual(stored['user_message'], LATEST) self.assertEqual(stored['resolved_message'], RESOLVED) + self.assertEqual(stored['request_resolution']['clarification'], '') self.assertEqual(len(stored['conversation_context']['messages']), 4) self.assertEqual(stored['planning_token_usage']['total_tokens'], 30) response = self.run_plan(plan) @@ -325,7 +604,92 @@ def test_multiturn_plan_and_run_share_context_in_every_approval_mode(self): self.assertEqual(updated['status'], 'completed') self.assertEqual(updated['token_usage']['total_tokens'], 45) + def test_new_conversation_second_question_accepts_an_unused_null_clarification(self): + self.use_modern_models() + self.messages.items.clear() + first = self.planned(message='Find tide pools near Crescent City.', turn_id='first-turn') + self.assertFalse(any(event.get('error') for event in frames(self.run_plan(first)))) + first_run = self.runs.read_item(first['run_id'], 'conv1') + question = 'Find wineries open Wednesday on Route 199 from Medford to Crescent City after 1 PM.' + self.model.resolution_override = { + 'relationship': 'new_topic', 'resolved_message': question, + 'message_ids': [], 'requires_retrieval': True, 'clarification': None, + } + second = self.planned(message=question, turn_id='second-turn') + second_run = self.runs.read_item(second['run_id'], 'conv1') + self.assertEqual( + {item['id'] for item in second_run['conversation_context']['messages']}, + {first_run['user_message_id'], first_run['assistant_message_id']}, + ) + self.assertEqual(second_run['resolved_message'], question) + self.assertEqual(second_run['request_resolution']['message_ids'], []) + self.assertEqual(second_run['request_resolution']['clarification'], '') + self.assertFalse(any(event.get('error') for event in frames(self.run_plan(second)))) + self.assertEqual(self.runs.read_item(second['run_id'], 'conv1')['status'], 'completed') + self.assertEqual(len(self.messages.items), 4) + resolution_calls = [ + call for call in self.model.calls + if call['messages'][0]['content'] == self.modules.planner.RESOLUTION_SYSTEM_PROMPT + ] + self.assertEqual(len(resolution_calls), 1) + self.assertEqual({call['model'] for call in self.model.calls}, {'gpt-5.6-terra'}) + + def test_repaired_resolution_keeps_context_usage_and_single_turn_persistence(self): + valid = { + 'relationship': 'follow_up', 'resolved_message': RESOLVED, + 'message_ids': ['u1', 'u2', 'a2'], 'requires_retrieval': True, 'clarification': '', + } + self.model.resolution_responses = [ + {**valid, 'message_ids': ['foreign-message']}, valid, + ] + plan = self.planned() + stored = self.runs.read_item(plan['run_id'], 'conv1') + self.assertEqual(stored['request_resolution']['message_ids'], valid['message_ids']) + self.assertEqual(len(stored['conversation_context']['messages']), 4) + self.assertEqual(stored['planning_token_usage']['total_tokens'], 45) + self.assertFalse(any(event.get('error') for event in frames(self.run_plan(plan)))) + self.assertEqual(self.runs.read_item(plan['run_id'], 'conv1')['token_usage']['total_tokens'], 60) + self.assertEqual(self.search_queries, [RESOLVED, RESOLVED]) + self.assertEqual(sum(row.get('content') == LATEST for row in self.messages.items.values()), 1) + self.assertEqual(len(self.runs.items), 1) + + def test_persistent_resolution_failure_has_safe_searchable_diagnostics_and_no_writes(self): + self.model.resolution_override = { + 'relationship': 'follow_up', 'resolved_message': 'PRIVATE_MODEL_TEXT', + 'message_ids': ['u1'], 'requires_retrieval': 'false', 'clarification': '', + } + with patch.object(self.route, 'log_event') as route_log, \ + patch.object(self.modules.planner, 'log_event') as planner_log: + response, events = self.plan() + self.assertEqual(response.status_code, 200) + self.assertEqual([event['error'] for event in events if event.get('error')], [ + 'The conversation could not be interpreted. Please retry your request.', + ]) + self.assertEqual(len(self.model.calls), 2) + self.assertEqual(len(self.messages.items), 4) + self.assertEqual(self.runs.items, {}) + self.assertEqual(self.search_queries, []) + properties = route_log.call_args.kwargs['extra'] + resource = f"conversation:{hashlib.sha256(b'conv1').hexdigest()}" + self.assertEqual(properties['resource'], resource) + self.assertEqual(properties['reason'], 'invalid_retrieval_flag') + self.assertEqual(properties['attempt'], 2) + logged = repr(route_log.call_args_list) + repr(planner_log.call_args_list) + for private in ('PRIVATE_MODEL_TEXT', LATEST, 'Schmidt', 'conv1'): + self.assertNotIn(private, logged) + + saved_modules = _install_logging_stubs(debug_enabled=False) + try: + logger = importlib.import_module('functions_appinsights') + forwarded = logger._build_logger_extra(route_log.call_args.args[0], properties) + self.assertEqual(forwarded['sc_resource'], resource) + self.assertEqual(forwarded['sc_reason'], 'invalid_retrieval_flag') + self.assertEqual(forwarded['sc_attempt'], 2) + finally: + _restore_modules(saved_modules) + def test_action_followup_keeps_resolved_context_and_all_model_usage(self): + self.use_modern_models() self.settings.update({ 'enable_semantic_kernel': True, 'enable_chat_orchestration_actions': True, @@ -391,6 +755,12 @@ async def invoke_action(action_ref, task, context, **kwargs): self.assertEqual(context.user_message, LATEST) self.assertEqual(context.resolved_message, RESOLVED) self.assertEqual(context.context_message_ids, ['u1', 'u2', 'a2']) + self.assertEqual(context.gpt_model, 'gpt-5.6-terra') + self.assertEqual(context.model_context, { + 'model_id': 'terra-model', 'endpoint_id': 'selected-endpoint', 'provider': 'aoai', + 'model_deployment': 'gpt-5.6-terra', 'user_id': 'user1', 'active_group_ids': [], + }) + self.assertEqual(context.planner_deployment, 'gpt-5.6-terra') updated = self.runs.read_item(plan['run_id'], 'conv1') self.assertEqual(updated['status'], 'completed') self.assertEqual(updated['token_usage'], { @@ -691,5 +1061,5 @@ def test_ledger_does_not_reintroduce_masked_turn_context(self): if __name__ == '__main__': - assert_app_version_at_least('0.261.096') + assert_app_version_at_least('0.261.101') unittest.main() diff --git a/functional_tests/test_orchestration_model_selection.py b/functional_tests/test_orchestration_model_selection.py new file mode 100644 index 000000000..eb133c49f --- /dev/null +++ b/functional_tests/test_orchestration_model_selection.py @@ -0,0 +1,440 @@ +# test_orchestration_model_selection.py +""" +Functional regressions for authorized orchestration model selection and SDK parameters. +Version: 0.261.101 +Implemented in: 0.261.101 + +Exercises the real selection/binding code with endpoint authorization and client creation +replaced at their existing boundaries. No Azure resources or credentials are used. +""" + +import importlib +import json +import sys +import unittest +from types import SimpleNamespace +from unittest.mock import Mock, patch + +from test_orchestration_conversation_context import ( + LATEST, RESOLVED, fake_module, load_modules, winery_history, +) +from test_support.app_stubs import stubbed_config +from test_support.versioning import assert_app_version_at_least + + +TERRA_SELECTION = { + 'model_deployment': 'gpt-5.6-terra', 'model_provider': 'aoai', + 'model_endpoint_id': 'selected-endpoint', 'model_id': 'terra-model', +} + + +def model_endpoint(): + return { + 'id': 'selected-endpoint', 'provider': 'aoai', 'enabled': True, + 'connection': { + 'endpoint': 'https://selected.example.test', + 'openai_api_version': '2025-04-01-preview', + }, + 'auth': {'type': 'api_key', 'api_key': 'test-only-key'}, + 'models': [ + { + 'id': 'terra-model', 'deploymentName': 'gpt-5.6-terra', + 'modelName': 'gpt-5.6-terra', 'enabled': True, 'responseLength': 16000, + }, + { + 'id': 'luna-model', 'deploymentName': 'gpt-5.6-luna', + 'modelName': 'gpt-5.6-luna', 'enabled': True, + }, + ], + } + + +def endpoint_runtime(endpoint, client): + return fake_module( + 'functions_model_endpoint_runtime', + MODEL_ENDPOINT_PROVIDER_ALLOWLIST={'aoai', 'aifoundry', 'new_foundry', 'anthropic', 'claude'}, + resolve_model_endpoint_from_context=Mock(return_value=endpoint), + build_model_endpoint_sync_chat_client=Mock(return_value=(client, 'azure_openai')), + ) + + +class ModelSelectionTests(unittest.TestCase): + @classmethod + def setUpClass(cls): + cls.modules = load_modules() + with stubbed_config(): + cls.models = importlib.import_module('functions_orchestration_models') + + def setUp(self): + self.endpoint = model_endpoint() + self.client = Mock() + self.legacy_client = Mock() + self.runtime = endpoint_runtime(self.endpoint, self.client) + self.settings = { + 'enable_multi_model_endpoints': True, + 'default_model_selection': { + 'endpoint_id': 'selected-endpoint', 'model_id': 'terra-model', 'provider': 'aoai', + }, + 'gpt_model': {'selected': [{'deploymentName': 'gpt-4o'}]}, + } + self.identity = {'user_id': 'user1', 'user_roles': ['User'], 'user_email': 'test@example.test'} + patcher = patch.dict(sys.modules, {'functions_model_endpoint_runtime': self.runtime}) + patcher.start() + self.addCleanup(patcher.stop) + patcher = patch.object( + self.modules.planner, 'resolve_planner_client', + side_effect=lambda settings: ( + self.legacy_client, settings.get('chat_orchestration_planner_deployment') or 'gpt-4o' + ), + ) + self.legacy_resolver = patcher.start() + self.addCleanup(patcher.stop) + + def resolve(self, selection=None, **kwargs): + seeds = {'model': selection or {}, 'reasoning_effort': 'high', 'active_group_ids': ['group1']} + binding = self.models.resolve_orchestration_model( + self.settings, user_id='user1', seeds=seeds, identity_context=self.identity, **kwargs, + ) + self.addCleanup(binding.close) + return binding + + def test_explicit_selection_beats_default_and_legacy_with_authorized_identity(self): + self.settings['default_model_selection']['model_id'] = 'luna-model' + binding = self.resolve(TERRA_SELECTION) + self.assertIs(binding.client, self.client) + self.assertEqual(binding.deployment, 'gpt-5.6-terra') + self.assertEqual(binding.source, 'request') + self.assertEqual(binding.answer_model_selection(), TERRA_SELECTION) + self.runtime.resolve_model_endpoint_from_context.assert_called_once_with( + self.settings, { + 'endpoint_id': 'selected-endpoint', 'model_id': 'terra-model', + 'model_deployment': 'gpt-5.6-terra', 'provider': 'aoai', + 'user_id': 'user1', 'active_group_ids': ['group1'], + }, authorize=True, + ) + self.runtime.build_model_endpoint_sync_chat_client.assert_called_once_with( + self.endpoint['auth'], 'aoai', 'https://selected.example.test', + '2025-04-01-preview', 'gpt-5.6-terra', settings=self.settings, + endpoint_config=self.endpoint, identity_context=self.identity, + ) + self.legacy_resolver.assert_not_called() + self.assertNotIn('test-only-key', str(binding.metadata())) + self.assertNotIn('selected.example.test', str(binding.metadata())) + + def test_admin_default_wins_when_no_model_is_supplied(self): + binding = self.resolve() + self.assertEqual(binding.deployment, 'gpt-5.6-terra') + self.assertEqual(binding.source, 'default') + self.assertEqual(binding.answer_model_selection(), TERRA_SELECTION) + self.legacy_resolver.assert_not_called() + + def test_planner_override_is_separate_from_persisted_answer_selection(self): + self.settings['chat_orchestration_planner_deployment'] = 'small-planner' + planner = self.resolve(TERRA_SELECTION, planner=True) + answer = self.resolve(planner.answer_model_selection()) + self.assertEqual(planner.deployment, 'small-planner') + self.assertEqual(planner.source, 'planner_override') + self.assertEqual(planner.answer_model_selection(), TERRA_SELECTION) + self.assertIs(planner.client, self.legacy_client) + self.assertEqual(answer.deployment, 'gpt-5.6-terra') + self.assertIs(answer.client, self.client) + self.assertEqual(self.runtime.resolve_model_endpoint_from_context.call_count, 2) + self.assertEqual(self.runtime.build_model_endpoint_sync_chat_client.call_count, 1) + + def test_unavailable_modern_model_never_falls_back_even_with_planner_override(self): + self.runtime.resolve_model_endpoint_from_context.return_value = None + for override in ('', 'small-planner'): + self.settings['chat_orchestration_planner_deployment'] = override + for selection in (None, TERRA_SELECTION): + with self.subTest(override=override, selection=selection): + with self.assertRaises(self.models.OrchestrationModelError): + self.resolve(selection, planner=True) + self.legacy_resolver.assert_not_called() + self.runtime.build_model_endpoint_sync_chat_client.assert_not_called() + + def test_configured_planner_endpoint_is_authorized_independently_of_the_answer(self): + planner_endpoint = model_endpoint() + planner_endpoint.update(id='planner-endpoint', provider='new_foundry') + planner_endpoint['connection'] = {'endpoint': 'https://planner.example.test/openai/v1'} + planner_endpoint['models'] = [{ + 'id': 'planner-model', 'deploymentName': 'gpt-4o-mini', 'enabled': True, + }] + self.settings.update({ + 'chat_orchestration_planner_model_endpoint_id': 'planner-endpoint', + 'chat_orchestration_planner_model_id': 'planner-model', + 'chat_orchestration_planner_model_provider': 'new_foundry', + }) + self.runtime.resolve_model_endpoint_from_context.side_effect = [ + self.endpoint, planner_endpoint, + ] + binding = self.resolve(TERRA_SELECTION, planner=True) + self.assertEqual(binding.deployment, 'gpt-4o-mini') + self.assertEqual(binding.provider, 'new_foundry') + self.assertEqual(binding.endpoint_id, 'planner-endpoint') + self.assertEqual(binding.answer_model_selection(), TERRA_SELECTION) + self.assertEqual(binding.reasoning_effort, '') + self.assertTrue(self.models.has_planner_model_override(self.settings)) + self.assertEqual(self.runtime.resolve_model_endpoint_from_context.call_count, 2) + for call in self.runtime.resolve_model_endpoint_from_context.call_args_list: + self.assertTrue(call.kwargs['authorize']) + self.assertEqual(call.args[1]['user_id'], 'user1') + self.assertEqual(self.runtime.build_model_endpoint_sync_chat_client.call_args.args[1:5], ( + 'new_foundry', 'https://planner.example.test/openai/v1', '', 'gpt-4o-mini', + )) + self.legacy_resolver.assert_not_called() + + def test_partial_planner_identity_is_rejected_instead_of_using_the_legacy_connection(self): + for override in ( + {'chat_orchestration_planner_model_id': 'planner-model'}, + {'chat_orchestration_planner_model_provider': 'aoai'}, + {'chat_orchestration_planner_deployment': 'claude-sonnet', + 'chat_orchestration_planner_model_provider': 'anthropic'}, + ): + with self.subTest(override=override): + with patch.dict(self.settings, override): + with self.assertRaises(self.models.OrchestrationModelError): + self.resolve(planner=True) + self.legacy_resolver.assert_not_called() + self.runtime.build_model_endpoint_sync_chat_client.assert_not_called() + + def test_endpoint_authorization_failure_is_not_replaced_with_a_default(self): + self.runtime.resolve_model_endpoint_from_context.side_effect = PermissionError('Test denial') + with self.assertRaises(PermissionError): + self.resolve(TERRA_SELECTION) + self.legacy_resolver.assert_not_called() + self.runtime.build_model_endpoint_sync_chat_client.assert_not_called() + + def test_incomplete_or_inconsistent_selection_is_rejected(self): + for selection in ( + {'model_id': 'terra-model'}, + {'model_endpoint_id': 'selected-endpoint'}, + {'model_provider': 'aoai'}, + {**TERRA_SELECTION, 'model_endpoint_id': 'other-endpoint'}, + {**TERRA_SELECTION, 'model_id': 'luna-model'}, + {**TERRA_SELECTION, 'model_deployment': 'gpt-4o'}, + {**TERRA_SELECTION, 'model_provider': 'new_foundry'}, + ): + with self.subTest(selection=selection): + with self.assertRaises(self.models.OrchestrationModelError): + self.resolve(selection) + self.legacy_resolver.assert_not_called() + self.runtime.build_model_endpoint_sync_chat_client.assert_not_called() + + def test_disabled_or_misconfigured_endpoint_and_model_are_rejected(self): + for change in ( + lambda endpoint: endpoint.update(enabled=False), + lambda endpoint: endpoint['models'][0].update(enabled=False), + lambda endpoint: endpoint.update(models=[]), + lambda endpoint: endpoint['connection'].update(endpoint=''), + lambda endpoint: endpoint['connection'].update(openai_api_version=''), + ): + with self.subTest(change=change): + endpoint = model_endpoint() + change(endpoint) + self.runtime.resolve_model_endpoint_from_context.return_value = endpoint + with self.assertRaises(self.models.OrchestrationModelError): + self.resolve(TERRA_SELECTION) + self.legacy_resolver.assert_not_called() + self.runtime.build_model_endpoint_sync_chat_client.assert_not_called() + + def test_stale_defaults_are_rejected_without_a_legacy_fallback(self): + for default in ( + {'endpoint_id': 'selected-endpoint'}, + {'model_id': 'terra-model'}, + {'endpoint_id': ' ', 'model_id': 'terra-model'}, + {'endpoint_id': 'selected-endpoint', 'model_id': ' '}, + {'endpoint_id': 'selected-endpoint', 'model_id': 'deleted-model'}, + ): + with self.subTest(default=default): + self.settings['default_model_selection'] = default + with self.assertRaises(self.models.OrchestrationModelError): + self.resolve() + self.legacy_resolver.assert_not_called() + self.runtime.build_model_endpoint_sync_chat_client.assert_not_called() + + def test_modern_selection_requires_enabled_runtime_and_a_captured_user(self): + self.settings['enable_multi_model_endpoints'] = False + with self.assertRaises(self.models.OrchestrationModelError): + self.resolve(TERRA_SELECTION) + self.settings['enable_multi_model_endpoints'] = True + with self.assertRaises(self.models.OrchestrationModelError): + self.models.resolve_orchestration_model( + self.settings, user_id='', seeds={'model': TERRA_SELECTION}, + ) + self.legacy_resolver.assert_not_called() + self.runtime.resolve_model_endpoint_from_context.assert_not_called() + + def test_legacy_models_and_apim_keep_their_configured_connection(self): + self.settings['enable_multi_model_endpoints'] = False + self.settings['chat_orchestration_planner_deployment'] = 'small-planner' + for apim in (False, True): + with self.subTest(apim=apim): + self.settings['enable_gpt_apim'] = apim + self.settings['azure_apim_gpt_deployment'] = 'gpt-4o, gpt-5' + self.settings['gpt_model']['selected'].append({'deploymentName': 'gpt-5'}) + answer = self.resolve({'model_deployment': 'gpt-5'}) + self.assertIs(answer.client, self.legacy_client) + self.assertEqual(answer.deployment, 'gpt-5') + self.assertEqual( + self.legacy_resolver.call_args.args[0]['chat_orchestration_planner_deployment'], '', + ) + planner = self.resolve({'model_deployment': 'gpt-5'}, planner=True) + self.assertEqual(planner.deployment, 'small-planner') + self.assertEqual(planner.answer_model_selection()['model_deployment'], 'gpt-5') + with self.assertRaises(self.models.OrchestrationModelError): + self.resolve({'model_deployment': 'not-configured'}) + self.runtime.resolve_model_endpoint_from_context.assert_not_called() + + def test_reasoning_planning_uses_bounded_budget_and_compatible_parameters(self): + binding = self.resolve(TERRA_SELECTION) + messages = [{'role': 'user', 'content': 'Return a JSON object.'}] + binding.as_planner_client().chat.completions.create( + model=binding.deployment, messages=messages, max_tokens=1200, + temperature=0, response_format={'type': 'json_object'}, + ) + parameters = self.client.chat.completions.create.call_args.kwargs + self.assertEqual(parameters['model'], 'gpt-5.6-terra') + self.assertEqual(parameters['messages'], messages) + self.assertEqual(parameters['max_completion_tokens'], 8192) + self.assertEqual(parameters['reasoning_effort'], 'high') + self.assertEqual(parameters['response_format'], {'type': 'json_object'}) + self.assertNotIn('max_tokens', parameters) + self.assertNotIn('temperature', parameters) + + def test_foundry_and_anthropic_use_their_selected_protocol_without_an_azure_api_version(self): + for provider, address, deployment in ( + ('new_foundry', 'https://selected.example.test/openai/v1', 'gpt-4o-mini'), + ('anthropic', 'https://selected.example.test/anthropic/v1/messages', 'claude-sonnet'), + ): + with self.subTest(provider=provider): + self.endpoint['provider'] = provider + self.endpoint['connection'] = {'endpoint': address} + self.endpoint['models'][0].update(deploymentName=deployment, modelName=deployment) + binding = self.resolve({ + **TERRA_SELECTION, 'model_provider': provider, 'model_deployment': deployment, + }) + self.assertEqual( + self.runtime.build_model_endpoint_sync_chat_client.call_args.args[1:5], + (provider, address, '', deployment), + ) + binding.create_completion(messages=[], max_tokens=1200, temperature=0) + parameters = self.client.chat.completions.create.call_args.kwargs + self.assertEqual(parameters['model'], deployment) + self.assertEqual(parameters['max_tokens'], 1200) + self.assertNotIn('reasoning_effort', parameters) + self.assertNotIn('max_completion_tokens', parameters) + self.legacy_resolver.assert_not_called() + + def claude_resolver(self): + endpoint_clients = importlib.import_module('model_endpoint_clients') + self.endpoint['provider'] = 'anthropic' + self.endpoint['connection'] = {'endpoint': 'https://selected.example.test/anthropic/v1/messages'} + self.endpoint['models'][0].update(deploymentName='claude-sonnet-4', modelName='claude-sonnet-4') + client = endpoint_clients.AnthropicChatCompletionClient( + endpoint=self.endpoint['connection']['endpoint'], api_key='test-only-key', + ) + self.runtime.build_model_endpoint_sync_chat_client.return_value = client, 'anthropic' + binding = self.resolve({ + **TERRA_SELECTION, 'model_provider': 'anthropic', 'model_deployment': 'claude-sonnet-4', + }) + snapshot = self.modules.context.build_conversation_snapshot(winery_history()) + response = Mock(status_code=200) + payload = { + 'content': [{'type': 'text', 'text': json.dumps({ + 'relationship': 'follow_up', 'resolved_message': RESOLVED, + 'message_ids': ['u1', 'u2', 'a2'], 'requires_retrieval': True, 'clarification': None, + })}], + 'usage': {'input_tokens': 10, 'output_tokens': 5}, + } + response.json.return_value = payload + return binding, snapshot, response, payload + + def test_anthropic_completed_followup_preserves_context_without_json_repair(self): + binding, snapshot, response, payload = self.claude_resolver() + for reason in ('end_turn', 'stop_sequence'): + with self.subTest(reason=reason): + payload['stop_reason'] = reason + with patch('model_endpoint_clients.requests.post', return_value=response) as post: + resolution = self.modules.planner.resolve_conversation_request( + LATEST, snapshot, settings=self.settings, planner_model=binding, + ) + self.assertEqual(resolution['resolved_message'], RESOLVED) + self.assertEqual(resolution['clarification'], '') + self.assertEqual(resolution['token_usage']['total_tokens'], 15) + post.assert_called_once() + self.assertEqual(post.call_args.kwargs['json']['model'], 'claude-sonnet-4') + self.assertEqual( + post.call_args.kwargs['json']['max_tokens'], self.modules.planner.RESOLUTION_MAX_TOKENS, + ) + + def test_anthropic_truncation_refusal_and_incomplete_turns_are_not_repaired(self): + binding, snapshot, response, payload = self.claude_resolver() + for reason, expected in ( + ('max_tokens', 'incomplete_completion'), + ('model_context_window_exceeded', 'incomplete_completion'), + ('tool_use', 'incomplete_completion'), + ('pause_turn', 'incomplete_completion'), + ('refusal', 'model_refusal'), + ): + with self.subTest(reason=reason): + payload['stop_reason'] = reason + with patch('model_endpoint_clients.requests.post', return_value=response) as post: + with self.assertRaises(self.modules.planner.ConversationResolutionError) as raised: + self.modules.planner.resolve_conversation_request( + LATEST, snapshot, settings=self.settings, planner_model=binding, + ) + self.assertEqual(raised.exception.reason, expected) + self.assertEqual(raised.exception.attempts, 1) + post.assert_called_once() + + def test_answer_respects_configured_limit_and_underlying_reasoning_model_alias(self): + self.endpoint['models'][0].update(deploymentName='production-answer', responseLength=2048) + binding = self.resolve({**TERRA_SELECTION, 'model_deployment': 'production-answer'}) + binding.create_completion( + messages=[], max_tokens=4000, temperature=0.3, use_model_response_length=True, + ) + parameters = self.client.chat.completions.create.call_args.kwargs + self.assertEqual(parameters['model'], 'production-answer') + self.assertEqual(parameters['max_completion_tokens'], 2048) + self.assertNotIn('temperature', parameters) + + def test_invalid_response_lengths_use_the_bounded_answer_budget(self): + for value in (None, False, 0, -1, 'invalid'): + with self.subTest(value=value): + self.endpoint['models'][0]['responseLength'] = value + binding = self.resolve(TERRA_SELECTION) + binding.create_completion(messages=[], max_tokens=4000, use_model_response_length=True) + self.assertEqual( + self.client.chat.completions.create.call_args.kwargs['max_completion_tokens'], 8192, + ) + + def test_non_reasoning_models_keep_temperature_and_legacy_token_parameter(self): + binding = self.models.OrchestrationModel(self.client, 'gpt-4o', reasoning_effort='high') + binding.create_completion(messages=[], max_tokens=1200, temperature=0.3) + self.assertEqual(self.client.chat.completions.create.call_args.kwargs, { + 'model': 'gpt-4o', 'messages': [], 'max_tokens': 1200, 'temperature': 0.3, + }) + + def test_binding_cannot_be_retargeted_and_closes_its_sdk_client_once(self): + binding = self.resolve(TERRA_SELECTION) + with self.assertRaises(self.models.OrchestrationModelError): + binding.create_completion(model='gpt-4o', messages=[]) + self.client.chat.completions.create.assert_not_called() + binding.close() + binding.close() + self.client.close.assert_called_once_with() + + def test_research_uses_the_captured_planner_instead_of_resolving_a_legacy_client(self): + binding = self.resolve(TERRA_SELECTION) + context = SimpleNamespace( + planner_client=binding.as_planner_client(), planner_deployment=binding.deployment, + ) + client, deployment = self.modules.adapters._resolve_source_review_planner(self.settings, context) + self.assertIs(client, context.planner_client) + self.assertEqual(deployment, 'gpt-5.6-terra') + self.legacy_resolver.assert_not_called() + + +if __name__ == '__main__': + assert_app_version_at_least('0.261.101') + unittest.main() diff --git a/ui_tests/test_v2_orchestration_conversation_context.py b/ui_tests/test_v2_orchestration_conversation_context.py index 7be94b6f9..d34b086e3 100644 --- a/ui_tests/test_v2_orchestration_conversation_context.py +++ b/ui_tests/test_v2_orchestration_conversation_context.py @@ -1,8 +1,9 @@ # test_v2_orchestration_conversation_context.py """ Browser regressions for orchestration follow-up and clarification transport. -Version: 0.261.096 +Version: 0.261.101 Implemented in: 0.261.096 +Model selection transport and live answer attribution: 0.261.101 Runs the shipped controller, stores, elicitation card, and approval card in the existing local Playwright harness. HTTP/SSE is stubbed here; the companion @@ -83,13 +84,19 @@ return stream({ done: true, message_id: 'answer-1', conversation_id: conversationId, full_content: 'The request concerns wineries near Grants Pass.', + model_deployment_name: 'gpt-5.6-terra', }); } return new Response('{}', { status: 200, headers: { 'Content-Type': 'application/json' } }); }; await H.controller.startOrchestrationPlan({ conversationId, message: 'Which are open on Wednesdays?', approvalMode: mode, - seeds: { selected_document_ids: ['hours-document'] }, + seeds: { + selected_document_ids: ['hours-document'], + model_deployment: 'gpt-5.6-terra', model_provider: 'aoai', + model_endpoint_id: 'selected-endpoint', model_id: 'terra-model', + reasoning_effort: 'high', + }, }); const turnId = window.contextPlanCalls[0].turn_id; H.mount('mount-a', ask ? 'ElicitationCard' : 'OrchestrationPlanCard', { conversationId, turnId }); @@ -136,6 +143,8 @@ def test_accept_sends_matching_question_answer_and_stable_turn(self): self.assertEqual(second['turn_id'], turn_id) self.assertEqual(second['revision'], 1) self.assertEqual(second['selected_document_ids'], ['hours-document']) + for key in ('model_deployment', 'model_provider', 'model_endpoint_id', 'model_id', 'reasoning_effort'): + self.assertEqual(second[key], first[key]) self.assertEqual(second['elicitation']['elicitation_id'], 'question-1') self.assertEqual(second['elicitation']['turn_id'], turn_id) self.assertEqual(second['elicitation_response'], { @@ -178,12 +187,19 @@ def test_all_approval_modes_run_the_server_plan_without_uploading_history(self): plans, runs = self.page.evaluate('() => [window.contextPlanCalls, window.contextRunCalls]') self.assertEqual(len(plans), 1) self.assertEqual(plans[0]['message'], 'Which are open on Wednesdays?') + self.assertEqual(plans[0]['model_deployment'], 'gpt-5.6-terra') + self.assertEqual(plans[0]['model_endpoint_id'], 'selected-endpoint') + self.assertEqual(plans[0]['model_id'], 'terra-model') self.assertNotIn('recent_messages', plans[0]) self.assertEqual(len(runs), 1) self.assertEqual(runs[0]['conversation_id'], conversation_id) self.assertEqual(runs[0]['run_id'], f'run-{turn_id}') self.assertNotIn('message', runs[0]) self.assertNotIn('recent_messages', runs[0]) + self.page.evaluate("() => window.OrchHarness.mount('mount-b', 'MessageList')") + model_label = self.page.get_by_text('gpt-5.6-terra', exact=True) + model_label.wait_for(state='visible') + self.assertTrue(model_label.is_visible()) def test_navigation_does_not_retarget_a_pending_plan(self): conversation_id, turn_id = self.start(ask=False) @@ -201,5 +217,5 @@ def test_navigation_does_not_retarget_a_pending_plan(self): if __name__ == '__main__': - assert_app_version_at_least('0.261.096') + assert_app_version_at_least('0.261.101') unittest.main()