Cleanup: address reviewer feedback on PR #62

.pre-commit-config.yaml

@@ -19,11 +19,11 @@ repos:
       # Remove trailing whitespace
       - id: trailing-whitespace
         args: [--markdown-linebreak-ext=md]
-        exclude: ^(.*\.min\.(js|css)|.*\.lock|.*\.log|.*\.pdf|.*\.zip|.*\.tar\.gz)
+        exclude: ^(assets/roster/.*\.svg|.*\.min\.(js|css)|.*\.lock|.*\.log|.*\.pdf|.*\.zip|.*\.tar\.gz)
 
       # Ensure files end with a newline
       - id: end-of-file-fixer
-        exclude: ^(.*\.min\.(js|css)|.*\.lock|.*\.log|.*\.pdf|.*\.zip|.*\.tar\.gz)
+        exclude: ^(assets/roster/.*\.svg|.*\.min\.(js|css)|.*\.lock|.*\.log|.*\.pdf|.*\.zip|.*\.tar\.gz)
 
       # Check YAML files for syntax errors
       - id: check-yaml
@@ -49,40 +49,30 @@ repos:
   # ============================================
   # Python code formatting and linting
   # ============================================
-  # Note: 使用 ruff format 替代 black，避免格式化工具冲突
-  # ruff format 更快且与 ruff linter 配合更好
   - repo: https://github.com/astral-sh/ruff-pre-commit
     rev: v0.14.7
     hooks:
-      # Ruff linting (只自动修复，不阻止提交)
+      # Ruff linting - removing --exit-zero to ensure we catch issues before they land
       - id: ruff
         args: [--fix, --exit-zero]
-        files: ^(src/|scripts/).*\.py$
-        # --exit-zero: 即使有错误也不阻止提交，只自动修复
 
-      # Ruff import sorting (replaces isort)
+      # Ruff import sorting
       - id: ruff-format
-        files: ^(src/|scripts/).*\.py$
-
-  # Note: python-check-docstring-first hook removed as it's not available
-  # Ruff already handles most code quality checks including import ordering
 
   # ============================================
   # Frontend code formatting and linting
   # ============================================
-  # Note: ESLint 暂时禁用，因为 Next.js 项目通常有自己的 ESLint 配置
-  # 如果需要启用，可以取消下面的注释并配置 ESLint
+  # Note: ESLint is disabled for now due to missing config file in /web
   # - repo: https://github.com/pre-commit/mirrors-eslint
-  #   rev: v10.0.0-alpha.1
+  #   rev: v9.17.0
   #   hooks:
   #     - id: eslint
   #       files: ^web/.*\.(js|jsx|ts|tsx)$
   #       exclude: ^web/(node_modules|\.next|out|dist|build)/
   #       additional_dependencies:
-  #         - eslint@^8.57.0
-  #         - '@typescript-eslint/parser@^6.0.0'
-  #         - '@typescript-eslint/eslint-plugin@^6.0.0'
-  #         - eslint-config-next@14.0.3
+  #         - eslint@^9.0.0
+  #         - eslint-config-next@^15.0.0
+  #         - "@eslint/eslintrc"
   #       args: [--fix]
 
   - repo: https://github.com/pre-commit/mirrors-prettier
@@ -101,5 +91,4 @@ repos:
       - id: detect-secrets
         args: ['--baseline', '.secrets.baseline']
         exclude: package-lock.json
-        # Only scan staged files for performance
         pass_filenames: false

tests/core/test_prompt_manager.py

