diff --git a/src/unstract/llmwhisperer/client_v2.py b/src/unstract/llmwhisperer/client_v2.py index 4f47005..cf97f4b 100644 --- a/src/unstract/llmwhisperer/client_v2.py +++ b/src/unstract/llmwhisperer/client_v2.py @@ -22,6 +22,7 @@ import logging import os import time +import warnings from typing import IO, Any import requests @@ -367,6 +368,48 @@ def whisper_detail(self, whisper_hash: str) -> Any: raise LLMWhispererClientException(err, response.status_code) return json.loads(response.text) + def _resolve_deprecated_param( + self, + name: str, + value: str | None, + deprecated_name: str, + deprecated_value: str | None, + default: str, + *, + forward: bool, + ) -> str: + """Resolves a renamed parameter, warning when the old name is used. + + Args: + name: The supported parameter name. + value: Value passed under the supported name, None when unset. + deprecated_name: The deprecated parameter name. + deprecated_value: Value passed under the deprecated name, None when unset. + default: Value to use when neither name is passed. + forward: Whether the deprecated value is honoured. False for parameters the + service never received, where applying the value now would silently + change extraction output. + + Returns: + The resolved value. + + Raises: + LLMWhispererClientException: If both names are passed. + """ + if deprecated_value is None: + return default if value is None else value + if value is not None: + raise LLMWhispererClientException( + f"Cannot pass both '{deprecated_name}' and '{name}', use '{name}' only", + 1, + ) + message = f"'{deprecated_name}' is deprecated and will be removed in a future release, use '{name}' instead" + if not forward: + message += f". The value passed is ignored: '{deprecated_name}' never reached the service" + self.logger.warning(message) + warnings.warn(message, DeprecationWarning, stacklevel=3) + return deprecated_value if forward else default + def whisper( self, file_path: str = "", @@ -374,7 +417,7 @@ def whisper( url: str = "", mode: str = "form", output_mode: str = "layout_preserving", - page_seperator: str = "<<<", + page_seperator: str | None = None, pages_to_extract: str = "", median_filter_size: int = 0, gaussian_blur_radius: int = 0, @@ -382,18 +425,21 @@ def whisper( horizontal_stretch_factor: float = 1.0, mark_vertical_lines: bool = False, mark_horizontal_lines: bool = False, - line_spitter_strategy: str = "left-priority", + line_spitter_strategy: str | None = None, add_line_nos: bool = False, include_line_confidence: bool = False, word_confidence_threshold: float = 0.3, lang: str = "eng", tag: str = "default", - filename: str = "", + filename: str | None = None, webhook_metadata: str = "", use_webhook: str = "", wait_for_completion: bool = False, wait_timeout: int = 180, encoding: str = "utf-8", + page_separator: str | None = None, + line_splitter_strategy: str | None = None, + file_name: str | None = None, ) -> Any: """Sends a request to the LLMWhisperer API to process a document. Refer to https://docs.unstract.com/llm_whisperer/apis/llm_whisperer_text_extraction_api. @@ -406,7 +452,8 @@ def whisper( or "table". Defaults to "high_quality". output_mode (str, optional): The output mode. Can be "layout_preserving" or "text". Defaults to "layout_preserving". - page_seperator (str, optional): The page separator. Defaults to "<<<". + page_seperator (str, optional): Deprecated misspelling of page_separator, still + honoured. Defaults to None. pages_to_extract (str, optional): The pages to extract. Defaults to "". median_filter_size (int, optional): The size of the median filter. Defaults to 0. gaussian_blur_radius (int, optional): The radius of the Gaussian blur. Defaults to 0. @@ -414,7 +461,9 @@ def whisper( horizontal_stretch_factor (float, optional): The horizontal stretch factor. Defaults to 1.0. mark_vertical_lines (bool, optional): Whether to mark vertical lines. Defaults to False. mark_horizontal_lines (bool, optional): Whether to mark horizontal lines. Defaults to False. - line_spitter_strategy (str, optional): The line splitter strategy. Defaults to "left-priority". + line_spitter_strategy (str, optional): Deprecated misspelling of + line_splitter_strategy. The value is ignored, since it was never sent under a + name the service reads. Defaults to None. add_line_nos (bool, optional): Adds line numbers to the extracted text and saves line metadata, which can be queried later using the highlights API. include_line_confidence (bool, optional): Adds line confidence to the line metadata returned by @@ -426,7 +475,8 @@ def whisper( modes. Defaults to 0.3. lang (str, optional): The language of the document. Defaults to "eng". tag (str, optional): The tag for the document. Defaults to "default". - filename (str, optional): The name of the file to store in reports. Defaults to "". + filename (str, optional): Deprecated name for file_name, still honoured. + Defaults to None. webhook_metadata (str, optional): The webhook metadata. This data will be passed to the webhook if webhooks are used Defaults to "". use_webhook (str, optional): Webhook name to call. Defaults to "". If not provided, then @@ -436,6 +486,10 @@ def whisper( wait_timeout (int, optional): The number of seconds to wait for the whisper operation to complete. Defaults to 180. encoding (str): The character encoding to use for processing the text. Defaults to "utf-8". + page_separator (str, optional): The page separator. Defaults to "<<<". + line_splitter_strategy (str, optional): The line splitter strategy. + Defaults to "left-priority". + file_name (str, optional): The name of the file to store in reports. Defaults to "". Returns: Dict[Any, Any]: The response from the API as a dictionary. @@ -443,13 +497,27 @@ def whisper( Raises: LLMWhispererClientException: If the API request fails, it raises an exception with the error message and status code returned by the API. + Also raised when a parameter is passed under both its + deprecated and its supported name. """ self.logger.debug("whisper called") + page_separator = self._resolve_deprecated_param( + "page_separator", page_separator, "page_seperator", page_seperator, "<<<", forward=True + ) + line_splitter_strategy = self._resolve_deprecated_param( + "line_splitter_strategy", + line_splitter_strategy, + "line_spitter_strategy", + line_spitter_strategy, + "left-priority", + forward=False, + ) + file_name = self._resolve_deprecated_param("file_name", file_name, "filename", filename, "", forward=True) api_url = f"{self.base_url}/whisper" params = { "mode": mode, "output_mode": output_mode, - "page_seperator": page_seperator, + "page_separator": page_separator, "pages_to_extract": pages_to_extract, "median_filter_size": median_filter_size, "gaussian_blur_radius": gaussian_blur_radius, @@ -457,13 +525,13 @@ def whisper( "horizontal_stretch_factor": horizontal_stretch_factor, "mark_vertical_lines": mark_vertical_lines, "mark_horizontal_lines": mark_horizontal_lines, - "line_spitter_strategy": line_spitter_strategy, + "line_splitter_strategy": line_splitter_strategy, "add_line_nos": add_line_nos, "include_line_confidence": include_line_confidence, "word_confidence_threshold": word_confidence_threshold, "lang": lang, "tag": tag, - "filename": filename, + "file_name": file_name, "webhook_metadata": webhook_metadata, "use_webhook": use_webhook, } diff --git a/tests/integration/client_v2_test.py b/tests/integration/client_v2_test.py index 7ec0a5a..2c2e863 100644 --- a/tests/integration/client_v2_test.py +++ b/tests/integration/client_v2_test.py @@ -287,6 +287,24 @@ def test_whisper_detail_not_found(client_v2: LLMWhispererClientV2) -> None: assert "message" in error +def test_whisper_line_splitter_strategy_reaches_service( + client_v2: LLMWhispererClientV2, data_dir: str +) -> None: + """An unknown strategy is rejected, which only happens if the param arrives.""" + file_path = os.path.join(data_dir, "credit_card.pdf") + + with pytest.raises(LLMWhispererClientException) as exc_info: + client_v2.whisper( + mode="native_text", + output_mode="text", + file_path=file_path, + line_splitter_strategy="not-a-strategy", + wait_for_completion=True, + ) + + assert exc_info.value.error_message()["status_code"] == 400 + + def assert_error_message(whisper_result: dict) -> None: assert isinstance(whisper_result, dict) assert whisper_result["status"] == "error" diff --git a/tests/unit/client_v2_test.py b/tests/unit/client_v2_test.py index eb5921f..9f7a7fa 100644 --- a/tests/unit/client_v2_test.py +++ b/tests/unit/client_v2_test.py @@ -153,7 +153,8 @@ def test_whisper_invalid_json_response_202(mocker: MockerFixture, client_v2: LLM def test_whisper_default_word_confidence_threshold(mocker: MockerFixture, client_v2: LLMWhispererClientV2) -> None: - """whisper() sends the default word_confidence_threshold when not specified.""" + """Whisper() sends the default word_confidence_threshold when not + specified.""" mock_send = mocker.patch("requests.Session.send") mock_send.return_value = _mock_response(200, '{"status_code": 200, "extraction": {"text": "ok"}}') @@ -165,7 +166,8 @@ def test_whisper_default_word_confidence_threshold(mocker: MockerFixture, client def test_whisper_custom_word_confidence_threshold(mocker: MockerFixture, client_v2: LLMWhispererClientV2) -> None: - """whisper() forwards a custom word_confidence_threshold as a request param.""" + """Whisper() forwards a custom word_confidence_threshold as a request + param.""" mock_send = mocker.patch("requests.Session.send") mock_send.return_value = _mock_response(200, '{"status_code": 200, "extraction": {"text": "ok"}}') @@ -180,6 +182,90 @@ def test_whisper_custom_word_confidence_threshold(mocker: MockerFixture, client_ assert query["word_confidence_threshold"] == ["0.75"] +# --- Deprecated parameter tests --- + + +def _whisper_query(mocker: MockerFixture, client_v2: LLMWhispererClientV2, **kwargs: object) -> dict[str, list[str]]: + """Calls whisper() with a mocked transport and returns the query params + sent.""" + mock_send = mocker.patch("requests.Session.send") + mock_send.return_value = _mock_response(200, '{"status_code": 200, "extraction": {"text": "ok"}}') + + client_v2.whisper(url="https://example.com/test.pdf", wait_for_completion=False, **kwargs) + + return parse_qs(urlparse(mock_send.call_args[0][0].url).query) + + +def test_whisper_sends_corrected_param_names(mocker: MockerFixture, client_v2: LLMWhispererClientV2) -> None: + """Whisper() sends the names the service reads, not the misspelled ones.""" + query = _whisper_query( + mocker, + client_v2, + page_separator="---", + line_splitter_strategy="mid-priority", + file_name="invoice.pdf", + ) + + assert query["page_separator"] == ["---"] + assert query["line_splitter_strategy"] == ["mid-priority"] + assert query["file_name"] == ["invoice.pdf"] + assert "page_seperator" not in query + assert "line_spitter_strategy" not in query + assert "filename" not in query + + +def test_whisper_defaults_when_no_param_passed(mocker: MockerFixture, client_v2: LLMWhispererClientV2) -> None: + """Omitting the renamed params keeps the previous defaults.""" + query = _whisper_query(mocker, client_v2) + + assert query["page_separator"] == ["<<<"] + assert query["line_splitter_strategy"] == ["left-priority"] + assert "file_name" not in query # sent blank, and parse_qs drops blank values + + +def test_whisper_deprecated_page_seperator_is_forwarded(mocker: MockerFixture, client_v2: LLMWhispererClientV2) -> None: + """page_seperator still applies, under the corrected name.""" + with pytest.warns(DeprecationWarning, match="page_separator"): + query = _whisper_query(mocker, client_v2, page_seperator="---") + + assert query["page_separator"] == ["---"] + + +def test_whisper_deprecated_filename_is_forwarded(mocker: MockerFixture, client_v2: LLMWhispererClientV2) -> None: + """Deprecated filename still applies, under the corrected name.""" + with pytest.warns(DeprecationWarning, match="file_name"): + query = _whisper_query(mocker, client_v2, filename="invoice.pdf") + + assert query["file_name"] == ["invoice.pdf"] + + +def test_whisper_deprecated_line_spitter_strategy_is_ignored( + mocker: MockerFixture, client_v2: LLMWhispererClientV2 +) -> None: + """line_spitter_strategy never reached the service, so its value stays + unused.""" + with pytest.warns(DeprecationWarning, match="line_splitter_strategy"): + query = _whisper_query(mocker, client_v2, line_spitter_strategy="mid-priority") + + assert query["line_splitter_strategy"] == ["left-priority"] + + +@pytest.mark.parametrize( + ("deprecated_name", "name"), + [ + ("page_seperator", "page_separator"), + ("line_spitter_strategy", "line_splitter_strategy"), + ("filename", "file_name"), + ], +) +def test_whisper_rejects_both_spellings( + mocker: MockerFixture, client_v2: LLMWhispererClientV2, deprecated_name: str, name: str +) -> None: + """Passing a param under both names is ambiguous and fails loudly.""" + with pytest.raises(LLMWhispererClientException, match="Cannot pass both"): + _whisper_query(mocker, client_v2, **{deprecated_name: "x", name: "y"}) + + # --- Retry behavior tests ---