Merge pull request #284 from Vaso73/fix/openai-compatible-ai-models

fix(notifications): respect OpenAI-compatible model aliases

AppImage/scripts/ai_providers/openai_provider.py

@@ -183,6 +183,12 @@ class OpenAIProvider(AIProvider):
             ],
         }
 
+        # Custom OpenAI-compatible endpoints often expose opaque aliases whose
+        # upstream capabilities are known only to the proxy. Do not infer
+        # sampling or reasoning parameters from those aliases; let the proxy
+        # apply model-specific defaults.
+        if self.base_url:
+            payload['max_tokens'] = max_tokens
         # Reasoning models (o1/o3/o4/gpt-5*, excluding *-chat-latest) use a
         # different parameter contract: max_completion_tokens instead of
         # max_tokens, and no temperature field. Sending the classic chat
@@ -196,7 +202,7 @@ class OpenAIProvider(AIProvider):
         # exactly what this pipeline wants. OpenAI documents 'minimal',
         # 'low', 'medium', 'high' — 'minimal' is the right setting for a
         # straightforward translate+explain task.
-        if self._is_reasoning_model(self.model):
+        elif self._is_reasoning_model(self.model):
             payload['max_completion_tokens'] = max_tokens
             payload['reasoning_effort'] = 'minimal'
         else:

AppImage/scripts/notification_manager.py

@@ -895,6 +895,40 @@ class NotificationManager:
             self._config[key] = value
         except Exception as e:
             print(f"[NotificationManager] Failed to save setting {key}: {e}")
