diff --git a/.cspell.json b/.cspell.json index 1d97d3c469d..b13c4312bc2 100644 --- a/.cspell.json +++ b/.cspell.json @@ -217,10 +217,11 @@ "dcid", "piezo", "Piezo", - "cmpop" + "cmpop", + "omap" ], "flagWords": [ "Prompt Flow" ], "allowCompoundWords": true -} \ No newline at end of file +} diff --git a/src/promptflow-core/promptflow/_utils/flow_utils.py b/src/promptflow-core/promptflow/_utils/flow_utils.py index 0962a675de8..58fb7839edc 100644 --- a/src/promptflow-core/promptflow/_utils/flow_utils.py +++ b/src/promptflow-core/promptflow/_utils/flow_utils.py @@ -21,7 +21,7 @@ ) from promptflow._core._errors import MetaFileNotFound, MetaFileReadError from promptflow._utils.logger_utils import LoggerFactory -from promptflow._utils.utils import strip_quotation +from promptflow._utils.utils import convert_ordered_dict_to_dict, strip_quotation from promptflow._utils.yaml_utils import dump_yaml, load_yaml from promptflow.contracts.flow import Flow as ExecutableFlow from promptflow.exceptions import ErrorTarget, UserErrorException, ValidationException @@ -157,7 +157,8 @@ def dump_flow_dag(flow_dag: dict, flow_path: Path): flow_dir, flow_filename = resolve_flow_path(flow_path, check_flow_exist=False) flow_path = flow_dir / flow_filename with open(flow_path, "w", encoding=DEFAULT_ENCODING) as f: - dump_yaml(flow_dag, f) + # directly dumping ordered dict will bring !!omap tag in yaml + dump_yaml(convert_ordered_dict_to_dict(flow_dag, remove_empty=False), f) return flow_path diff --git a/src/promptflow-core/promptflow/_utils/utils.py b/src/promptflow-core/promptflow/_utils/utils.py index 7af01b61774..26f52e3fabd 100644 --- a/src/promptflow-core/promptflow/_utils/utils.py +++ b/src/promptflow-core/promptflow/_utils/utils.py @@ -434,3 +434,48 @@ def strip_quotation(value): return value[1:-1] else: return value + + +def is_empty_target(obj: Optional[Dict]) -> bool: + """Determines if it's empty target + + :param obj: The object to check + :type obj: Optional[Dict] + :return: True if obj is None or an empty Dict + :rtype: bool + """ + return ( + obj is None + # some objs have overloaded "==" and will cause error. e.g CommandComponent obj + or (isinstance(obj, dict) and len(obj) == 0) + ) + + +def convert_ordered_dict_to_dict(target_object: Union[Dict, List], remove_empty: bool = True) -> Union[Dict, List]: + """Convert ordered dict to dict. Remove keys with None value. + This is a workaround for rest request must be in dict instead of + ordered dict. + + :param target_object: The object to convert + :type target_object: Union[Dict, List] + :param remove_empty: Whether to omit values that are None or empty dictionaries. Defaults to True. + :type remove_empty: bool + :return: Converted ordered dict with removed None values + :rtype: Union[Dict, List] + """ + # OrderedDict can appear nested in a list + if isinstance(target_object, list): + new_list = [] + for item in target_object: + item = convert_ordered_dict_to_dict(item, remove_empty=remove_empty) + if not is_empty_target(item) or not remove_empty: + new_list.append(item) + return new_list + if isinstance(target_object, dict): + new_dict = {} + for key, value in target_object.items(): + value = convert_ordered_dict_to_dict(value, remove_empty=remove_empty) + if not is_empty_target(value) or not remove_empty: + new_dict[key] = value + return new_dict + return target_object diff --git a/src/promptflow-devkit/tests/sdk_cli_test/e2etests/test_csharp_sdk.py b/src/promptflow-devkit/tests/sdk_cli_test/e2etests/test_csharp_sdk.py index 0d66d21bd2e..0261997a2df 100644 --- a/src/promptflow-devkit/tests/sdk_cli_test/e2etests/test_csharp_sdk.py +++ b/src/promptflow-devkit/tests/sdk_cli_test/e2etests/test_csharp_sdk.py @@ -31,7 +31,11 @@ class TestCSharpSdk: "language": {"default": "chinese", "type": "string"}, "topic": {"default": "ocean", "type": "string"}, }, - "outputs": {"output": {"type": "object"}}, + "outputs": { + "Answer": {"type": "string"}, + "AnswerLength": {"type": "int"}, + "PoemLanguage": {"type": "string"}, + }, }, id="function_mode_basic", ), @@ -39,7 +43,7 @@ class TestCSharpSdk: { "init": {"connection": {"type": "AzureOpenAIConnection"}, "name": {"type": "string"}}, "inputs": {"question": {"default": "What is Promptflow?", "type": "string"}}, - "outputs": {"output": {"type": "object"}}, + "outputs": {"output": {"type": "string"}}, }, id="class_init_flex_flow", ), diff --git a/src/promptflow-devkit/tests/sdk_cli_test/e2etests/test_flow_run.py b/src/promptflow-devkit/tests/sdk_cli_test/e2etests/test_flow_run.py index 53c77bc7311..3539d113d0f 100644 --- a/src/promptflow-devkit/tests/sdk_cli_test/e2etests/test_flow_run.py +++ b/src/promptflow-devkit/tests/sdk_cli_test/e2etests/test_flow_run.py @@ -1376,6 +1376,8 @@ def test_flex_flow_run( yaml_dict = load_yaml(local_storage._dag_path) assert yaml_dict == expected_snapshot_yaml + assert not local_storage._dag_path.read_text().startswith("!!omap") + # actual result will be entry2:my_flow2 details = pf.get_details(run.name) # convert DataFrame to dict diff --git a/src/promptflow-tools/promptflow/tools/common.py b/src/promptflow-tools/promptflow/tools/common.py index f957f55f6f6..1737c114328 100644 --- a/src/promptflow-tools/promptflow/tools/common.py +++ b/src/promptflow-tools/promptflow/tools/common.py @@ -222,7 +222,7 @@ def validate_tools(tools): def try_parse_name_and_content(role_prompt): # customer can add ## in front of name/content for markdown highlight. # and we still support name/content without ## prefix for backward compatibility. - pattern = r"\n*#{0,2}\s*name:\n+\s*(\S+)\s*\n*#{0,2}\s*content:\n?(.*)" + pattern = r"\n*#{0,2}\s*name\s*:\s*\n+\s*(\S+)\s*\n*#{0,2}\s*content\s*:\s*\n?(.*)" match = re.search(pattern, role_prompt, re.DOTALL) if match: return match.group(1), match.group(2) @@ -232,7 +232,7 @@ def try_parse_name_and_content(role_prompt): def try_parse_tool_call_id_and_content(role_prompt): # customer can add ## in front of tool_call_id/content for markdown highlight. # and we still support tool_call_id/content without ## prefix for backward compatibility. - pattern = r"\n*#{0,2}\s*tool_call_id:\n+\s*(\S+)\s*\n*#{0,2}\s*content:\n?(.*)" + pattern = r"\n*#{0,2}\s*tool_call_id\s*:\s*\n+\s*(\S+)\s*\n*#{0,2}\s*content\s*:\s*\n?(.*)" match = re.search(pattern, role_prompt, re.DOTALL) if match: return match.group(1), match.group(2) diff --git a/src/promptflow-tools/tests/test_common.py b/src/promptflow-tools/tests/test_common.py index 78f30790b0c..97373babdaa 100644 --- a/src/promptflow-tools/tests/test_common.py +++ b/src/promptflow-tools/tests/test_common.py @@ -214,7 +214,10 @@ def test_success_parse_role_prompt(self, chat_str, images, image_detail, expecte ("\nsystem:\nname:\n\n content:\nfirst", [ {'role': 'system', 'content': 'name:\n\n content:\nfirst'}]), ("\nsystem:\nname:\n\n", [ - {'role': 'system', 'content': 'name:'}]) + {'role': 'system', 'content': 'name:'}]), + # portal may add extra \r to new line character. + ("function:\r\nname:\r\n AI\ncontent :\r\nfirst", [ + {'role': 'function', 'name': 'AI', 'content': 'first'}]), ], ) def test_parse_chat_with_name_in_role_prompt(self, chat_str, expected_result): @@ -240,6 +243,20 @@ def test_try_parse_chat_with_tools(self, example_prompt_template_with_tool, pars actual_result = parse_chat(example_prompt_template_with_tool) assert actual_result == parsed_chat_with_tools + @pytest.mark.parametrize( + "chat_str, expected_result", + [ + ("\n#tool:\n## tool_call_id:\nid \n content:\nfirst\n\n#user:\nsecond", [ + {'role': 'tool', 'tool_call_id': 'id', 'content': 'first'}, {'role': 'user', 'content': 'second'}]), + # portal may add extra \r to new line character. + ("\ntool:\ntool_call_id :\r\nid\n content:\r\n", [ + {'role': 'tool', 'tool_call_id': 'id', 'content': ''}]), + ], + ) + def test_parse_tool_call_id_and_content(self, chat_str, expected_result): + actual_result = parse_chat(chat_str) + assert actual_result == expected_result + @pytest.mark.parametrize("chunk, error_msg, success", [ (""" ## tool_calls: @@ -275,6 +292,14 @@ def test_try_parse_chat_with_tools(self, example_prompt_template_with_tool, pars "function": {"name": "func1", "arguments": ""} }] """, "", True), + # portal may add extra \r to new line character. + (""" + ## tool_calls:\r + [{ + "id": "tool_call_id", "type": "function", + "function": {"name": "func1", "arguments": ""} + }] + """, "", True), ]) def test_parse_tool_calls_for_assistant(self, chunk: str, error_msg: str, success: bool): last_message = {'role': 'assistant'} diff --git a/src/promptflow-tools/tests/test_handle_openai_error.py b/src/promptflow-tools/tests/test_handle_openai_error.py index 686a2c7a8a2..cfad9281161 100644 --- a/src/promptflow-tools/tests/test_handle_openai_error.py +++ b/src/promptflow-tools/tests/test_handle_openai_error.py @@ -261,6 +261,7 @@ def test_input_invalid_function_role_prompt(self, azure_open_ai_connection): ) assert "'name' is required if role is function," in exc_info.value.message + @pytest.mark.skip(reason="Skip temporarily because there is something issue with test AOAI resource response.") def test_completion_with_chat_model(self, azure_open_ai_connection): with pytest.raises(UserErrorException) as exc_info: completion(connection=azure_open_ai_connection, prompt="hello", deployment_name="gpt-35-turbo") diff --git a/src/promptflow/tests/executor/unittests/batch/test_csharp_executor_proxy.py b/src/promptflow/tests/executor/unittests/batch/test_csharp_executor_proxy.py index dcc22683270..0e36f198ca0 100644 --- a/src/promptflow/tests/executor/unittests/batch/test_csharp_executor_proxy.py +++ b/src/promptflow/tests/executor/unittests/batch/test_csharp_executor_proxy.py @@ -1,4 +1,6 @@ import json +import platform +import signal import socket import subprocess from pathlib import Path @@ -62,7 +64,10 @@ async def test_destroy_with_terminates_gracefully(self): await executor_proxy.destroy() mock_process.poll.assert_called_once() - mock_process.terminate.assert_called_once() + if platform.system() != "Windows": + mock_process.terminate.assert_called_once() + else: + mock_process.send_signal.assert_called_once_with(signal.CTRL_BREAK_EVENT) mock_process.wait.assert_called_once_with(timeout=5) mock_process.kill.assert_not_called() @@ -77,7 +82,10 @@ async def test_destroy_with_force_kill(self): await executor_proxy.destroy() mock_process.poll.assert_called_once() - mock_process.terminate.assert_called_once() + if platform.system() != "Windows": + mock_process.terminate.assert_called_once() + else: + mock_process.send_signal.assert_called_once_with(signal.CTRL_BREAK_EVENT) mock_process.wait.assert_called_once_with(timeout=5) mock_process.kill.assert_called_once()