Improve performance and error handling in web components

.github/workflows/ci.yml

@@ -137,6 +137,7 @@ jobs:
           python-version: '3.14'
           allow-prereleases: true
           cache: 'pip'
+          cache-dependency-path: src/Garage.ChatService/requirements.txt
       - name: Install dependencies
         run: |
           python -m pip install --upgrade pip

global.json

@@ -1,6 +1,6 @@
 {
   "sdk": {
     "version": "10.0.100",
-    "rollForward": "latestPatch"
+    "rollForward": "latestFeature"
   }
 }

src/Garage.ApiService/Services/WinnersService.cs

@@ -20,21 +20,21 @@ public async Task<IEnumerable<Winner>> GetAllWinnersAsync()
             .SetTargetingKey(Guid.NewGuid().ToString())
             .Build();
 
-        var winners = await featureClient.GetBooleanValueAsync("enable-database-winners", false, evaluationContext)
-            ? await GetAllDatabaseWinnersAsync()
-            : await GetAllJsonWinnersAsync();
-
         var count = await featureClient.GetIntegerDetailsAsync("winners-count", 5, evaluationContext);
 
-        return winners.Take(count.Value);
+        return await featureClient.GetBooleanValueAsync("enable-database-winners", false, evaluationContext)
+            ? await GetAllDatabaseWinnersAsync(count.Value)
+            : await GetAllJsonWinnersAsync(count.Value);
     }
 
-    private async Task<IEnumerable<Winner>> GetAllDatabaseWinnersAsync()
+    private async Task<IEnumerable<Winner>> GetAllDatabaseWinnersAsync(int count)
     {
         try
         {
             var winnersDatabase = await context.Winners
+                .AsNoTracking()
                 .OrderByDescending(w => w.Year)
+                .Take(count)
                 .ToListAsync();
 
             var mapper = new WinnerMapper();
@@ -48,7 +48,7 @@ private async Task<IEnumerable<Winner>> GetAllDatabaseWinnersAsync()
         }
     }
 
-    private async Task<IEnumerable<Winner>> GetAllJsonWinnersAsync()
+    private async Task<IEnumerable<Winner>> GetAllJsonWinnersAsync(int count)
     {
         await SlowDownAsync();
         var dataFilePath = Path.Combine(AppContext.BaseDirectory, "Data", "winners.json");
@@ -60,7 +60,7 @@ private async Task<IEnumerable<Winner>> GetAllJsonWinnersAsync()
                 PropertyNameCaseInsensitive = true
             });
 
