Fix reviewer feedback: timeout constant, simplified error message, test integration

Author: DongilMin

src/co_op_translator/core/llm/markdown_translator.py

@@ -29,6 +29,8 @@ class MarkdownTranslator(ABC):
     Provides common utilities and abstract methods to be implemented by providers.
     """
 
+    TRANSLATION_TIMEOUT_SECONDS = 300  # Translation timeout in seconds
+
     def __init__(self, root_dir: Path = None):
         """Initialize translator with project configuration.
 
@@ -165,19 +167,23 @@ async def _run_prompts_sequentially(self, prompts, md_file_path):
         for index, prompt in enumerate(prompts):
             try:
                 result = await asyncio.wait_for(
-                    self._run_prompt(prompt, index + 1, len(prompts)), timeout=300
+                    self._run_prompt(prompt, index + 1, len(prompts)),
+                    timeout=self.TRANSLATION_TIMEOUT_SECONDS,
                 )
                 results.append(result)
             except asyncio.TimeoutError:
                 logger.warning(
-                    f"Translation timeout for chunk {index + 1} of file '{md_file_path.name}': Request exceeded 300 seconds. Check your network connection and API response time."
+                    f"Translation timeout for chunk {index + 1} of file '{md_file_path.name}': "
+                    f"Request exceeded {self.TRANSLATION_TIMEOUT_SECONDS} seconds. "
+                    f"Check your network connection and API response time."
                 )
                 results.append(
                     f"Translation for chunk {index + 1} of '{md_file_path.name}' skipped due to timeout."
                 )
             except Exception as e:
                 logger.error(
-                    f"Translation failed for chunk {index + 1} of file '{md_file_path.name}': {str(e)}. Check your API configuration and network connection."
+                    f"Translation failed for chunk {index + 1} of file '{md_file_path.name}': {str(e)}. "
+                    f"Check your API configuration and network connection."
                 )
                 results.append(
                     f"Error translating chunk {index + 1} of '{md_file_path.name}': {str(e)}"

src/co_op_translator/core/vision/image_translator.py

@@ -109,7 +109,9 @@ def extract_line_bounding_boxes(self, image_path):
             return line_bounding_boxes
         else:
             raise Exception(
-                f"No text detected in image '{Path(image_path).name}': The image may not contain readable text, or the text quality is too poor for recognition. Try using images with clear, high-contrast text."
+                f"No text detected in image '{Path(image_path).name}': "
+                f"The image may not contain clear, high-contrast text, or the text quality is too poor for recognition. "
+                f"Please ensure the image contains readable text."
             )
 
     def plot_annotated_image(
@@ -204,7 +206,8 @@ def plot_annotated_image(
                 base_font = ImageFont.truetype(font_path, font_size)
             except IOError:
                 logger.error(
-                    f"Font file not found for language '{target_language_code}' at '{font_path}' in image '{Path(image_path).name}': Using default font. Install the required font or check font configuration."
+                    f"Font file not found for language '{target_language_code}' at '{font_path}' in image '{Path(image_path).name}': "
+                    f"Using default font. Install the required font or check font configuration."
                 )
                 base_font = ImageFont.load_default()
 
@@ -229,7 +232,9 @@ def plot_annotated_image(
                     bounding_box_flat = line_info.get("bounding_box", [])
                     if len(bounding_box_flat) != 8:
                         logger.error(
-                            f"Invalid text detection data in image '{Path(image_path).name}': Expected 8 coordinates but got {len(bounding_box_flat)}. The text detection may be corrupted."
+                            f"Invalid text detection data in image '{Path(image_path).name}': "
+                            f"Expected 8 coordinates but got {len(bounding_box_flat)}. "
+                            f"The text detection may be corrupted."
                         )
                         continue
 
@@ -238,7 +243,8 @@ def plot_annotated_image(
                     )
                     if len(bounding_box_tuples) < 4:
                         logger.error(
-                            f"Insufficient bounding box points in image '{Path(image_path).name}': Text detection data is incomplete. Try re-processing the image."
+                            f"Insufficient bounding box points in image '{Path(image_path).name}': "
+                            f"Text detection data is incomplete. Try re-processing the image."
                         )
                         continue
 
@@ -250,7 +256,8 @@ def plot_annotated_image(
                         angle = -angle  # Invert angle for proper rotation.
                     except ValueError:
                         logger.error(
-                            f"Invalid bounding box coordinates in image '{Path(image_path).name}': Text detection geometry is malformed. The image may have distorted text regions."
+                            f"Invalid bounding box coordinates in image '{Path(image_path).name}': "
+                            f"Text detection geometry is malformed. The image may have distorted text regions."
                         )
                         continue
 
@@ -288,7 +295,8 @@ def plot_annotated_image(
                             font = ImageFont.truetype(font_path, optimal_font_size)
                         except IOError:
                             logger.error(
-                                f"Font file not found for language '{target_language_code}' at '{font_path}' in image '{Path(image_path).name}': Using default font. Install the required font or check font configuration."
+                                f"Font file not found for language '{target_language_code}' at '{font_path}' in image '{Path(image_path).name}': "
+                                f"Using default font. Install the required font or check font configuration."
                             )
                             font = ImageFont.load_default()
 
@@ -353,7 +361,8 @@ def plot_annotated_image(
                 font = ImageFont.truetype(font_path, font_size)
             except IOError:
                 logger.error(
-                    f"Font file not found for language '{target_language_code}' at '{font_path}' in image '{Path(image_path).name}': Using default font. Install the required font or check font configuration."
+                    f"Font file not found for language '{target_language_code}' at '{font_path}' in image '{Path(image_path).name}': "
+                    f"Using default font. Install the required font or check font configuration."
                 )
                 font = ImageFont.load_default()
 
@@ -461,7 +470,9 @@ def translate_image(
             # Check if any text was recognized
             if not line_bounding_boxes:
                 logger.info(
-                    f"No text detected in image '{image_path.name}': Saving original image as translation result. The image may not contain readable text or text may be too small/blurry to detect."
+                    f"No text detected in image '{image_path.name}': "
+                    f"Saving original image as translation result. "
+                    f"The image may not contain readable text or text may be too small/blurry to detect."
                 )
 
                 # Load the original image and save it with the new name
@@ -492,7 +503,8 @@ def translate_image(
 
         except Exception as e:
             logger.error(
-                f"Failed to translate image '{image_path.name}': {str(e)}. Saving original image instead. Check Computer Vision API configuration and image file accessibility."
+                f"Failed to translate image '{image_path.name}': {str(e)}. "
+                f"Saving original image instead."
             )
 
             # Load the original image and save it with the new name
@@ -543,5 +555,7 @@ def create(
         except (ImportError, ValueError) as e:
             logger.warning(f"Computer Vision is not properly configured: {e}")
             raise ValueError(
-                "Computer Vision not configured: Missing required environment variables (AZURE_COMPUTER_VISION_KEY, AZURE_COMPUTER_VISION_ENDPOINT). Please check your .env file and API configuration."
+                "Image translation is not configured: Missing required environment variables "
+                "(AZURE_AI_SERVICE_API_KEY, AZURE_AI_SERVICE_ENDPOINT). "
+                "Please check your .env file and ensure your Azure AI service credentials are set correctly."
             )

tests/co_op_translator/core/project/test_project_translator.py

@@ -155,3 +155,154 @@ async def test_markdown_only_mode(temp_project_dir):
             ]
             is True
         )
+
+
+class TestProjectTranslatorErrorMessages:
+    """Test improved error messages in ProjectTranslator.
+
+    This test class verifies error message improvements made for issue #46.
+    These tests ensure that error messages are user-friendly and provide helpful context.
+    """
+
+    def test_computer_vision_config_error_message(self, tmp_path, caplog):
+        """Test Computer Vision configuration error message."""
+        # Set logging level to capture info messages
+        caplog.set_level("INFO")
+
+        with patch(
+            "co_op_translator.core.vision.image_translator.ImageTranslator.create",
+            side_effect=ValueError("CV error"),
+        ):
+            with patch(
+                "co_op_translator.core.llm.text_translator.TextTranslator.create"
+            ):
+                with patch(
+                    "co_op_translator.core.llm.markdown_translator.MarkdownTranslator.create"
+                ):
+                    translator = ProjectTranslator(
+                        "ko", str(tmp_path), markdown_only=False
+                    )
+
+                    # Check that it auto-switched to markdown_only
+                    assert translator.markdown_only is True
+
+                    # Check improved log message
+                    log_text = caplog.text
+                    assert "Computer Vision not configured" in log_text
+                    assert "AZURE_COMPUTER_VISION_KEY" in log_text
+
+    def test_markdown_only_mode_message(self, tmp_path, caplog):
+        """Test markdown-only mode initialization message."""
+        # Set logging level to capture info messages
+        caplog.set_level("INFO")
+
+        with patch("co_op_translator.core.llm.text_translator.TextTranslator.create"):
+            with patch(
+                "co_op_translator.core.llm.markdown_translator.MarkdownTranslator.create"
+            ):
+                translator = ProjectTranslator("ko", str(tmp_path), markdown_only=True)
+
+                # Check improved log message for markdown-only mode
+                log_text = caplog.text
+                assert "Running in markdown-only mode" in log_text
+                assert "Image translation disabled" in log_text
+
+    def test_missing_api_keys_scenario(self, tmp_path):
+        """Test scenario where all API keys are missing."""
+        with patch(
+            "co_op_translator.config.llm_config.config.LLMConfig.get_available_provider",
+            return_value=None,
+        ):
+            with patch(
+                "co_op_translator.config.vision_config.config.VisionConfig.get_available_provider",
+                side_effect=ValueError("No CV"),
+            ):
+                # Should raise error when trying to create without proper config
+                with pytest.raises(ValueError) as exc_info:
+                    ProjectTranslator("ko", str(tmp_path), markdown_only=False)
+
+                # Check error message contains the actual message from the code
+                error_msg = str(exc_info.value)
+                assert "No valid LLM provider configured" in error_msg
+
+    def test_project_initialization_with_missing_services(self, tmp_path, caplog):
+        """Test ProjectTranslator initialization when services are missing."""
+        # Set logging level to capture info messages
+        caplog.set_level("INFO")
+
+        with patch("co_op_translator.core.llm.text_translator.TextTranslator.create"):
+            with patch(
+                "co_op_translator.core.llm.markdown_translator.MarkdownTranslator.create"
+            ):
+                with patch(
+                    "co_op_translator.core.vision.image_translator.ImageTranslator.create",
+                    side_effect=ValueError("No CV"),
+                ):
+                    translator = ProjectTranslator(
+                        "ko ja", str(tmp_path), markdown_only=False
+                    )
+
+                    # Should auto-switch to markdown-only
+                    assert translator.markdown_only is True
+                    assert translator.image_translator is None
+
+                    # Should have helpful error message
+                    log_text = caplog.text
+                    assert "Computer Vision not configured" in log_text
+                    assert "AZURE_COMPUTER_VISION_KEY" in log_text
+
+    def test_environment_variable_references_in_logs(self, tmp_path, caplog):
+        """Test that log messages reference environment variables."""
+        caplog.set_level("INFO")
+
+        with patch(
+            "co_op_translator.core.vision.image_translator.ImageTranslator.create",
+            side_effect=ValueError("Image translation error"),
+        ):
+            with patch(
+                "co_op_translator.core.llm.text_translator.TextTranslator.create"
+            ):
+                with patch(
+                    "co_op_translator.core.llm.markdown_translator.MarkdownTranslator.create"
+                ):
+                    ProjectTranslator("ko", str(tmp_path), markdown_only=False)
+
+                    log_text = caplog.text
+                    # Check the actual log message format
+                    assert "Computer Vision not configured" in log_text
+                    assert "AZURE_COMPUTER_VISION_KEY" in log_text
+
+
+def test_computer_vision_config_error_message(tmp_path, caplog):
+    """Test Computer Vision configuration error message."""
+    caplog.set_level("INFO")
+
+    with patch(
+        "co_op_translator.core.vision.image_translator.ImageTranslator.create",
+        side_effect=ValueError("CV error"),
+    ):
+        with patch("co_op_translator.core.llm.text_translator.TextTranslator.create"):
+            with patch(
+                "co_op_translator.core.llm.markdown_translator.MarkdownTranslator.create"
+            ):
+                translator = ProjectTranslator("ko", str(tmp_path), markdown_only=False)
+
+                assert translator.markdown_only is True
+                log_text = caplog.text
+                assert "Computer Vision not configured" in log_text
+                assert "AZURE_COMPUTER_VISION_KEY" in log_text
+
+
+def test_markdown_only_mode_message(tmp_path, caplog):
+    """Test markdown-only mode initialization message."""
+    caplog.set_level("INFO")
+
+    with patch("co_op_translator.core.llm.text_translator.TextTranslator.create"):
+        with patch(
+            "co_op_translator.core.llm.markdown_translator.MarkdownTranslator.create"
+        ):
+            translator = ProjectTranslator("ko", str(tmp_path), markdown_only=True)
+
+            log_text = caplog.text
+            assert "Running in markdown-only mode" in log_text
+            assert "Image translation disabled" in log_text