diff --git a/src/poly/handlers/interface.py b/src/poly/handlers/interface.py index 5bb2b757..a8121ad5 100644 --- a/src/poly/handlers/interface.py +++ b/src/poly/handlers/interface.py @@ -12,7 +12,6 @@ from poly.handlers.platform_api import PlatformAPIHandler from poly.handlers.posthog import PosthogHandler from poly.handlers.protobuf.commands_pb2 import Command -from poly.handlers.protobuf.handoff_pb2 import Handoff_SetDefault from poly.handlers.sdk import SourcererAPIError from poly.handlers.sync_client import SyncClientHandler from poly.resources import ( @@ -486,19 +485,6 @@ def queue_resources( ) ) - # is_default is not part of create/update protos; it requires a separate command - for resource_dict in [new_resources, updated_resources]: - for resource in resource_dict.get(Handoff, {}).values(): - if isinstance(resource, Handoff) and resource.is_default: - commands.append( - Command( - type="handoff_set_default", - command_id=str(uuid.uuid4()), - metadata=metadata, - handoff_set_default=Handoff_SetDefault(id=resource.resource_id), - ) - ) - for command in commands: self.sync_client.sdk.add_command_to_queue(command) diff --git a/src/poly/project.py b/src/poly/project.py index 47e7a508..85fd2b80 100644 --- a/src/poly/project.py +++ b/src/poly/project.py @@ -51,6 +51,7 @@ load_resources_from_projection, ) from poly.utils import prepush +from poly.utils.commands import queue_set_default_commands logger = logging.getLogger(__name__) @@ -1172,6 +1173,13 @@ def _stage_commands( ) ) + queue_set_default_commands( + new_resources, + updated_resources, + commands, + queue_command=lambda command: self.api_handler.queue_command(command), + ) + return commands def push_project( @@ -1466,8 +1474,6 @@ def _clean_resources_before_push( post push: delete dummy ) - Only update the default variant if it's being enabled. - If a function is new or updated and it references a variable, update the variable references. Args: @@ -1519,7 +1525,6 @@ def _clean_resources_before_push( prepush.default_new_variant_attributes( new_resources, deleted_resources, current_resources=self.resources ) - prepush.filter_nondefault_variant_updates(updated_resources) prepush.fix_conditions_for_deleted_steps( new_resources, updated_resources, diff --git a/src/poly/resources/variant_attributes.py b/src/poly/resources/variant_attributes.py index a67cfdb3..c8b9269c 100644 --- a/src/poly/resources/variant_attributes.py +++ b/src/poly/resources/variant_attributes.py @@ -14,8 +14,8 @@ Variant_CreateVariant, Variant_DeleteAttribute, Variant_DeleteVariant, - Variant_SetDefaultVariant, Variant_UpdateAttribute, + Variant_UpdateVariant, VariantValues, ) from poly.resources.resource import MultiResourceYamlResource, ResourceMapping, register_resource @@ -93,11 +93,12 @@ def create_command_type(self) -> str: @property def update_command_type(self) -> str: - return "variant_set_default_variant" + return "variant_update_variant" - def build_update_proto(self): - return Variant_SetDefaultVariant( + def build_update_proto(self) -> Variant_UpdateVariant: + return Variant_UpdateVariant( id=self.resource_id, + name=self.name, ) def build_delete_proto(self) -> Variant_DeleteVariant: diff --git a/src/poly/tests/project_test.py b/src/poly/tests/project_test.py index b2f79067..9d6bcbce 100644 --- a/src/poly/tests/project_test.py +++ b/src/poly/tests/project_test.py @@ -15,6 +15,8 @@ from unittest.mock import MagicMock, patch import poly.resources.resource_utils as resource_utils +from poly.handlers.interface import AgentStudioInterface +from poly.handlers.protobuf.commands_pb2 import Command from poly.project import AgentStudioProject, DeploymentMode from poly.resources import ( AsrSettings, @@ -28,6 +30,7 @@ FlowStep, Function, FunctionStep, + Handoff, KeyphraseBoosting, Pronunciation, Resource, @@ -2039,6 +2042,24 @@ def test_new_variant_excludes_deleted_attribute_ids(self): self.assertEqual(new_variant.attribute_ids, ["VARIANT_ATTRIBUTES-keep"]) + def test_non_default_variant_update_is_kept(self): + """A renamed non-default variant must still be pushed as an update.""" + renamed_variant = Variant( + resource_id="VARIANTS-production", + name="HME_Specialists - Inbound Call Campaign", + is_default=False, + ) + updated_resources = {Variant: {"VARIANTS-production": renamed_variant}} + + push_changes = self.project._clean_resources_before_push( + {}, + {}, + updated_resources, + {}, + ) + + self.assertEqual(push_changes.main.updated[Variant], {"VARIANTS-production": renamed_variant}) + class PushProjectTest(unittest.TestCase): """Tests for the push_project method""" @@ -2476,6 +2497,143 @@ def test_push_project_dry_run(self): self.mock_api_handler.clear_command_queue.assert_called_once() +class StageSetDefaultCommandsTest(unittest.TestCase): + """Tests for the set-default commands _stage_commands emits for handoffs and variants.""" + + def setUp(self): + """Give the project an api_handler whose only mocked part is the network.""" + # The real interface still builds real Command protos out of the staged resources, + # so the assertions below reflect what the platform would actually be sent. + self.api_handler = AgentStudioInterface() + self.api_handler.sync_client = MagicMock() + self.api_handler.sync_client.get_queued_commands.return_value = [] + self.api_handler.sync_client.sdk.create_metadata.return_value = Command().metadata + # queue_command is what puts a standalone command in the real send queue + self.queued_commands = [] + self.api_handler.sync_client.queue_command.side_effect = self.queued_commands.append + + # Reading the api_handler property saves the project config as a side effect, + # which would write _gen/.agent_studio_config into the fixture project + patch.object(AgentStudioProject, "save_config").start() + self.project = AgentStudioProject.from_dict(PROJECT_DATA, TEST_DIR) + self.project._api_handler = self.api_handler + # Start from empty current state so the fixture project's own resources don't add + # unrelated commands (e.g. prepush's orphaned-variable reference updates) + self.project.resources = {} + + def tearDown(self): + patch.stopall() + + def stage(self, new_resources=None, updated_resources=None) -> list: + """Stage commands for the given new and updated resources.""" + return self.project._stage_commands( + {}, + new_resources or {}, + updated_resources or {}, + {}, + ) + + def test_new_default_variant_is_created_and_then_set_as_default(self): + """A brand new default variant is created first, then explicitly made default.""" + new_default = Variant(resource_id="VARIANT-new", name="production", is_default=True) + + commands = self.stage(new_resources={Variant: {"VARIANT-new": new_default}}) + + types = [command.type for command in commands] + self.assertEqual(types, ["variant_create_variant", "variant_set_default_variant"]) + self.assertEqual(commands[-1].variant_set_default_variant.id, "VARIANT-new") + + def test_switching_default_variant_sets_only_the_new_default(self): + """Renaming both variants updates both, but only the new default is set as default.""" + old_default = Variant(resource_id="VARIANT-a", name="variant a", is_default=False) + new_default = Variant(resource_id="VARIANT-b", name="variant b", is_default=True) + + commands = self.stage( + updated_resources={Variant: {"VARIANT-a": old_default, "VARIANT-b": new_default}} + ) + + types = [command.type for command in commands] + self.assertEqual(types.count("variant_update_variant"), 2) + set_defaults = [c for c in commands if c.type == "variant_set_default_variant"] + self.assertEqual([c.variant_set_default_variant.id for c in set_defaults], ["VARIANT-b"]) + + def test_non_default_variant_update_produces_no_set_default(self): + """Updating a non-default variant never promotes it to default.""" + renamed = Variant(resource_id="VARIANT-a", name="variant a renamed", is_default=False) + + commands = self.stage(updated_resources={Variant: {"VARIANT-a": renamed}}) + + types = [command.type for command in commands] + self.assertEqual(types, ["variant_update_variant"]) + self.assertEqual(commands[0].variant_update_variant.name, "variant a renamed") + + def test_new_default_handoff_is_created_and_then_set_as_default(self): + """A brand new default handoff is created first, then explicitly made default.""" + new_default = Handoff(resource_id="HANDOFF-new", name="escalate", is_default=True) + + commands = self.stage(new_resources={Handoff: {"HANDOFF-new": new_default}}) + + types = [command.type for command in commands] + self.assertEqual(types, ["handoff_create", "handoff_set_default"]) + self.assertEqual(commands[-1].handoff_set_default.id, "HANDOFF-new") + + def test_switching_default_handoff_sets_only_the_new_default(self): + """Updating both handoffs updates both, but only the new default is set as default.""" + old_default = Handoff(resource_id="HANDOFF-a", name="escalate", is_default=False) + new_default = Handoff(resource_id="HANDOFF-b", name="voicemail", is_default=True) + + commands = self.stage( + updated_resources={Handoff: {"HANDOFF-a": old_default, "HANDOFF-b": new_default}} + ) + + types = [command.type for command in commands] + self.assertEqual(types.count("handoff_update"), 2) + set_defaults = [c for c in commands if c.type == "handoff_set_default"] + self.assertEqual([c.handoff_set_default.id for c in set_defaults], ["HANDOFF-b"]) + + def test_non_default_handoff_update_produces_no_set_default(self): + """Updating a non-default handoff never promotes it to default.""" + renamed = Handoff(resource_id="HANDOFF-a", name="escalate to agent", is_default=False) + + commands = self.stage(updated_resources={Handoff: {"HANDOFF-a": renamed}}) + + types = [command.type for command in commands] + self.assertEqual(types, ["handoff_update"]) + self.assertEqual(commands[0].handoff_update.name, "escalate to agent") + + def test_set_default_is_staged_after_every_create_and_update_command(self): + """Set-default comes last: the platform rejects it for a resource that does not exist yet.""" + new_variant = Variant(resource_id="VARIANT-new", name="production", is_default=True) + new_handoff = Handoff(resource_id="HANDOFF-new", name="escalate", is_default=True) + updated_entity = Entity(resource_id="ENTITY-a", name="postcode", entity_type="free_text") + + commands = self.stage( + new_resources={ + Variant: {"VARIANT-new": new_variant}, + Handoff: {"HANDOFF-new": new_handoff}, + }, + updated_resources={Entity: {"ENTITY-a": updated_entity}}, + ) + + types = [command.type for command in commands] + first_set_default = min( + types.index("handoff_set_default"), types.index("variant_set_default_variant") + ) + self.assertEqual(len(types) - 2, first_set_default) + self.assertLess(types.index("handoff_create"), first_set_default) + self.assertLess(types.index("variant_create_variant"), first_set_default) + self.assertLess(types.index("entity_update"), first_set_default) + + def test_set_default_command_is_handed_to_the_send_queue(self): + """The set-default command is queued, not just returned, so it is actually sent.""" + new_default = Variant(resource_id="VARIANT-new", name="production", is_default=True) + + self.stage(new_resources={Variant: {"VARIANT-new": new_default}}) + + self.assertEqual([c.type for c in self.queued_commands], ["variant_set_default_variant"]) + self.assertEqual(self.queued_commands[0].variant_set_default_variant.id, "VARIANT-new") + + class ValidateProjectTest(unittest.TestCase): """Tests for the validate_project method""" diff --git a/src/poly/tests/resources_test.py b/src/poly/tests/resources_test.py index 646f3885..fd9d10e8 100644 --- a/src/poly/tests/resources_test.py +++ b/src/poly/tests/resources_test.py @@ -12,6 +12,7 @@ from jsonschema import ValidationError import poly.resources.resource_utils as resource_utils +from poly.handlers.protobuf.variant_pb2 import Variant_UpdateVariant from poly.resources.agent_settings import ( SettingsPersona, SettingsRules, @@ -4603,6 +4604,34 @@ def test_validate_no_default_variant(self): str(cm.exception), ) + def test_update_command_type_is_variant_update_variant(self): + """The update command type doubles as the Command oneof kwarg, so it must stay exact.""" + self.assertEqual(TEST_VARIANT.update_command_type, "variant_update_variant") + + def test_build_update_proto_carries_id_and_name(self): + """An updated variant sends its new name so renames reach the platform.""" + renamed = Variant(resource_id="VARIANT-default", name="HME_Specialists - Inbound") + + proto = renamed.build_update_proto() + + self.assertIsInstance(proto, Variant_UpdateVariant) + self.assertEqual(proto.id, "VARIANT-default") + self.assertEqual(proto.name, "HME_Specialists - Inbound") + + def test_build_update_proto_leaves_attribute_values_unset(self): + """Regression guard: setting attribute_values would wipe the variant's stored values. + + The platform only rewrites a variant's attribute values when the field is present on + the wire, and a present map must cover every non-archived attribute or the command is + rejected. Variant updates must therefore never populate it. + """ + variant = Variant(resource_id="VARIANT-default", name="default") + variant.attribute_ids = ["attr-customer-name"] + + proto = variant.build_update_proto() + + self.assertFalse(proto.HasField("attribute_values")) + class VariantAttributeTests(unittest.TestCase): """Tests for VariantAttribute resource.""" diff --git a/src/poly/utils/commands.py b/src/poly/utils/commands.py index 1025846d..3397e041 100644 --- a/src/poly/utils/commands.py +++ b/src/poly/utils/commands.py @@ -3,6 +3,8 @@ Copyright PolyAI Limited """ +from typing import Callable, TypeAlias + from poly.handlers.protobuf.channels_pb2 import ( Channel_UpdateStatus, ChannelStatus, @@ -10,6 +12,11 @@ ) from poly.handlers.protobuf.commands_pb2 import Command from poly.handlers.protobuf.flows_pb2 import Flow_ClearStepSettings +from poly.handlers.protobuf.handoff_pb2 import Handoff_SetDefault +from poly.handlers.protobuf.variant_pb2 import Variant_SetDefaultVariant +from poly.resources import Handoff, Resource, Variant + +ResourceMap: TypeAlias = dict[type[Resource], dict[str, Resource]] def create_command_webchat_channel_update_status(enabled: bool) -> Command: @@ -38,3 +45,54 @@ def create_command_clear_flow_settings( sections=cleared_fields, ), ) + + +def create_command_handoff_set_default(handoff_id: str) -> Command: + """Create a command to make a handoff the default.""" + return Command( + type="handoff_set_default", + handoff_set_default=Handoff_SetDefault(id=handoff_id), + ) + + +def create_command_variant_set_default(variant_id: str) -> Command: + """Create a command to make a variant the default.""" + return Command( + type="variant_set_default_variant", + variant_set_default_variant=Variant_SetDefaultVariant(id=variant_id), + ) + + +# is_default is not part of any create or update proto, so a resource that can be +# "the default" needs a separate command once it exists on the platform. +SET_DEFAULT_COMMAND_BUILDERS = { + Handoff: create_command_handoff_set_default, + Variant: create_command_variant_set_default, +} + + +def queue_set_default_commands( + new_resources: ResourceMap, + updated_resources: ResourceMap, + commands: list[Command], + queue_command: Callable[..., None], +) -> None: + """Queue a set-default command for every new or updated default resource. + + is_default is not part of the create or update protos, so the platform needs a + separate command. Queue these after the creates and updates, so that the resource + already exists by the time the platform applies them. + + Args: + new_resources (ResourceMap): New resources being pushed. + updated_resources (ResourceMap): Updated resources being pushed. + commands (list[Command]): Command list to append to, kept in send order. + queue_command (Callable[..., None]): Callback that queues a single command. + """ + for resource_dict in (new_resources, updated_resources): + for resource_type, build_command in SET_DEFAULT_COMMAND_BUILDERS.items(): + for resource in resource_dict.get(resource_type, {}).values(): + if resource.is_default: + command = build_command(resource.resource_id) + queue_command(command) + commands.append(command) diff --git a/src/poly/utils/prepush.py b/src/poly/utils/prepush.py index b6f2ee5f..3f71b66e 100644 --- a/src/poly/utils/prepush.py +++ b/src/poly/utils/prepush.py @@ -340,15 +340,6 @@ def default_new_variant_attributes( variant.attribute_ids = attribute_ids -def filter_nondefault_variant_updates(updated_resources: ResourceMap) -> None: - """Drop updates for non-default variants (only the default variant is pushed).""" - # Only update the default variant if it's being enabled - updated_variants: list[Variant] = list(updated_resources.get(Variant, {}).values()) - for variant in updated_variants: - if not variant.is_default: - updated_resources[Variant].pop(variant.resource_id, None) - - def fix_conditions_for_deleted_steps( new_resources: ResourceMap, updated_resources: ResourceMap,