-            return winners?.OrderByDescending(w => w.Year) ?? Enumerable.Empty<Winner>();
+            return winners?.OrderByDescending(w => w.Year).Take(count) ?? Enumerable.Empty<Winner>();
         }
         catch (Exception ex)
         {

src/Garage.ChatService/main.py

@@ -1,10 +1,13 @@
 """Le Mans Chatbot Service - FastAPI application using GitHub Models and OpenFeature."""
 
 import os
+import time
 import logging
 from contextlib import asynccontextmanager
 from pathlib import Path
 
+import grpc
+
 from fastapi import FastAPI, HTTPException
 from fastapi.middleware.cors import CORSMiddleware
 from pydantic import BaseModel
@@ -104,7 +107,6 @@ def setup_telemetry():
                 logger.info(f"Using CA cert from SSL_CERT_FILE: {ca_cert_path}")
                 with open(ca_cert_path, "rb") as f:
                     ca_cert = f.read()
-                import grpc
                 credentials = grpc.ssl_channel_credentials(root_certificates=ca_cert)
 
         # Setup tracing
@@ -245,7 +247,6 @@ async def health_check():
 @app.post("/chat", response_model=ChatResponse)
 async def chat(request: ChatRequest):
     """Chat endpoint that uses GitHub Models with dynamic prompt selection."""
-    import time
     start_time = time.time()
 
     tracer = trace.get_tracer(__name__)

src/Garage.ChatService/prompt_loader.py

@@ -1,20 +1,25 @@
 """Utility module for loading and rendering GitHub Repository Prompts (.prompt.yml files)."""
 
 import yaml
+from functools import lru_cache
 from pathlib import Path
 from typing import Any
 
 
+@lru_cache(maxsize=10)
 def load_prompt(prompt_name: str, prompts_dir: str = "prompts") -> dict[str, Any]:
     """Load a .prompt.yml file and return parsed content.
-
+
+    Results are cached in-memory so repeated calls with the same arguments
+    avoid redundant disk I/O and YAML parsing.
+
     Args:
         prompt_name: Name of the prompt (without extension)
         prompts_dir: Directory containing prompt files
-        
+
     Returns:
         Parsed YAML content as a dictionary
-        
+
     Raises:
         FileNotFoundError: If the prompt file doesn't exist
     """
@@ -25,13 +30,13 @@ def load_prompt(prompt_name: str, prompts_dir: str = "prompts") -> dict[str, Any
 
 def render_messages(prompt: dict[str, Any], variables: dict[str, str]) -> list[dict[str, str]]:
     """Render prompt messages with variable substitution.
-    
+
     Replaces {{variable}} placeholders in message content with provided values.
-    
+
     Args:
         prompt: Parsed prompt dictionary containing 'messages' key
         variables: Dictionary of variable names to values for substitution
-        
+
     Returns:
         List of message dictionaries with role and content keys
     """
@@ -46,10 +51,10 @@ def render_messages(prompt: dict[str, Any], variables: dict[str, str]) -> list[d
 
 def get_model_parameters(prompt: dict[str, Any]) -> dict[str, Any]:
     """Extract model parameters from a prompt.
-    
+
     Args:
         prompt: Parsed prompt dictionary
-        
+
     Returns:
         Dictionary of model parameters (e.g., temperature)
     """

src/Garage.ChatService/requirements-test.txt

@@ -0,0 +1 @@
+pytest>=8.3.0

src/Garage.ChatService/test_prompt_loader.py

@@ -0,0 +1,105 @@
+"""Unit tests for prompt_loader module."""
+
+import sys
+from pathlib import Path
+
+import pytest
+
+# Ensure the directory containing this test (and prompt_loader.py) is on sys.path
+_THIS_DIR = Path(__file__).resolve().parent
+if str(_THIS_DIR) not in sys.path:
+    sys.path.insert(0, str(_THIS_DIR))
+from prompt_loader import load_prompt, render_messages, get_model_parameters
+
+
+class TestRenderMessages:
+    def test_substitutes_single_variable(self):
+        prompt = {"messages": [{"role": "user", "content": "Hello {{name}}!"}]}
+        result = render_messages(prompt, {"name": "World"})
+        assert result == [{"role": "user", "content": "Hello World!"}]
+
+    def test_substitutes_multiple_variables(self):
+        prompt = {
+            "messages": [
+                {"role": "system", "content": "You are {{persona}}."},
+                {"role": "user", "content": "{{message}}"},
+            ]
+        }
+        result = render_messages(prompt, {"persona": "an expert", "message": "Tell me more"})
+        assert result[0]["content"] == "You are an expert."
+        assert result[1]["content"] == "Tell me more"
+
+    def test_leaves_unmatched_placeholder_unchanged(self):
+        prompt = {"messages": [{"role": "user", "content": "Hello {{unknown}}!"}]}
+        result = render_messages(prompt, {"name": "World"})
+        assert result == [{"role": "user", "content": "Hello {{unknown}}!"}]
+
+    def test_empty_messages_returns_empty_list(self):
+        assert render_messages({}, {}) == []
+        assert render_messages({"messages": []}, {}) == []
+
+    def test_preserves_message_roles(self):
+        prompt = {
+            "messages": [
+                {"role": "system", "content": "System prompt"},
+                {"role": "user", "content": "User {{msg}}"},
+                {"role": "assistant", "content": "Assistant response"},
+            ]
+        }
+        result = render_messages(prompt, {"msg": "question"})
+        assert [m["role"] for m in result] == ["system", "user", "assistant"]
+
+    def test_returns_new_list_not_mutation(self):
+        original = [{"role": "user", "content": "Hello {{x}}"}]
+        prompt = {"messages": original}
+        result = render_messages(prompt, {"x": "test"})
+        assert result is not prompt["messages"]
+        assert original[0]["content"] == "Hello {{x}}"
+
+
+class TestGetModelParameters:
+    def test_returns_model_parameters(self):
+        prompt = {"modelParameters": {"temperature": 0.7, "max_tokens": 100}}
+        result = get_model_parameters(prompt)
+        assert result == {"temperature": 0.7, "max_tokens": 100}
+
+    def test_returns_empty_dict_when_missing(self):
+        assert get_model_parameters({}) == {}
+        assert get_model_parameters({"messages": []}) == {}
+
+    def test_returns_empty_dict_for_empty_parameters(self):
+        assert get_model_parameters({"modelParameters": {}}) == {}
+
+
+class TestLoadPrompt:
+    def test_loads_valid_prompt_file(self, tmp_path: Path):
+        prompt_dir = tmp_path / "prompts"
+        prompt_dir.mkdir()
+        prompt_file = prompt_dir / "test.prompt.yml"
+        prompt_file.write_text(
+            "name: Test Prompt\n"
+            "messages:\n"
+            "  - role: user\n"
+            "    content: Hello {{message}}\n"
+            "modelParameters:\n"
+            "  temperature: 0.5\n",
+            encoding="utf-8",
+        )
+        result = load_prompt("test", str(prompt_dir))
+        assert result["name"] == "Test Prompt"
+        assert result["modelParameters"]["temperature"] == 0.5
+        assert result["messages"][0]["role"] == "user"
+
+    def test_raises_file_not_found_for_missing_prompt(self, tmp_path: Path):
+        with pytest.raises(FileNotFoundError):
+            load_prompt("nonexistent", str(tmp_path))
+
+    def test_loads_prompt_with_default_directory(self, tmp_path: Path):
+        """Verify load_prompt uses the prompts_dir parameter correctly."""
+        prompt_dir = tmp_path / "prompts"
+        prompt_dir.mkdir()
+        (prompt_dir / "simple.prompt.yml").write_text(
+            "name: Simple\nmessages: []\n", encoding="utf-8"
+        )
+        result = load_prompt("simple", str(prompt_dir))
+        assert result["name"] == "Simple"

src/Garage.FeatureFlags/main_test.go

@@ -90,8 +90,8 @@ func fetch(t *testing.T, url string, method string, body io.Reader) (int, string
 		t.Fatalf("reading response body: %v", err)
 	}
 
-	if err != resp.Body.Close() {
-		t.Fatalf("making request: %v", err)
+	if err := resp.Body.Close(); err != nil {
+		t.Fatalf("closing response body: %v", err)
 	}
 
 	return resp.StatusCode, string(data)

src/Garage.Web/src/components/CarCard.tsx

@@ -10,7 +10,6 @@ const CarCard: React.FC<CarCardProps> = ({ car, onOwnershipChanged }) => {
   const toggleOwnership = (event: React.ChangeEvent<HTMLInputElement>) => {
     const updatedCar = { ...car, isOwned: event.target.checked };
     onOwnershipChanged(updatedCar);
-    console.log(`Toggling ownership for car ${car.year}`);
   };
 
   return (

src/Garage.Web/src/components/FeatureFlagsModal.tsx

@@ -16,6 +16,7 @@ type FlagsMap = Record<string, FlagState>;
 const FeatureFlagsModal = ({ isOpen, onClose }: FeatureFlagsModalProps) => {
   const [flags, setFlags] = useState<FlagsMap>({});
   const [loading, setLoading] = useState(true);
+  const [fetchError, setFetchError] = useState<string | null>(null);
   const timeoutRefs = useState<Map<string, NodeJS.Timeout>>(
     () => new Map()
   )[0];
@@ -25,6 +26,7 @@ const FeatureFlagsModal = ({ isOpen, onClose }: FeatureFlagsModalProps) => {
   const fetchFlags = useCallback(async () => {
     try {
       setLoading(true);
+      setFetchError(null);
       const response = await fetch(`/flags/${userId}`);
       if (response.ok) {
         const data = await response.json();
@@ -36,8 +38,12 @@ const FeatureFlagsModal = ({ isOpen, onClose }: FeatureFlagsModalProps) => {
           };
         }
         setFlags(flagsData);
+      } else {
+        setFetchError("Failed to load flags. Please try again.");
+        console.error("Failed to fetch flags:", response.statusText);
       }
     } catch (err) {
+      setFetchError("Failed to load flags. Please try again.");
       console.error("Failed to fetch flags:", err);
     } finally {
       setLoading(false);
@@ -136,6 +142,8 @@ const FeatureFlagsModal = ({ isOpen, onClose }: FeatureFlagsModalProps) => {
         <div className="modal-body">
           {loading ? (
             <div className="loading">Loading flags...</div>
+          ) : fetchError ? (
+            <div className="error">{fetchError}</div>
           ) : flagKeys.length === 0 ? (
             <div className="no-flags">No flags available</div>
           ) : (

src/Garage.Web/src/components/Home.tsx

@@ -53,7 +53,6 @@ const Home = () => {
         }
         const winnersData: Winner[] = await response.json();
         setWinners(winnersData);
-        console.log(`Loaded ${winnersData.length} winners`);
       } catch (err) {
         console.error("Failed to load winners:", err);
         setError("Failed to load winners");
@@ -72,7 +71,6 @@ const Home = () => {
         winner.year === updatedCar.year ? updatedCar : winner
       )
     );
-    console.log(`Updated ownership for ${updatedCar.year} ${updatedCar.model}`);
   };
 
   const setFilter = (filterType: FilterType) => {