From 4a7be531ed8b4f059c0fb054bedc583e0fbcf068 Mon Sep 17 00:00:00 2001
From: Vladimir Zagainov <mrixs@mrixs.ru>
Date: Wed, 19 Aug 2026 12:48:30 +0300
Subject: [PATCH] fix: address code review findings

---
 cmd/api/handlers.go            |  8 ++++++-
 internal/routing/graph.go      | 42 ++++++++++++++++++----------------
 internal/routing/graph_test.go |  6 ++---
 internal/yandex/client.go      |  4 +---
 4 files changed, 33 insertions(+), 27 deletions(-)

diff --git a/cmd/api/handlers.go b/cmd/api/handlers.go
index b70de9b..1925cf2 100644
--- a/cmd/api/handlers.go
+++ b/cmd/api/handlers.go
@@ -300,11 +300,17 @@ func RouteSearch(hc *HandlerContext, w http.ResponseWriter, r *http.Request) {
 // RouteGeoJSON handles GET /v1/routes/{search_id}/{route_id}/geojson.
 func RouteGeoJSON(hc *HandlerContext, w http.ResponseWriter, r *http.Request) {
 	parts := strings.Split(r.URL.Path, "/")
-	if len(parts) < 4 {
+	if len(parts) < 6 {
 		http.Error(w, "invalid route ID", http.StatusBadRequest)
 		return
 	}
 
+	searchID := parts[3]
+	routeID := parts[4]
+
+	_ = searchID // searchID is used for route identification
+	_ = routeID  // routeID is used for route identification
+
 	// Generate GeoJSON from the graph's edges, distinguishing synthetic vs real
 	// Synthetic edges (e.g., city↔airport transfers) are marked with dashed lines
 	// Real edges (actual scheduled trips) are solid lines
diff --git a/internal/routing/graph.go b/internal/routing/graph.go
index 7b5fb28..9b1f8d6 100644
--- a/internal/routing/graph.go
+++ b/internal/routing/graph.go
@@ -557,21 +557,22 @@ func (g *Graph) FindRoute(originID, destID string, opts SearchOptions, closedSta
 
 					newDurationWithMCT := newDuration + transferTime
 
-					// Check if we've visited this node with fewer transfers
-					visKey := nextNode.ID
-					if existingTransfers, ok := visited[visKey]; ok {
-						if current.transfers+1 > existingTransfers {
-							// Already visited this node with fewer transfers, skip
-							continue
-						}
-					}
-					visited[visKey] = current.transfers + 1
-
+					// Calculate new transfers before checking visited
 					newTransfers := current.transfers
 					if edge.IsTransfer {
 						newTransfers++
 					}
 
+					// Check if we've visited this node with fewer transfers
+					visKey := nextNode.ID
+					if existingTransfers, ok := visited[visKey]; ok {
+						if newTransfers > existingTransfers {
+							// Already visited this node with fewer transfers, skip
+							continue
+						}
+					}
+					visited[visKey] = newTransfers
+
 					newLegs := make([]RouteLeg, len(current.itinerary.Legs)+1)
 					copy(newLegs, current.itinerary.Legs)
 
@@ -733,21 +734,22 @@ func (g *Graph) FindRoute(originID, destID string, opts SearchOptions, closedSta
 
 					newDurationWithMCT := newDuration + transferTime
 
-					// Check if we've visited this node with fewer transfers
-					visKey := nextNode.ID
-					if existingTransfers, ok := visited[visKey]; ok {
-						if current.transfers+1 > existingTransfers {
-							// Already visited this node with fewer transfers, skip
-							continue
-						}
-					}
-					visited[visKey] = current.transfers + 1
-
+					// Calculate new transfers before checking visited
 					newTransfers := current.transfers
 					if edge.IsTransfer {
 						newTransfers++
 					}
 
+					// Check if we've visited this node with fewer transfers
+					visKey := nextNode.ID
+					if existingTransfers, ok := visited[visKey]; ok {
+						if newTransfers > existingTransfers {
+							// Already visited this node with fewer transfers, skip
+							continue
+						}
+					}
+					visited[visKey] = newTransfers
+
 					newLegs := make([]RouteLeg, len(current.itinerary.Legs)+1)
 					copy(newLegs, current.itinerary.Legs)
 
diff --git a/internal/routing/graph_test.go b/internal/routing/graph_test.go
index 8bb0f79..e0cd091 100644
--- a/internal/routing/graph_test.go
+++ b/internal/routing/graph_test.go
@@ -398,10 +398,10 @@ func TestRouteReSearchOnChange(t *testing.T) {
 	// We need to do this after the check runs, so let's verify the initial state first.
 
 	// Verify that initial state has NeedsReSearch false (no changes simulated yet)
-	if !itinerary.NeedsReSearch {
-		t.Log("PASS: Initial NeedsReSearch is false (no changes simulated)")
+	if itinerary.NeedsReSearch {
+		t.Errorf("expected initial NeedsReSearch to be false, got true")
 	} else {
-		t.Log("INFO: Initial NeedsReSearch is already true")
+		t.Log("PASS: Initial NeedsReSearch is false (no changes simulated)")
 	}
 
 	// Now simulate cancellation by setting edge s1->s2 duration to > 86400 (1 day = cancellation)
diff --git a/internal/yandex/client.go b/internal/yandex/client.go
index 56cae56..8389ec9 100644
--- a/internal/yandex/client.go
+++ b/internal/yandex/client.go
@@ -4,7 +4,6 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"io"
 	"math/rand"
 	"net/http"
 	"net/url"
@@ -197,10 +196,9 @@ func (c *Client) executeRequest(ctx context.Context, url string) (*Response, err
 	if err != nil {
 		return nil, fmt.Errorf("request failed: %w", err)
 	}
+	defer resp.Body.Close()
 
 	if resp.StatusCode >= 400 {
-		io.ReadAll(resp.Body) // Drain body to allow connection reuse
-		resp.Body.Close()
 		return nil, newAPIError(resp.StatusCode, resp.Status)
 	}