From a6a1476066f2b331020521b4078488845dc852f2 Mon Sep 17 00:00:00 2001 From: Nicholas Albion Date: Fri, 6 Oct 2023 14:01:20 +1100 Subject: [PATCH] "Expecting value" thrown in 2 scenarios: - expecting more value (send me the rest) - expecting valid value (`True` is not a value) --- pilot/utils/llm_connection.py | 60 +++++--- pilot/utils/test_llm_connection.py | 223 ++++++++++++++++++++++++++++- 2 files changed, 255 insertions(+), 28 deletions(-) diff --git a/pilot/utils/llm_connection.py b/pilot/utils/llm_connection.py index 3ff1f00..b4e3117 100644 --- a/pilot/utils/llm_connection.py +++ b/pilot/utils/llm_connection.py @@ -144,14 +144,19 @@ def get_tokens_in_messages_from_openai_error(error_message): def retry_on_exception(func): + def update_error_count(args): + function_error_count = 1 if 'function_error' not in args[0] else args[0]['function_error_count'] + 1 + args[0]['function_error_count'] = function_error_count + return function_error_count + + def set_function_error(args, err_str: str): + logger.info(err_str) + + args[0]['function_error'] = err_str + if 'function_buffer' in args[0]: + del args[0]['function_buffer'] + def wrapper(*args, **kwargs): - # spinner = None - - def update_error_count(args): - function_error_count = 1 if 'function_error' not in args[0] else args[0]['function_error_count'] + 1 - args[0]['function_error_count'] = function_error_count - return function_error_count - while True: try: # spinner_stop(spinner) @@ -161,28 +166,41 @@ def retry_on_exception(func): err_str = str(e) if isinstance(e, json.JSONDecodeError): - if 'Invalid' in e.msg: - # 'Invalid control character at', 'Invalid \\escape' - function_error_count = update_error_count(args) - logger.warning('Received invalid character in JSON response from LLM. Asking to retry...') - args[0]['function_error'] = err_str - if function_error_count < 3: + # codellama-34b-instruct seems to send incomplete JSON responses. + # We ask for the rest of the JSON object for the following errors: + # - 'Expecting value' (error if `e.pos` not at the end of the doc: True instead of true) + # - "Expecting ':' delimiter" + # - 'Expecting property name enclosed in double quotes' + # - 'Unterminated string starting at' + if e.msg.startswith('Expecting') or e.msg == 'Unterminated string starting at': + if e.msg == 'Expecting value' and len(e.doc) > e.pos: + # TODO: attempt to heal True/False boolean values + # if e.doc[e.pos] == '{': + err_str = re.split(r'[},\\n]', e.doc[e.pos:])[0] + err_str = f'Invalid value: `{err_str}`' + else: + # if e.msg == 'Unterminated string starting at' or len(e.doc) == e.pos: + logger.info('Received incomplete JSON response from LLM. Asking for the rest...') + args[0]['function_buffer'] = e.doc + if 'function_error' in args[0]: + del args[0]['function_error'] continue - else: - # 'Expecting value', 'Unterminated string starting at', - # codellama-34b-instruct seems to send incomplete JSON responses - logger.info('Received incomplete JSON response from LLM. Asking for the rest...') - args[0]['function_buffer'] = e.doc + + # TODO: (if it ever comes up) e.msg == 'Extra data' -> trim the response + # 'Invalid control character at', 'Invalid \\escape', 'Invalid control character', + # or `Expecting value` with `pos` before the end of `e.doc` + function_error_count = update_error_count(args) + logger.warning('Received invalid character in JSON response from LLM. Asking to retry...') + set_function_error(args, err_str) + if function_error_count < 3: continue elif isinstance(e, ValidationError): function_error_count = update_error_count(args) logger.warning('Received invalid JSON response from LLM. Asking to retry...') - logger.info(f' at {e.json_path} {e.message}') # eg: # json_path: '$.type' # message: "'command' is not one of ['automated_test', 'command_test', 'manual_test', 'no_test']" - args[0]['function_error'] = f'at {e.json_path} - {e.message}' - + set_function_error(args, f'at {e.json_path} - {e.message}') # Attempt retry if the JSON schema is invalid, but avoid getting stuck in a loop if function_error_count < 3: continue diff --git a/pilot/utils/test_llm_connection.py b/pilot/utils/test_llm_connection.py index e3319e3..0d9e79b 100644 --- a/pilot/utils/test_llm_connection.py +++ b/pilot/utils/test_llm_connection.py @@ -14,7 +14,7 @@ from utils.function_calling import parse_agent_response, FunctionType from test.test_utils import assert_non_empty_string from test.mock_questionary import MockQuestionary from utils.llm_connection import create_gpt_chat_completion, stream_gpt_completion, \ - assert_json_response, assert_json_schema, clean_json_response + assert_json_response, assert_json_schema, clean_json_response, retry_on_exception from main import get_custom_print load_dotenv() @@ -76,6 +76,215 @@ def test_clean_json_response_boolean_in_python(): assert '"content": "json = {\'is_true\': True,\\n \'is_false\': False}"' in response +@patch('utils.llm_connection.styled_text', return_value='') +class TestRetryOnException: + def setup_method(self): + self.function: FunctionType = { + 'name': 'test', + 'description': 'test schema', + 'parameters': { + 'type': 'object', + 'properties': { + 'foo': {'type': 'string'}, + 'boolean': {'type': 'boolean'}, + 'items': {'type': 'array'} + }, + 'required': ['foo'] + } + } + + def _create_wrapped_function(self, json_responses: list[str]): + args = {}, 'test', project + + def retryable_assert_json_schema(data, _req_type, _project): + json_string = json_responses.pop(0) + if 'function_buffer' in data: + json_string = data['function_buffer'] + json_string + assert_json_schema(json_string, [self.function]) + return json_string + + return retry_on_exception(retryable_assert_json_schema), args + + def test_incomplete_value_string(self, mock_styled_text): + # Given incomplete JSON + wrapper, args = self._create_wrapped_function(['{"foo": "bar', '"}']) + + # When + response = wrapper(*args) + + # Then should tell the LLM the JSON response is incomplete and to continue + # 'Unterminated string starting at' + assert response == '{"foo": "bar"}' + assert 'function_error' not in args[0] + # And the user should not need to be notified + assert mock_styled_text.call_count == 0 + + def test_incomplete_key(self, mock_styled_text): + # Given invalid JSON boolean + wrapper, args = self._create_wrapped_function([ + '{"foo', + '": "bar"}' + ]) + + # When + response = wrapper(*args) + + # Then should tell the LLM the JSON response is incomplete and to continue + # 'Unterminated string starting at: line 1 column 2 (char 1)' + assert response == '{"foo": "bar"}' + assert 'function_error' not in args[0] + # And the user should not need to be notified + assert mock_styled_text.call_count == 0 + + def test_incomplete_value_missing(self, mock_styled_text): + # Given invalid JSON boolean + wrapper, args = self._create_wrapped_function([ + '{"foo":', + ' "bar"}' + ]) + + # When + response = wrapper(*args) + + # Then should tell the LLM the JSON response is incomplete and to continue + # 'Expecting value: line 1 column 8 (char 7)' + assert response == '{"foo": "bar"}' + assert 'function_error' not in args[0] + # And the user should not need to be notified + assert mock_styled_text.call_count == 0 + + def test_invalid_boolean(self, mock_styled_text): + # Given invalid JSON boolean + wrapper, args = self._create_wrapped_function([ + '{"foo": "bar", "boolean": True}', + '{"foo": "bar", "boolean": True}', + '{"foo": "bar", "boolean": True}', + '{"foo": "bar", "boolean": true}', + ]) + + # When + response = wrapper(*args) + + # Then should tell the LLM there is an error in the JSON response + # 'Expecting value: line 1 column 13 (char 12)' + assert response == '{"foo": "bar", "boolean": true}' + assert args[0]['function_error'] == 'Invalid value: `True`' + assert 'function_buffer' not in args[0] + # And the user should not need to be notified + assert mock_styled_text.call_count == 1 + + def test_invalid_escape(self, mock_styled_text): + # Given invalid JSON boolean + wrapper, args = self._create_wrapped_function([ + '{"foo": "\\!"}', + '{"foo": "\\xBADU"}', + '{"foo": "\\xd800"}', + '{"foo": "bar"}', + ]) + + # When + response = wrapper(*args) + + # Then should tell the LLM there is an error in the JSON response + # 'Invalid \\escape: line 1 column 10 (char 9)' + assert response == '{"foo": "bar"}' + assert len(args[0]['function_error']) > 0 + assert 'function_buffer' not in args[0] + # And the user should not need to be notified + assert mock_styled_text.call_count == 1 + + def test_incomplete_json_item(self, mock_styled_text): + # Given incomplete JSON + wrapper, args = self._create_wrapped_function([ + '{"foo": "bar",', + ' "boolean"', + ': true}']) + + # When + response = wrapper(*args) + + # Then should tell the LLM the JSON response is incomplete and to continue + # 'Expecting property name enclosed in double quotes: line 1 column 15 (char 14)' + # "Expecting ':' delimiter: line 1 column 25 (char 24)" + assert response == '{"foo": "bar", "boolean": true}' + assert 'function_error' not in args[0] + # And the user should not need to be notified + assert mock_styled_text.call_count == 0 + + def test_incomplete_json_array(self, mock_styled_text): + # Given incomplete JSON + wrapper, args = self._create_wrapped_function([ + '{"foo": "bar", "items": [1, 2, 3, "4"', + ', 5]}']) + + # When + response = wrapper(*args) + + # Then should tell the LLM the JSON response is incomplete and to continue + # "Expecting ',' delimiter: line 1 column 24 (char 23)" + assert response == '{"foo": "bar", "items": [1, 2, 3, "4", 5]}' + assert 'function_error' not in args[0] + # And the user should not need to be notified + assert mock_styled_text.call_count == 0 + + def test_incomplete_then_invalid_by_schema(self, mock_styled_text): + # Given incomplete JSON + wrapper, args = self._create_wrapped_function([ + '{"items": [1, 2, 3, "4"', + ', 5]}', + # Please try again with a valid JSON object, referring to the previous JSON schema I provided above + '{"foo": "bar",', + ' "items": [1, 2, 3, "4"', + ', 5]}' + ]) + + # When + response = wrapper(*args) + + # Then should tell the LLM the JSON response is incomplete and to continue + # "Expecting ',' delimiter: line 1 column 24 (char 23)" + # "'foo' is a required property" + assert response == '{"foo": "bar", "items": [1, 2, 3, "4", 5]}' + assert 'function_error' not in args[0] + # And the user should not need to be notified + assert mock_styled_text.call_count == 0 + + def test_invalid_boolean_max_retries(self, mock_styled_text): + # Given invalid JSON boolean + wrapper, args = self._create_wrapped_function([ + '{"boolean": True, "foo": "bar"}', + '{"boolean": True,\n "foo": "bar"}', + '{"boolean": True}', + '{"boolean": true, "foo": "bar"}', + ]) + + # When + response = wrapper(*args) + + # Then should tell the LLM there is an error in the JSON response + assert response == '{"boolean": true, "foo": "bar"}' + assert args[0]['function_error'] == 'Invalid value: `True`' + assert mock_styled_text.call_count == 1 + + def test_extra_data(self, mock_styled_text): + # Given invalid JSON boolean + wrapper, args = self._create_wrapped_function([ + '{"boolean": true, "foo": "bar"}\n I hope that helps', + '{"boolean": true, "foo": "bar"}\n I hope that helps', + '{"boolean": true, "foo": "bar"}\n I hope that helps', + '{"boolean": true, "foo": "bar"}', + ]) + + # When + response = wrapper(*args) + + # Then should tell the LLM there is an error in the JSON response + assert response == '{"boolean": true, "foo": "bar"}' + # assert len(args[0]['function_error']) > 0 + assert args[0]['function_error'] == 'Extra data: line 2 column 2 (char 33)' + assert mock_styled_text.call_count == 1 + + class TestSchemaValidation: def setup_method(self): self.function: FunctionType = { @@ -101,18 +310,18 @@ class TestSchemaValidation: # Then no errors assert(assert_json_schema('{"foo": "bar"}', [self.function])) - def test_assert_json_schema_invalid(self): - # When assert_json_schema is called with invalid JSON - # Then error is raised - with pytest.raises(ValidationError, match="1 is not of type 'string'"): - assert_json_schema('{"foo": 1}', [self.function]) - def test_assert_json_schema_incomplete(self): # When assert_json_schema is called with incomplete JSON # Then error is raised with pytest.raises(JSONDecodeError): assert_json_schema('{"foo": "b', [self.function]) + def test_assert_json_schema_invalid(self): + # When assert_json_schema is called with invalid JSON + # Then error is raised + with pytest.raises(ValidationError, match="1 is not of type 'string'"): + assert_json_schema('{"foo": 1}', [self.function]) + def test_assert_json_schema_required(self): # When assert_json_schema is called with missing required property # Then error is raised