fix: address code review findings

internal/cache/store.go

@@ -70,14 +70,11 @@ func (r *redisClient) Set(ctx context.Context, key *CacheKey, value []byte, ttl
 
 // Exists checks if a key exists in cache.
 func (r *redisClient) Exists(ctx context.Context, key *CacheKey) (bool, error) {
-	_, err := r.client.Exists(ctx, keyString(key)).Result()
+	count, err := r.client.Exists(ctx, keyString(key)).Result()
-	if errors.Is(err, redis.Nil) {
-		return false, nil
-	}
 	if err != nil {
 		return false, fmt.Errorf("cache exists: %w", err)
 	}
-	return true, nil
+	return count > 0, nil
 }
 
 // Delete removes a key from cache.

internal/routing/graph.go

@@ -1068,7 +1068,7 @@ func (g *Graph) checkRouteForChanges(itinerary *Itinerary) bool {
 					break
 				}
 				// Check for significant delay (more than 2x normal duration)
-				if edge.Duration > leg.Cost*2 && leg.Cost > 0 { // simplified delay check
+				if edge.Duration > leg.Duration*2 && leg.Duration > 0 {
 					if !needsReSearch || itinerary.ReSearchReason == string(reasonNone) {
 						needsReSearch = true
 						itinerary.NeedsReSearch = true

internal/routing/graph_test.go

@@ -325,9 +325,6 @@ func TestLazyExpansionDepthLimit(t *testing.T) {
 // TestRouteReSearchOnChange tests that the route change detection logic correctly
 // identifies when a route leg has undergone significant changes (cancellation or major delay)
 // and triggers a re-search to find an updated route.
-// TestRouteReSearchOnChange tests that the route change detection logic correctly
-// identifies when a route leg has undergone significant changes (cancellation or major delay)
-// and triggers a re-search to find an updated route.
 func TestRouteReSearchOnChange(t *testing.T) {
 	graph := NewGraph()
 

internal/routing/search_cache.go

@@ -2,6 +2,7 @@ package routing
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 
 	"trip-planner/internal/cache"
@@ -74,19 +75,23 @@ func (s *SearchCacheService) performYandexSearch(ctx context.Context, from, to,
 
 // parseItineraryFromBytes parses an itinerary from byte data.
 func parseItineraryFromBytes(data []byte, result *Itinerary) error {
-	// This is a placeholder - in real implementation, this would parse JSON
-	// into the Itinerary struct
 	if len(data) == 0 {
 		return fmt.Errorf("empty data")
 	}
+	if err := json.Unmarshal(data, result); err != nil {
+		return fmt.Errorf("failed to parse itinerary from bytes: %w", err)
+	}
 	return nil
 }
 
 // convertResponseToBytes converts Yandex API response to bytes for caching.
 func convertResponseToBytes(resp *yandex.Response) ([]byte, error) {
-	// This is a placeholder - in real implementation, this would serialize the response
 	if resp == nil {
 		return nil, fmt.Errorf("nil response")
 	}
-	return []byte(`{"search":{"from":"%s","to":"%s"},"segments":[]}`), nil
+	data, err := json.Marshal(resp)
+	if err != nil {
+		return nil, fmt.Errorf("failed to marshal response: %w", err)
+	}
+	return data, nil
 }

internal/routing/search_cache_test.go

@@ -77,10 +77,10 @@ func TestParseItineraryFromBytes(t *testing.T) {
 		t.Error("expected error for empty data")
 	}
 
-	// Test with non-empty data (placeholder implementation returns nil error)
+	// Test with invalid JSON data
-	err = parseItineraryFromBytes([]byte("test"), &result)
+	err = parseItineraryFromBytes([]byte("invalid json data"), &result)
-	if err != nil {
+	if err == nil {
-		t.Errorf("expected no error for non-empty data, got %v", err)
+		t.Error("expected error for invalid JSON data")
 	}
 }
 

internal/yandex/client_test.go

@@ -85,13 +85,18 @@ func TestCircuitBreakerOpenAfterFailures(t *testing.T) {
 		t.Errorf("expected state open, got %v", cb.state)
 	}
 
-	// Wait for timeout
+	// Test state transition to half-open by manually setting state and time
-	time.Sleep(31 * time.Second)
+	cb.state = open
+	cb.openSince = time.Now().Add(-31 * time.Second)
 
-	// Should transition to half-open/open after timeout - allow() should return true
+	// Should transition to half-open after timeout - allow() should return true
 	if !cb.allow() {
 		t.Error("expected allow() to return true after timeout")
 	}
+
+	if cb.state != halfOpen {
+		t.Errorf("expected state halfOpen after timeout, got %v", cb.state)
+	}
 }
 
 func TestCircuitBreakerRecordSuccess(t *testing.T) {
@@ -199,19 +204,10 @@ func TestRetryExhaustion(t *testing.T) {
 		jitter:      false,
 	}
 
-	// Simulate consecutive failures
+	// Verify maxRetries=2 means 3 total attempts (0, 1, 2)
-	var lastErr error
+	totalAttempts := cfg.maxRetries + 1
-	for attempt := 0; attempt <= cfg.maxRetries; attempt++ {
+	if totalAttempts != 3 {
-		// Simulate a non-retryable error that gets recorded as failure
+		t.Errorf("expected 3 total attempts with maxRetries=2, got %d", totalAttempts)
-		// In real code, isRetryableError would return false
-		lastErr = fmt.Errorf("attempt %d failed", attempt)
-		_ = lastErr // track last error
-	}
-
-	// After maxRetries+1 attempts (0-indexed: 0 to maxRetries), we've done 3 attempts
-	// with 2 retries (attempts 0->1, 1->2), the 3rd attempt (index 2) is the last
-	if cfg.maxRetries+1 < 3 {
-		t.Error("expected at least 3 attempts with maxRetries=2")
 	}
 }