@@ -0,0 +1,193 @@
+#!/usr/bin/env python
+"""
+Unit tests for the unified PromptManager.
+"""
+
+import pytest
+
+from src.core.prompt_manager import PromptManager, get_prompt_manager
+
+
+class TestPromptManager:
+    """Test cases for PromptManager."""
+
+    def setup_method(self):
+        """Reset singleton and cache before each test."""
+        PromptManager._instance = None
+        PromptManager._cache = {}
+
+    def test_singleton_pattern(self):
+        """Test that PromptManager uses singleton pattern."""
+        pm1 = PromptManager()
+        pm2 = PromptManager()
+        assert pm1 is pm2
+
+    def test_get_prompt_manager_returns_singleton(self):
+        """Test that get_prompt_manager returns the same instance."""
+        pm1 = get_prompt_manager()
+        pm2 = get_prompt_manager()
+        assert pm1 is pm2
+
+    def test_load_prompts_research_module(self):
+        """Test loading prompts for research module."""
+        pm = get_prompt_manager()
+        prompts = pm.load_prompts(
+            module_name="research",
+            agent_name="research_agent",
+            language="en",
+        )
+        assert isinstance(prompts, dict)
+        # research_agent should have system section
+        assert "system" in prompts or prompts == {}
+
+    def test_load_prompts_solve_module(self):
+        """Test loading prompts for solve module."""
+        pm = get_prompt_manager()
+        prompts = pm.load_prompts(
+            module_name="solve",
+            agent_name="solve_agent",
+            language="en",
+        )
+        assert isinstance(prompts, dict)
+
+    def test_load_prompts_guide_module(self):
+        """Test loading prompts for guide module."""
+        pm = get_prompt_manager()
+        prompts = pm.load_prompts(
+            module_name="guide",
+            agent_name="chat_agent",
+            language="en",
+        )
+        assert isinstance(prompts, dict)
+
+    def test_load_prompts_with_subdirectory(self):
+        """Test loading prompts with subdirectory (e.g., solve_loop)."""
+        pm = get_prompt_manager()
+        prompts = pm.load_prompts(
+            module_name="solve",
+            agent_name="solve_agent",
+            language="en",
+            subdirectory="solve_loop",
+        )
+        assert isinstance(prompts, dict)
+
+    def test_caching(self):
+        """Test that prompts are cached after first load."""
+        pm = get_prompt_manager()
+
+        # First load
+        prompts1 = pm.load_prompts("research", "research_agent", "en")
+
+        # Second load should return cached version
+        prompts2 = pm.load_prompts("research", "research_agent", "en")
+
+        assert prompts1 is prompts2
+
+    def test_clear_cache_all(self):
+        """Test clearing all cache."""
+        pm = get_prompt_manager()
+
+        # Load some prompts
+        pm.load_prompts("research", "research_agent", "en")
+        pm.load_prompts("guide", "chat_agent", "en")
+
+        assert len(pm._cache) >= 2
+
+        pm.clear_cache()
+        assert len(pm._cache) == 0
+
+    def test_clear_cache_module_specific(self):
+        """Test clearing cache for specific module."""
+        pm = get_prompt_manager()
+
+        # Load prompts for multiple modules
+        pm.load_prompts("research", "research_agent", "en")
+        pm.load_prompts("guide", "chat_agent", "en")
+
+        initial_count = len(pm._cache)
+
+        # Clear only research cache
+        pm.clear_cache("research")
+
+        # Guide prompts should still be cached
+        assert any("guide" in k for k in pm._cache)
+        assert not any("research" in k for k in pm._cache)
+
+    def test_get_prompt_helper(self):
+        """Test the get_prompt helper method."""
+        pm = get_prompt_manager()
+
+        test_prompts = {
+            "system": {
+                "role": "You are a helpful assistant",
+                "task": "Answer questions",
+            },
+            "simple_key": "Simple value",
+        }
+
+        # Test nested access
+        role = pm.get_prompt(test_prompts, "system", "role")
+        assert role == "You are a helpful assistant"
+
+        # Test simple access (no field)
+        simple = pm.get_prompt(test_prompts, "simple_key")
+        assert simple == "Simple value"
+
+        # Test fallback
+        missing = pm.get_prompt(test_prompts, "nonexistent", "field", "fallback_value")
+        assert missing == "fallback_value"
+
+    def test_language_fallback(self):
+        """Test language fallback chain."""
+        pm = get_prompt_manager()
+
+        # Even with a potentially missing language, should fallback
+        prompts = pm.load_prompts("research", "research_agent", "zh")
+        assert isinstance(prompts, dict)
+
+    def test_reload_prompts(self):
+        """Test force reload bypasses cache."""
+        pm = get_prompt_manager()
+
+        # Load and cache
+        prompts1 = pm.load_prompts("research", "research_agent", "en")
+
+        # Force reload
+        prompts2 = pm.reload_prompts("research", "research_agent", "en")
+
+        # They should be equal but not the same object
+        assert prompts1 == prompts2
+        # After reload, cache should have fresh entry
+        cache_key = "research_research_agent_en"
+        assert cache_key in pm._cache
+
+
+class TestPromptManagerLanguages:
+    """Test language handling."""
+
+    def setup_method(self):
+        PromptManager._instance = None
+        PromptManager._cache = {}
+
+    def test_english_prompts(self):
+        """Test loading English prompts."""
+        pm = get_prompt_manager()
+        prompts = pm.load_prompts("guide", "chat_agent", "en")
+        assert isinstance(prompts, dict)
+
+    def test_chinese_prompts(self):
+        """Test loading Chinese prompts."""
+        pm = get_prompt_manager()
+        prompts = pm.load_prompts("guide", "chat_agent", "zh")
+        assert isinstance(prompts, dict)
+
+    def test_invalid_language_falls_back(self):
+        """Test that invalid language code falls back gracefully."""
+        pm = get_prompt_manager()
+        # Should not raise, should fallback
+        prompts = pm.load_prompts("research", "research_agent", "invalid")
+        assert isinstance(prompts, dict)
+
+
+if __name__ == "__main__":
+    pytest.main([__file__, "-v"])