+
+    def _active_ai_model(self, provider_name: str) -> str:
+        """Return the model selected for the active provider.
+
+        `ai_model` is the legacy global key. Newer settings persist
+        provider-specific models as `ai_model_<provider>`, and those must win
+        whenever present so custom endpoints keep their opaque aliases.
+        """
+        return (
+            self._config.get(f'ai_model_{provider_name}', '')
+            or self._config.get('ai_model', '')
+        )
+
+    def _build_ai_config(self) -> Dict[str, Any]:
+        """Build the shared AI config passed to notification rewriters."""
+        ai_provider = self._config.get('ai_provider', 'groq')
+        ai_api_key = self._config.get(f'ai_api_key_{ai_provider}', '') or self._config.get('ai_api_key', '')
+        return {
+            'ai_enabled': self._config.get('ai_enabled', 'false'),
+            'ai_provider': ai_provider,
+            'ai_api_key': ai_api_key,
+            'ai_model': self._active_ai_model(ai_provider),
+            'ai_language': self._config.get('ai_language', 'en'),
+            'ai_ollama_url': self._config.get('ai_ollama_url', ''),
+            # `ai_openai_base_url` was previously dropped from this dict and
+            # the downstream `notification_templates.AIRewriter` read it from
+            # the dict — meaning a user who configured LiteLLM / Azure as a
+            # custom base_url passed the "Test AI" check (which DOES pass it)
+            # but every real notification silently went to api.openai.com.
+            # Privacy + UX deception bug. Audit Tier 3.2 #1.
+            'ai_openai_base_url': self._config.get('ai_openai_base_url', ''),
+            'ai_prompt_mode': self._config.get('ai_prompt_mode', 'default'),
+            'ai_custom_prompt': self._config.get('ai_custom_prompt', ''),
+        }
     
     def _rebuild_channels(self):
         """Rebuild channel instances from current config.
@@ -1216,26 +1250,7 @@ class NotificationManager:
         default_event_enabled = 'true' if template.get('default_enabled', True) else 'false'
         
         # Build AI config once (shared across channels, detail_level varies)
-        # Use per-provider API key
-        ai_provider = self._config.get('ai_provider', 'groq')
-        ai_api_key = self._config.get(f'ai_api_key_{ai_provider}', '') or self._config.get('ai_api_key', '')
-        ai_config = {
-            'ai_enabled': self._config.get('ai_enabled', 'false'),
-            'ai_provider': ai_provider,
-            'ai_api_key': ai_api_key,
-            'ai_model': self._config.get('ai_model', ''),
-            'ai_language': self._config.get('ai_language', 'en'),
-            'ai_ollama_url': self._config.get('ai_ollama_url', ''),
-            # `ai_openai_base_url` was previously dropped from this dict and
-            # the downstream `notification_templates.AIRewriter` read it from
-            # the dict — meaning a user who configured LiteLLM / Azure as a
-            # custom base_url passed the "Test AI" check (which DOES pass it)
-            # but every real notification silently went to api.openai.com.
-            # Privacy + UX deception bug. Audit Tier 3.2 #1.
-            'ai_openai_base_url': self._config.get('ai_openai_base_url', ''),
-            'ai_prompt_mode': self._config.get('ai_prompt_mode', 'default'),
-            'ai_custom_prompt': self._config.get('ai_custom_prompt', ''),
-        }
+        ai_config = self._build_ai_config()
         
         # Get journal context if available (will be enriched per-channel based on detail_level)
         raw_journal_context = data.get('_journal_context', '')
@@ -2163,26 +2178,8 @@ class NotificationManager:
             message = rendered['body']
             severity = severity or rendered['severity']
         
-        # AI config for enhancement - use per-provider API key
-        ai_provider = self._config.get('ai_provider', 'groq')
-        ai_api_key = self._config.get(f'ai_api_key_{ai_provider}', '') or self._config.get('ai_api_key', '')
-        ai_config = {
-            'ai_enabled': self._config.get('ai_enabled', 'false'),
-            'ai_provider': ai_provider,
-            'ai_api_key': ai_api_key,
-            'ai_model': self._config.get('ai_model', ''),
-            'ai_language': self._config.get('ai_language', 'en'),
-            'ai_ollama_url': self._config.get('ai_ollama_url', ''),
-            # `ai_openai_base_url` was previously dropped from this dict and
-            # the downstream `notification_templates.AIRewriter` read it from
-            # the dict — meaning a user who configured LiteLLM / Azure as a
-            # custom base_url passed the "Test AI" check (which DOES pass it)
-            # but every real notification silently went to api.openai.com.
-            # Privacy + UX deception bug. Audit Tier 3.2 #1.
-            'ai_openai_base_url': self._config.get('ai_openai_base_url', ''),
-            'ai_prompt_mode': self._config.get('ai_prompt_mode', 'default'),
-            'ai_custom_prompt': self._config.get('ai_custom_prompt', ''),
-        }
+        # AI config for enhancement
+        ai_config = self._build_ai_config()
         
         results = {}
         channels_sent = []
@@ -2268,26 +2265,9 @@ class NotificationManager:
             else:
                 return {'success': False, 'error': f'Channel {channel_name} not configured'}
         
-        # AI config for enhancement - use per-provider API key
-        ai_provider = self._config.get('ai_provider', 'groq')
-        ai_api_key = self._config.get(f'ai_api_key_{ai_provider}', '') or self._config.get('ai_api_key', '')
-        ai_config = {
-            'ai_enabled': self._config.get('ai_enabled', 'false'),
-            'ai_provider': ai_provider,
-            'ai_api_key': ai_api_key,
-            'ai_model': self._config.get('ai_model', ''),
-            'ai_language': self._config.get('ai_language', 'en'),
-            'ai_ollama_url': self._config.get('ai_ollama_url', ''),
-            # `ai_openai_base_url` was previously dropped from this dict and
-            # the downstream `notification_templates.AIRewriter` read it from
-            # the dict — meaning a user who configured LiteLLM / Azure as a
-            # custom base_url passed the "Test AI" check (which DOES pass it)
-            # but every real notification silently went to api.openai.com.
-            # Privacy + UX deception bug. Audit Tier 3.2 #1.
-            'ai_openai_base_url': self._config.get('ai_openai_base_url', ''),
-            'ai_prompt_mode': self._config.get('ai_prompt_mode', 'default'),
-            'ai_custom_prompt': self._config.get('ai_custom_prompt', ''),
-        }
+        # AI config for enhancement
+        ai_config = self._build_ai_config()
+        ai_provider = ai_config.get('ai_provider', 'groq')
         
         ai_enabled = self._config.get('ai_enabled', 'false')
         if isinstance(ai_enabled, str):
@@ -2718,7 +2698,7 @@ class NotificationManager:
             'ai_provider': current_provider,
             'ai_api_keys': ai_api_keys,
             'ai_models': ai_models,
-            'ai_model': self._config.get('ai_model', ''),
+            'ai_model': self._active_ai_model(current_provider),
             'ai_language': self._config.get('ai_language', 'en'),
             'ai_ollama_url': self._config.get('ai_ollama_url', 'http://localhost:11434'),
             'ai_openai_base_url': self._config.get('ai_openai_base_url', ''),
@@ -2893,7 +2873,7 @@ class NotificationManager:
             return {'checked': False, 'migrated': False, 'message': 'AI not enabled'}
         
         provider_name = self._config.get('ai_provider', 'groq')
-        current_model = self._config.get('ai_model', '')
+        current_model = self._active_ai_model(provider_name)
         
         # Skip Ollama - user manages their own models
         if provider_name == 'ollama':
@@ -2927,7 +2907,13 @@ class NotificationManager:
                 print(f"[NotificationManager] Failed to load verified models: {e}")
             
             from ai_providers import get_provider
-            provider = get_provider(provider_name, api_key=api_key, model=current_model)
+            provider_kwargs = {
+                'api_key': api_key,
+                'model': current_model,
+            }
+            if provider_name == 'openai':
+                provider_kwargs['base_url'] = self._config.get('ai_openai_base_url', '')
+            provider = get_provider(provider_name, **provider_kwargs)
             
             if not provider:
                 return {'checked': False, 'migrated': False, 'message': f'Unknown provider: {provider_name}'}
@@ -2935,8 +2921,24 @@ class NotificationManager:
             # Get available models from API
             api_models = provider.list_models()
             
-            # Combine: use verified models that are also in API (or all verified if API fails)
-            if api_models and verified_models:
+            # Combine: official providers intersect the API list with the
+            # verified catalogue. Custom OpenAI-compatible endpoints are
+            # authoritative for their own opaque aliases, so do not intersect
+            # them with ProxMenux's bundled official OpenAI IDs.
+            openai_custom_endpoint = (
+                provider_name == 'openai'
+                and bool(self._config.get('ai_openai_base_url', '').strip())
+            )
+            if openai_custom_endpoint:
+                if not api_models:
+                    return {
+                        'checked': True,
+                        'migrated': False,
+                        'new_model': current_model,
+                        'message': 'Could not retrieve custom endpoint model list'
+                    }
+                available_models = api_models
+            elif api_models and verified_models:
                 available_models = [m for m in verified_models if m in api_models]
             elif verified_models:
                 available_models = verified_models
@@ -2970,13 +2972,16 @@ class NotificationManager:
             try:
                 conn = sqlite3.connect(str(DB_PATH), timeout=10)
                 cursor = conn.cursor()
-                cursor.execute('''
-                    INSERT OR REPLACE INTO user_settings (setting_key, setting_value, updated_at)
-                    VALUES (?, ?, ?)
-                ''', (f'{SETTINGS_PREFIX}ai_model', recommended, datetime.now().isoformat()))
+                now_iso = datetime.now().isoformat()
+                for model_key in ('ai_model', f'ai_model_{provider_name}'):
+                    cursor.execute('''
+                        INSERT OR REPLACE INTO user_settings (setting_key, setting_value, updated_at)
+                        VALUES (?, ?, ?)
+                    ''', (f'{SETTINGS_PREFIX}{model_key}', recommended, now_iso))
                 conn.commit()
                 conn.close()
                 self._config['ai_model'] = recommended
+                self._config[f'ai_model_{provider_name}'] = recommended
                 
                 print(f"[NotificationManager] AI model migrated: {old_model} -> {recommended}")
                 

AppImage/scripts/tests/test_openai_compatible_ai_models.py

@@ -0,0 +1,185 @@
+import sqlite3
+import sys
+import tempfile
+import unittest
+from pathlib import Path
+from unittest import mock
+
+
+SCRIPTS_DIR = Path(__file__).resolve().parents[1]
+if str(SCRIPTS_DIR) not in sys.path:
+    sys.path.insert(0, str(SCRIPTS_DIR))
+
+import ai_providers
+import notification_manager
+from ai_providers.openai_provider import OpenAIProvider
+
+
+class CapturingOpenAIProvider(OpenAIProvider):
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        self.captured_payload = None
+
+    def _make_request(self, url, payload, headers):
+        self.captured_payload = payload
+        return {"choices": [{"message": {"content": "ok"}}]}
+
+
+class FakeProvider:
+    last_kwargs = None
+    models = []
+
+    def __init__(self, **kwargs):
+        FakeProvider.last_kwargs = kwargs
+
+    def list_models(self):
+        return list(FakeProvider.models)
+
+
+class OpenAICompatibleModelTests(unittest.TestCase):
+    def setUp(self):
+        FakeProvider.last_kwargs = None
+        FakeProvider.models = []
+        self.provider_patch = mock.patch.dict(
+            ai_providers.PROVIDERS,
+            {"openai": FakeProvider},
+        )
+        self.provider_patch.start()
+        self.addCleanup(self.provider_patch.stop)
+
+    def _manager(self, config):
+        manager = notification_manager.NotificationManager()
+        manager._config = dict(config)
+        manager._enabled = manager._config.get("enabled", "false") == "true"
+        return manager
+
+    def _temp_db_patch(self):
+        temp_dir = tempfile.TemporaryDirectory()
+        self.addCleanup(temp_dir.cleanup)
+        db_path = Path(temp_dir.name) / "health_monitor.db"
+        conn = sqlite3.connect(str(db_path))
+        conn.execute(
+            "CREATE TABLE user_settings (setting_key TEXT PRIMARY KEY, "
+            "setting_value TEXT, updated_at TEXT)"
+        )
+        conn.commit()
+        conn.close()
+        patcher = mock.patch.object(notification_manager, "DB_PATH", db_path)
+        patcher.start()
+        self.addCleanup(patcher.stop)
+        return db_path
+
+    def test_custom_openai_endpoint_omits_temperature_for_opaque_alias(self):
+        provider = CapturingOpenAIProvider(
+            api_key="token",
+            model="opaque-gpt5-alias",
+            base_url="https://litellm.example",
+        )
+
+        self.assertEqual(provider.generate("system", "user", max_tokens=123), "ok")
+
+        self.assertEqual(provider.captured_payload["model"], "opaque-gpt5-alias")
+        self.assertEqual(provider.captured_payload["max_tokens"], 123)
+        self.assertNotIn("temperature", provider.captured_payload)
+        self.assertNotIn("reasoning_effort", provider.captured_payload)
+        self.assertNotIn("max_completion_tokens", provider.captured_payload)
+
+    def test_official_openai_reasoning_model_still_uses_reasoning_contract(self):
+        provider = CapturingOpenAIProvider(
+            api_key="token",
+            model="gpt-5-mini",
+        )
+
+        self.assertEqual(provider.generate("system", "user", max_tokens=123), "ok")
+
+        self.assertEqual(provider.captured_payload["model"], "gpt-5-mini")
+        self.assertEqual(provider.captured_payload["max_completion_tokens"], 123)
+        self.assertEqual(provider.captured_payload["reasoning_effort"], "minimal")
+        self.assertNotIn("temperature", provider.captured_payload)
+        self.assertNotIn("max_tokens", provider.captured_payload)
+
+    def test_runtime_ai_config_prefers_provider_specific_model(self):
+        manager = self._manager({
+            "ai_enabled": "true",
+            "ai_provider": "openai",
+            "ai_api_key_openai": "token",
+            "ai_model": "gpt-4.1-nano",
+            "ai_model_openai": "proxy-alias",
+            "ai_openai_base_url": "https://litellm.example",
+        })
+
+        ai_config = manager._build_ai_config()
+
+        self.assertEqual(ai_config["ai_model"], "proxy-alias")
+        self.assertEqual(ai_config["ai_openai_base_url"], "https://litellm.example")
+
+    def test_model_verifier_uses_custom_endpoint_alias_without_migration(self):
+        manager = self._manager({
+            "ai_enabled": "true",
+            "ai_provider": "openai",
+            "ai_api_key_openai": "token",
+            "ai_model": "gpt-4.1-nano",
+            "ai_model_openai": "proxy-alias",
+            "ai_openai_base_url": "https://litellm.example",
+        })
+        FakeProvider.models = ["proxy-alias"]
+
+        result = manager.verify_and_update_ai_model()
+
+        self.assertTrue(result["checked"])
+        self.assertFalse(result["migrated"])
+        self.assertEqual(result["new_model"], "proxy-alias")
+        self.assertEqual(FakeProvider.last_kwargs["model"], "proxy-alias")
+        self.assertEqual(FakeProvider.last_kwargs["base_url"], "https://litellm.example")
+
+    def test_custom_endpoint_does_not_fallback_to_official_model_catalogue(self):
+        manager = self._manager({
+            "ai_enabled": "true",
+            "ai_provider": "openai",
+            "ai_api_key_openai": "token",
+            "ai_model": "gpt-4.1-nano",
+            "ai_model_openai": "proxy-alias",
+            "ai_openai_base_url": "https://litellm.example",
+        })
+        FakeProvider.models = []
+
+        result = manager.verify_and_update_ai_model()
+
+        self.assertTrue(result["checked"])
+        self.assertFalse(result["migrated"])
+        self.assertEqual(result["new_model"], "proxy-alias")
+        self.assertEqual(result["message"], "Could not retrieve custom endpoint model list")
+
+    def test_model_migration_updates_legacy_and_provider_specific_keys(self):
+        db_path = self._temp_db_patch()
+        manager = self._manager({
+            "ai_enabled": "true",
+            "ai_provider": "openai",
+            "ai_api_key_openai": "token",
+            "ai_model": "old-generic",
+            "ai_model_openai": "old-alias",
+            "ai_openai_base_url": "https://litellm.example",
+        })
+        FakeProvider.models = ["new-alias"]
+
+        result = manager.verify_and_update_ai_model()
+
+        self.assertTrue(result["checked"])
+        self.assertTrue(result["migrated"])
+        self.assertEqual(result["old_model"], "old-alias")
+        self.assertEqual(result["new_model"], "new-alias")
+        self.assertEqual(manager._config["ai_model"], "new-alias")
+        self.assertEqual(manager._config["ai_model_openai"], "new-alias")
+
+        conn = sqlite3.connect(str(db_path))
+        rows = dict(conn.execute(
+            "SELECT setting_key, setting_value FROM user_settings"
+        ).fetchall())
+        conn.close()
+
+        self.assertEqual(rows["notification.ai_model"], "new-alias")
+        self.assertEqual(rows["notification.ai_model_openai"], "new-alias")
+
+
+if __name__ == "__main__":
+    unittest.main()